Re: [PATCH] ext4: Use struct_size() to improve ext4_htree_store_dirent()

[Greg KH <gregkh@linuxfoundation.org>]

On Tue, Nov 05, 2024 at 12:06:42PM +0100, Thorsten Blum wrote:
> On 5. Nov 2024, at 11:39, Greg KH wrote:
> > On Tue, Nov 05, 2024 at 11:33:54AM +0100, Thorsten Blum wrote:
> >> Inline and use struct_size() to calculate the number of bytes to
> >> allocate for new_fn and remove the local variable len.
> >> 
> >> Reviewed-by: Jan Kara <jack@suse.cz>
> >> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> >> ---
> >> This change was originally part of another patch that was split into two
> >> separate patches after feedback from Greg KH
> >> - Link: https://lore.kernel.org/r/20241104234214.8094-2-thorsten.blum@linux.dev/
> >> ---
> >> fs/ext4/dir.c | 5 ++---
> >> 1 file changed, 2 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> >> index 233479647f1b..02d47a64e8d1 100644
> >> --- a/fs/ext4/dir.c
> >> +++ b/fs/ext4/dir.c
> >> @@ -471,14 +471,13 @@ int ext4_htree_store_dirent(struct file *dir_file, __u32 hash,
> >> struct rb_node **p, *parent = NULL;
> >> struct fname *fname, *new_fn;
> >> struct dir_private_info *info;
> >> - int len;
> >> 
> >> info = dir_file->private_data;
> >> p = &info->root.rb_node;
> >> 
> >> /* Create and allocate the fname structure */
> >> - len = sizeof(struct fname) + ent_name->len + 1;
> >> - new_fn = kzalloc(len, GFP_KERNEL);
> >> + new_fn = kzalloc(struct_size(new_fn, name, ent_name->len + 1),
> >> + GFP_KERNEL);
> > 
> > Does this actually matter and make the code any more robust or faster?
> > 
> > The original code here is easier to read and understand, why add
> > complexity if it is not required?
> 
> I find struct_size() to be more readable because it explicitly
> communicates the relationship between the flexible array member name and
> ent_name->len that the open-coded version doesn't. Plus, struct_size()
> has some additional compile-time checks (e.g., __must_be_array()).

Ah, missed that, sure, that makes more sense, sorry for the noise.

greg k-h