Re: [PATCH v6 6/6] fs: introduce file_getattr and file_setattr syscalls

Amir Goldstein
2025-07-02T09:14:01Z

> > +/*
> > + * Variable size structure for file_[sg]et_attr().
> > + *
> > + * Note. This is alternative to the structure 'struct fileattr'/'struct fsxattr'.
> > + * As this structure is passed to/from userspace with its size, this can
> > + * be versioned based on the size.
> > + */
> > +struct fsx_fileattr {
> > + __u32 fsx_xflags; /* xflags field value (get/set) */
> > + __u32 fsx_extsize; /* extsize field value (get/set)*/
> > + __u32 fsx_nextents; /* nextents field value (get) */
> > + __u32 fsx_projid; /* project identifier (get/set) */
> > + __u32 fsx_cowextsize; /* CoW extsize field value (get/set) */
>
> This misses a:
>
> __u32 __spare;
>
> so there's no holes in the struct. :)

Adding __spare and not verifying that it is zeroed gets us to the
point that we are not able to replace __spare with a real field later.

I suggest to resolve this hole as Darrick and Pali suggested by making it
__u64 fsx_xflags

w.r.t Darrick's comment, I kind of like it that the name for the UAPI
struct (fsxattr)
differs from the name of the kernel internal representation (fileattr), but
I agree that fsx_fileattr does not give a good hint on what it is.

I think that renaming struct fsx_fileattr to struct fsxattr64 along
with changing the
width of fsx_xflags will help reduce the confusion of users.

What do you guys think?

Thanks,
Amir.