On Sun, Jun 28, 2015 at 10:47 PM, Davide C. C. Italiano <dccitaliano@xxxxxxxxx> wrote: > From: Davide Italiano <dccitaliano@xxxxxxxxx> > > btrfs_end_transaction() can return an error -- this happens, e.g. > if it tries to commit and the transaction was aborted in the meanhwile. > Swallowing the error is wrong, so explicitly return it. > > Signed-off-by: Davide Italiano <dccitaliano@xxxxxxxxx> > --- > fs/btrfs/inode.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 59c475c..7764132 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -9199,7 +9199,7 @@ static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry, > btrfs_end_log_trans(root); > } > out_fail: > - btrfs_end_transaction(trans, root); > + ret = btrfs_end_transaction(trans, root); Hi, Good intention but it's now swallowing errors from earlier places in the code that jump to the out_fail label. For e.g. if the call to btrfs_set_inode_index() fails, we jump to out_fail and we lose the error value that it returned (btrfs_end_transaction() returns 0 for e.g.), so userspace thinks everything succeed when it didn't. Correct fix is to set ret to the return value of btrfs_end_transaction() only if ret is currently zero. thanks > out_notrans: > if (old_ino == BTRFS_FIRST_FREE_OBJECTID) > up_read(&root->fs_info->subvol_sem); > -- > 2.4.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That's why all progress depends on unreasonable men." -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
