hi??
there may be a bug in ext4_read_inline_dir() function with inline_data feature.
when I recursion call a sys_old_readdir to read a direntry's
sub_dentry,I find that the ext4_read_inline_dir return the len of the
inline_data ,but in the no inline_data case,it will return 0.It is
inconsistent.
So I try to make a patch to fix it.
what's ur point adding "ret = 0;" after "goto out;"
maybe before "goto"?
Best,
On Mon, Oct 21, 2013 at 12:33 PM, Boxi Liu <[email protected]> wrote:
> hi，
> there may be a bug in ext4_read_inline_dir() function with inline_data feature.
> when I recursion call a sys_old_readdir to read a direntry's
> sub_dentry,I find that the ext4_read_inline_dir return the len of the
> inline_data ,but in the no inline_data case,it will return 0.It is
> inconsistent.
> So I try to make a patch to fix it.
Hi Boxi,
On 10/22/2013 12:33 AM, Boxi Liu wrote:
> hi??
> there may be a bug in ext4_read_inline_dir() function with inline_data feature.
> when I recursion call a sys_old_readdir to read a direntry's
> sub_dentry,I find that the ext4_read_inline_dir return the len of the
> inline_data ,but in the no inline_data case,it will return 0.It is
> inconsistent.
> So I try to make a patch to fix it.
The fix looks good to me an you can add
Acked-by: Tao Ma <[email protected]>
But next time please generate the patch with git format-patch and send
it using git send-email.
Thanks,
Tao
Applied, thanks.
On Tue, Oct 22, 2013 at 10:10:27AM +0800, Tao Ma wrote:
> But next time please generate the patch with git format-patch and send
> it using git send-email.
Yes, please. :-)
I've reworded the commit description a little.
In ext4_read_inline_dir(), if there is inline data, the successful
return value is the return value of ext4_read_inline_data(). Howewer,
this is used by ext4_readdir(), and while it seems harmless to return
a positive value on success, it's inconsistent, since historically
we've always return 0 on success.
BTW, it looks like it would simplify the code a bit to change
ext4_read_inline_page() and ext4_read_line_data() to return 0 on
success, since it looks like the positive value on success isn't
needed anywhere, and this would allow us to remove lines such as this:
return ret >= 0 ? 0 : ret;
as well as the forced ret = 0; in this commit.
Could you take a look and verify that this is true, and I haven't
missed anything?
Also, it would be nice if the functions in fs/ext4/inline.c documented
what the return values are for each of the functions, one way or
another.
Thanks!!
- Ted
On 10/30/2013 08:13 PM, Theodore Ts'o wrote:
> Applied, thanks.
>
> On Tue, Oct 22, 2013 at 10:10:27AM +0800, Tao Ma wrote:
>> But next time please generate the patch with git format-patch and send
>> it using git send-email.
>
> Yes, please. :-)
>
> I've reworded the commit description a little.
>
> In ext4_read_inline_dir(), if there is inline data, the successful
> return value is the return value of ext4_read_inline_data(). Howewer,
> this is used by ext4_readdir(), and while it seems harmless to return
> a positive value on success, it's inconsistent, since historically
> we've always return 0 on success.
>
>
> BTW, it looks like it would simplify the code a bit to change
> ext4_read_inline_page() and ext4_read_line_data() to return 0 on
> success, since it looks like the positive value on success isn't
> needed anywhere, and this would allow us to remove lines such as this:
>
> return ret >= 0 ? 0 : ret;
>
> as well as the forced ret = 0; in this commit.
>
> Could you take a look and verify that this is true, and I haven't
> missed anything?
sure, I will look into it.
>
> Also, it would be nice if the functions in fs/ext4/inline.c documented
> what the return values are for each of the functions, one way or
> another.
OK, I will try to generate a patch for it.
Thanks,
Tao
>
> Thanks!!
>
> - Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>