On Tue, Jul 11, 2023 at 10:37:38PM -0700, Darrick J. Wong wrote:
> Here's my version, where I go for a straight a[1] -> a[] conversion and
> let downstream attrlist ioctl users eat their lumps. What do you think
> of the excess-documentation approach?
I think this is roughtly the right thing, with one big caveat:
> + /* In Linux 6.5 this flex array was changed from list[1] to list[]. */
For all the format headers there's no need for these comments. We've
done this for various other structures that had the old one size arrays
and never bothered.
> diff --git a/libxfs/xfs_fs.h b/libxfs/xfs_fs.h
> index 9c60ebb3..8927ecb2 100644
> --- a/libxfs/xfs_fs.h
> +++ b/libxfs/xfs_fs.h
> @@ -588,16 +588,19 @@ typedef struct xfs_attrlist_cursor {
> *
> * NOTE: struct xfs_attrlist must match struct attrlist defined in libattr, and
> * struct xfs_attrlist_ent must match struct attrlist_ent defined in libattr.
> + *
> + * WARNING: In Linux 6.5, al_offset and a_name were changed from array[1] to
> + * array[]. Anyone using sizeof is (already) broken!
> */
> struct xfs_attrlist {
> __s32 al_count; /* number of entries in attrlist */
> __s32 al_more; /* T/F: more attrs (do call again) */
> - __s32 al_offset[1]; /* byte offsets of attrs [var-sized] */
> + __s32 al_offset[]; /* byte offsets of attrs [var-sized] */
> };
>
> struct xfs_attrlist_ent { /* data from attr_list() */
> __u32 a_valuelen; /* number bytes in value of attr */
> - char a_name[1]; /* attr name (NULL terminated) */
> + char a_name[]; /* attr name (NULL terminated) */
> };
Now these are currently exposed in a xfsprogs headeer and IFF someone
is using them it would break them in nasty ways. That being said,
xfsprogs itself doesn't use them as it relies on identically layed
out but differntly named structures from libattr.
So I think we should just move these two out of xfs_fs.h, because
while they document a UAPI, they aren't actually used by userspace.
With that the need for any caution goes away and we can just fix the
definition without any caveats.
