2010-02-04 16:05:40

by Roel Kluin

Subject: [PATCH] ext4: ext4_xattr_ibody_set() error ignored in ext4_expand_extra_isize_ea()

The error handling was missing.

Signed-off-by: Roel Kluin <[email protected]>
---
Is this needed?

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index f3a2f7e..b77930c 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1302,6 +1302,8 @@ retry:

/* Remove the chosen entry from the inode */
error = ext4_xattr_ibody_set(handle, inode, &i, is);
+ if (error)
+ goto cleanup;

entry = IFIRST(header);
if (entry_size + EXT4_XATTR_SIZE(size) >= new_extra_isize)

2010-02-15 19:28:52

by Theodore Ts'o

Subject: Re: [PATCH] ext4: ext4_xattr_ibody_set() error ignored in ext4_expand_extra_isize_ea()

On Thu, Feb 04, 2010 at 05:11:27PM +0100, Roel Kluin wrote:
> The error handling was missing.
>
> Signed-off-by: Roel Kluin <[email protected]>
> ---
> Is this needed?

Yep, it's added, thanks. I'm not entirely right the error handling is
completely correct in this function; in particular, if we fail after
this point in the function, and we've already changed the status of
EXT4_STATE_XATTR flag, but then we fail due to some error (maybe an
I/O error), it's not clear the cleanup section is properly unwinding
all of the side effects made so far in this function.

Still, adding the error check clearly makes the code better than it
was before, so I've added it to the ext4 patch queue.

- Ted