> > static int ext4_fill_fiemap_extents(struct inode *inode,
> > ext4_lblk_t block, ext4_lblk_t num,
> > - struct fiemap_extent_info *fieinfo)
> > + struct fiemap_ctx *f_ctx)
> > {
> > struct ext4_ext_path *path = NULL;
> > struct ext4_extent *ex;
> > @@ -2277,7 +2277,8 @@ static int ext4_fill_fiemap_extents(struct inode *inode,
> > }
> >
> > if (exists) {
> > - err = fiemap_fill_next_extent(fieinfo,
> > + err = fiemap_fill_next_extent(
> > + (struct fiemap_extent_info *)f_ctx->fc_data,
>
> No need to cast void pointer. This also makes me wonder why you didn't name
> fc_data as fc_extent_info instead of making it a void pointer? I didn't see
> any users where it isn't a pointer to struct fiemap_extent_info?
On later patches, the void pointer is used to point to a userspace address or to
a kernel address, depending on the context. (userspace if FIEMAP, kernelspace if
FIBMAP).
>
> The same is true of all the other filesystem-specific patches - there is no
> need to cast from a void *. Is there a later user where this is used as a
> generic data pointer, or is it always going to be a fiemap_extent_info, and
> later a fiemap_ctx structure?
I believe I made the mistake to not quote the previous thread which led to this
patch, so, quoting it now:
https://www.spinics.net/lists/linux-fsdevel/msg131786.html
More specifically, I think the detailed information about the approach, was
discussed in the 2nd patch of the above RFC series:
https://www.spinics.net/lists/linux-fsdevel/msg131788.html
>
> > @@ -5040,14 +5041,14 @@ static int ext4_xattr_fiemap(struct inode *inode,
> > }
> >
> > if (physical)
> > - error = fiemap_fill_next_extent(fieinfo, 0, physical,
> > - length, flags);
> > + error = fiemap_fill_next_extent(
> > + (struct fiemap_extent_info *)f_ctx->fc_data,
>
> Same...
>
About the casts, I didn't remember if GCC allowed implicit casting when passing
it into functions (I remember variable assignment implicit cast though), so,
better safe than sorry, but I can remove it without problems in follow-up
patches.
Thanks
> > diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> > index 9c4bac18cc6c..7b9b0da60d54 100644
> > --- a/fs/ext4/inline.c
> > +++ b/fs/ext4/inline.c
> > @@ -1888,8 +1888,9 @@ int ext4_inline_data_fiemap(struct inode *inode,
> > physical += offsetof(struct ext4_inode, i_block);
> >
> > if (physical)
> > - error = fiemap_fill_next_extent(fieinfo, start, physical,
> > - inline_len, flags);
> > + error = fiemap_fill_next_extent(
> > + (struct fiemap_extent_info *)f_ctx->fc_data,
>
> Same...
>
>
> Cheers, Andreas
>
>
>
>
>
--
Carlos
