Re: [PATCH][next] UAPI: ndctl / acpi: intel: Avoid multiple -Wflex-array-member-not-at-end warnings

[Kees Cook <kees@kernel.org>]

On Thu, Feb 13, 2025 at 02:25:27PM -0800, Alison Schofield wrote:
> On Thu, Feb 13, 2025 at 03:29:12PM +1030, Gustavo A. R. Silva wrote:
> > -Wflex-array-member-not-at-end was introduced in GCC-14, and we are
> > getting ready to enable it, globally.
> > 
> > So, in order to avoid ending up with flexible-array members in the
> > middle of other structs, we use the `__struct_group()` helper to
> > separate the flexible array from the rest of the members in the
> > flexible structure. We then use the newly created tagged `struct
> > nd_cmd_pkg_hdr` to replace the type of the objects causing trouble
> > (`pkg`) in multiple structs.
> > 
> > So, with these changes, fix the following warnings:
> > 
> > drivers/acpi/nfit/intel.c:692:35: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> 
> One sample warning is good.
> 
> How about adding a comment on why the usage of __struct_group() here
> means this doesn't break userspace.
> 
> snip
> 
> > diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
> > index 73516e263627..34c11644d5d7 100644
> > --- a/include/uapi/linux/ndctl.h
> > +++ b/include/uapi/linux/ndctl.h
> 
> FWIW: In a patch like this where reviewers likely want to see
> the header file change first, put it first in the diff.
> (git diff.orderfile)

TIL about diff.orderfile! :) Just for my own education, what do you have
as the contents for your orderfile? Is it just simply:

*.h

?

> 
> 
> > @@ -227,12 +227,15 @@ enum ars_masks {
> >   */
> >  
> >  struct nd_cmd_pkg {
> > -	__u64   nd_family;		/* family of commands */
> > -	__u64   nd_command;
> > -	__u32   nd_size_in;		/* INPUT: size of input args */
> > -	__u32   nd_size_out;		/* INPUT: size of payload */
> > -	__u32   nd_reserved2[9];	/* reserved must be zero */
> > -	__u32   nd_fw_size;		/* OUTPUT: size fw wants to return */
> > +	/* New members MUST be added within the __struct_group() macro below. */
> > +	__struct_group(nd_cmd_pkg_hdr, __hdr, /* no attrs */,
> > +		__u64   nd_family;		/* family of commands */
> > +		__u64   nd_command;
> > +		__u32   nd_size_in;		/* INPUT: size of input args */
> > +		__u32   nd_size_out;		/* INPUT: size of payload */
> > +		__u32   nd_reserved2[9];	/* reserved must be zero */
> > +		__u32   nd_fw_size;		/* OUTPUT: size fw wants to return */
> > +	);
> >  	unsigned char nd_payload[];	/* Contents of call      */
> >  };

Gustavo, any updates on this patch? I'm skimming through patchwork to
check on stalled patches...

-- 
Kees Cook