Skip to contentSkip to conversation

btrfs: remove unneeded null check in btrfs_rename()

PATCH In review Dan Carpenter 3 participants 4 messages May 29, 2010 – May 31, 2010

Subsystems (2)
Kernel janitors Btrfs file system
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 5/11 btrfs: remove unneeded null check in btrfs_rename()

"old_inode" cannot be null here, because we dereference it unconditionally throughout the function.

Signed-off-by Dan Carpenter <[email protected]>
fs/btrfs/inode.c +1 -3
index fa6ccc1..0bc29be 100644
@@ -6487,10 +6487,8 @@ static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 	 * make sure the inode gets flushed if it is replacing
 	 * something.
 	 */
-	if (new_inode && new_inode->i_size &&
-	    old_inode && S_ISREG(old_inode->i_mode)) {
+	if (new_inode && new_inode->i_size && S_ISREG(old_inode->i_mode))
 		btrfs_add_ordered_operation(trans, root, old_inode);
-	}
 
 	old_dir->i_ctime = old_dir->i_mtime = ctime;
 	new_dir->i_ctime = new_dir->i_mtime = ctime;
#1 lore raw
Quoted 10 lines · Dan Carpenter

"old_inode" cannot be null here, because we dereference it unconditionally throughout the function.

Signed-off-by Dan Carpenter <[email protected]>
fs/btrfs/inode.c +0 -0
index fa6ccc1..0bc29be 100644

fs/btrfs/inode.c static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry, · line 6493

Quoted 8 lines
fs/btrfs/inode.c +1 -3
@@ -6487,10 +6487,8 @@ static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry,
         * make sure the inode gets flushed if it is replacing
         * something.
         */
-       if (new_inode && new_inode->i_size &&
-           old_inode && S_ISREG(old_inode->i_mode)) {
+       if (new_inode && new_inode->i_size && S_ISREG(old_inode->i_mode))
                btrfs_add_ordered_operation(trans, root, old_inode);
-       }

I think code like this is here because there are still a lot of features that are being added to btrfs and it's easier to have the additional checks than continually adding and removing them as the code changes.

#2 lore raw
Quoted 4 lines · Mike Fedyk

I think code like this is here because there are still a lot of features that are being added to btrfs and it's easier to have the additional checks than continually adding and removing them as the code changes.

_What_ features? Check for ->rename() argument not being negative is done outside of btrfs.

author
#3 lore raw

Re: [patch 5/11] btrfs: remove unneeded null check in

In reply to Mike Fedyk
Quoted 16 lines · Mike Fedyk
Quoted 15 lines · Dan Carpenter

"old_inode" cannot be null here, because we dereference it unconditionally throughout the function.

Signed-off-by Dan Carpenter <[email protected]>
fs/btrfs/inode.c +0 -2
index fa6ccc1..0bc29be 100644
@@ -6487,10 +6487,8 @@ static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry,
         * make sure the inode gets flushed if it is replacing
         * something.
         */
-       if (new_inode && new_inode->i_size &&
-           old_inode && S_ISREG(old_inode->i_mode)) {
Quoted 8 lines

fs/btrfs/inode.c static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry, · line 6493

Quoted 3 lines
fs/btrfs/inode.c +1 -1
+       if (new_inode && new_inode->i_size && S_ISREG(old_inode->i_mode))
                btrfs_add_ordered_operation(trans, root, old_inode);
-       }

I think code like this is here because there are still a lot of features that are being added to btrfs and it's easier to have the additional checks than continually adding and removing them as the code changes.

Right right. I understand about extra checks and api changes. But in this case that doesn't aply. There is no way this particular check helps now or in the future, and it just wastes time when someone is auditing for inconsistent null checking.

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html