[PATCH 0/2][next] firewire: Avoid -Wflex-array-member-not-at-end warnings

[PATCH 0/2][next] firewire: Avoid -Wflex-array-member-not-at-end warnings

[Gustavo A. R. Silva]

-Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
ready to enable it globally. So, we are deprecating flexible-array
members in the middle of another struct.

There are currently a couple of local structures (`u` and `template`)
that are using a flexible `struct fw_iso_packet` as header for a couple
of on-stack arrays.

We make use of the `struct_group_tagged()` helper to separate the
flexible array from the rest of the members in the flexible structure,
and, with this, we can now declare objects of the type of the tagged
struct, without embedding the flexible array in the middle of another
struct.

We also use `container_of()` whenever we need to retrieve a pointer to
the flexible structure, through which the flexible-array member can be
accessed.

With these changes, we fix a couple of -Wflex-array-member-not-at-end
warnings.

Gustavo A. R. Silva (2):
  firewire: Avoid -Wflex-array-member-not-at-end warning
  ALSA: firewire-lib: Avoid -Wflex-array-member-not-at-end warning

 drivers/firewire/core-cdev.c  |  9 +++++----
 include/linux/firewire.h      | 16 +++++++++-------
 sound/firewire/amdtp-stream.c |  8 +++++---
 3 files changed, 19 insertions(+), 14 deletions(-)

-- 
2.34.1

[PATCH 1/2][next] firewire: Avoid -Wflex-array-member-not-at-end warning

[Gustavo A. R. Silva]

-Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
ready to enable it globally.

There is currently a local structure `u` that is using a flexible
`struct fw_iso_packet` as header for an on-stack array `u8 header[256]`.

struct {
	struct fw_iso_packet packet;
	u8 header[256];
} u;

However, we are deprecating flexible arrays in the middle of another
struct. So, in order to avoid this, we use the `struct_group_tagged()`
helper to separate the flexible array from the rest of the members in
the flexible structure:

struct fw_iso_packet {
        struct_group_tagged(fw_iso_packet_hdr, hdr,
		... the rest of the members
        );
        u32 header[];           /* tx: Top of 1394 isoch. data_block    */
};

With the change described above, we can now declare an object of the
type of the tagged struct, without embedding the flexible array in the
middle of another struct:

struct {
        struct fw_iso_packet_hdr packet;
        u8 header[256];
} u;

We also use `container_of()` whenever we need to retrieve a pointer to
the flexible structure, through which the flexible-array member can be
accessed, as in this case.

So, with these changes, fix the following warning:

drivers/firewire/core-cdev.c: In function ‘ioctl_queue_iso’:
drivers/firewire/core-cdev.c:1129:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
 1129 |                 struct fw_iso_packet packet;
      |                                      ^~~~~~

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 drivers/firewire/core-cdev.c |  9 +++++----
 include/linux/firewire.h     | 16 +++++++++-------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 6274b86eb943..e1f1daa2e667 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1126,9 +1126,11 @@ static int ioctl_queue_iso(struct client *client, union ioctl_arg *arg)
 	u32 control;
 	int count;
 	struct {
-		struct fw_iso_packet packet;
+		struct fw_iso_packet_hdr packet;
 		u8 header[256];
 	} u;
+	struct fw_iso_packet *packet =
+			container_of(&u.packet, struct fw_iso_packet, hdr);
 
 	if (ctx == NULL || a->handle != 0)
 		return -EINVAL;
@@ -1192,7 +1194,7 @@ static int ioctl_queue_iso(struct client *client, union ioctl_arg *arg)
 		if (next > end)
 			return -EINVAL;
 		if (copy_from_user
-		    (u.packet.header, p->header, transmit_header_bytes))
+		    (packet->header, p->header, transmit_header_bytes))
 			return -EFAULT;
 		if (u.packet.skip && ctx->type == FW_ISO_CONTEXT_TRANSMIT &&
 		    u.packet.header_length + u.packet.payload_length > 0)
@@ -1200,8 +1202,7 @@ static int ioctl_queue_iso(struct client *client, union ioctl_arg *arg)
 		if (payload + u.packet.payload_length > buffer_end)
 			return -EINVAL;
 
-		if (fw_iso_context_queue(ctx, &u.packet,
-					 &client->buffer, payload))
+		if (fw_iso_context_queue(ctx, packet, &client->buffer, payload))
 			break;
 
 		p = next;
diff --git a/include/linux/firewire.h b/include/linux/firewire.h
index dd9f2d765e68..becd3a60d0fb 100644
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -456,13 +456,15 @@ void fw_core_remove_descriptor(struct fw_descriptor *desc);
  * scatter-gather streaming (e.g. assembling video frame automatically).
  */
 struct fw_iso_packet {
-	u16 payload_length;	/* Length of indirect payload		*/
-	u32 interrupt:1;	/* Generate interrupt on this packet	*/
-	u32 skip:1;		/* tx: Set to not send packet at all	*/
-				/* rx: Sync bit, wait for matching sy	*/
-	u32 tag:2;		/* tx: Tag in packet header		*/
-	u32 sy:4;		/* tx: Sy in packet header		*/
-	u32 header_length:8;	/* Length of immediate header		*/
+	struct_group_tagged(fw_iso_packet_hdr, hdr,
+		u16 payload_length;	/* Length of indirect payload		*/
+		u32 interrupt:1;	/* Generate interrupt on this packet	*/
+		u32 skip:1;		/* tx: Set to not send packet at all	*/
+		/* rx: Sync bit, wait for matching sy	*/
+		u32 tag:2;		/* tx: Tag in packet header		*/
+		u32 sy:4;		/* tx: Sy in packet header		*/
+		u32 header_length:8;	/* Length of immediate header		*/
+	);
 	u32 header[];		/* tx: Top of 1394 isoch. data_block	*/
 };
 
-- 
2.34.1

[PATCH 2/2][next] ALSA: firewire-lib: Avoid -Wflex-array-member-not-at-end warning

[Gustavo A. R. Silva]

-Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
ready to enable it globally.

There is currently a local structure `template` that is using a flexible
`struct fw_iso_packet` as a header for an on-stack array
`__be32 header[CIP_HEADER_QUADLETS];`.

struct {
	struct fw_iso_packet params;
	__be32 header[CIP_HEADER_QUADLETS];
} template = { {0}, {0} };

However, we are deprecating flexible arrays in the middle of another
struct. So, in order to avoid this, we use the `struct_group_tagged()`
helper to separate the flexible array from the rest of the members in
the flexible structure:

struct fw_iso_packet {
        struct_group_tagged(fw_iso_packet_hdr, hdr,
                ... the rest of the members
        );
        u32 header[];           /* tx: Top of 1394 isoch. data_block    */
};

With the change described above, we can now declare an object of the
type of the tagged struct, without embedding the flexible array in the
middle of another struct:

struct {
	struct fw_iso_packet_hdr params;
	__be32 header[CIP_HEADER_QUADLETS];
} template = { {0}, {0} };

We also use `container_of()` whenever we need to retrieve a pointer to
the flexible structure.

So, with these changes, fix the following warning:

sound/firewire/amdtp-stream.c: In function ‘process_rx_packets’:
sound/firewire/amdtp-stream.c:1184:46: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
 1184 |                         struct fw_iso_packet params;
      |

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 sound/firewire/amdtp-stream.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index c9f153f85ae6..7ba1cd64d7f1 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -1181,12 +1181,14 @@ static void process_rx_packets(struct fw_iso_context *context, u32 tstamp, size_
 
 	for (i = 0; i < packets; ++i) {
 		struct {
-			struct fw_iso_packet params;
+			struct fw_iso_packet_hdr params;
 			__be32 header[CIP_HEADER_QUADLETS];
 		} template = { {0}, {0} };
+		struct fw_iso_packet *params =
+			container_of(&template.params, struct fw_iso_packet, hdr);
 		bool sched_irq = false;
 
-		build_it_pkt_header(s, desc->cycle, &template.params, pkt_header_length,
+		build_it_pkt_header(s, desc->cycle, params, pkt_header_length,
 				    desc->data_blocks, desc->data_block_counter,
 				    desc->syt, i, curr_cycle_time);
 
@@ -1198,7 +1200,7 @@ static void process_rx_packets(struct fw_iso_context *context, u32 tstamp, size_
 			}
 		}
 
-		if (queue_out_packet(s, &template.params, sched_irq) < 0) {
+		if (queue_out_packet(s, params, sched_irq) < 0) {
 			cancel_stream(s);
 			return;
 		}
-- 
2.34.1

Re: [PATCH 0/2][next] firewire: Avoid -Wflex-array-member-not-at-end warnings

[Takashi Sakamoto]

Hi,

On Tue, Mar 05, 2024 at 11:24:15AM -0600, Gustavo A. R. Silva wrote:
> -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
> ready to enable it globally. So, we are deprecating flexible-array
> members in the middle of another struct.
> 
> There are currently a couple of local structures (`u` and `template`)
> that are using a flexible `struct fw_iso_packet` as header for a couple
> of on-stack arrays.
> 
> We make use of the `struct_group_tagged()` helper to separate the
> flexible array from the rest of the members in the flexible structure,
> and, with this, we can now declare objects of the type of the tagged
> struct, without embedding the flexible array in the middle of another
> struct.
> 
> We also use `container_of()` whenever we need to retrieve a pointer to
> the flexible structure, through which the flexible-array member can be
> accessed.
> 
> With these changes, we fix a couple of -Wflex-array-member-not-at-end
> warnings.
> 
> Gustavo A. R. Silva (2):
>   firewire: Avoid -Wflex-array-member-not-at-end warning
>   ALSA: firewire-lib: Avoid -Wflex-array-member-not-at-end warning
> 
>  drivers/firewire/core-cdev.c  |  9 +++++----
>  include/linux/firewire.h      | 16 +++++++++-------
>  sound/firewire/amdtp-stream.c |  8 +++++---
>  3 files changed, 19 insertions(+), 14 deletions(-)

Thanks for the improvements, however we are mostly at the end of
development period for v6.8 kernel. Let me postpone applying the patches
until closing the next merge window (for v6.9), since we need the term to
evaluate the change. I mean that it goes to v6.10 kernel.

If you would like me to applying the patch v6.9 kernel, please inform it
to us.


Thanks

Takashi Sakamoto

Re: [PATCH 0/2][next] firewire: Avoid -Wflex-array-member-not-at-end warnings

[Gustavo A. R. Silva]

> Thanks for the improvements, however we are mostly at the end of
> development period for v6.8 kernel. Let me postpone applying the patches
> until closing the next merge window (for v6.9), since we need the term to
> evaluate the change. I mean that it goes to v6.10 kernel.

Sure, no problem.

Actually, I'll send a v2 with DEFINE_FLEX(), instead.

Thanks
--
Gustavo

Re: [PATCH 0/2][next] firewire: Avoid -Wflex-array-member-not-at-end warnings

[Kees Cook]

On Wed, Mar 06, 2024 at 10:18:59AM -0600, Gustavo A. R. Silva wrote:
> 
> > Thanks for the improvements, however we are mostly at the end of
> > development period for v6.8 kernel. Let me postpone applying the patches
> > until closing the next merge window (for v6.9), since we need the term to
> > evaluate the change. I mean that it goes to v6.10 kernel.
> 
> Sure, no problem.
> 
> Actually, I'll send a v2 with DEFINE_FLEX(), instead.

I was just going through the patch tracker to make sure stuff got
handled -- did a v2 of these ever get posted? I don't see anything in
the tracker nor changes here in -next.

Thanks!

-Kees

-- 
Kees Cook

Re: [PATCH 0/2][next] firewire: Avoid -Wflex-array-member-not-at-end warnings

[Gustavo A. R. Silva]

On 4/29/24 12:30, Kees Cook wrote:
> On Wed, Mar 06, 2024 at 10:18:59AM -0600, Gustavo A. R. Silva wrote:
>>
>>> Thanks for the improvements, however we are mostly at the end of
>>> development period for v6.8 kernel. Let me postpone applying the patches
>>> until closing the next merge window (for v6.9), since we need the term to
>>> evaluate the change. I mean that it goes to v6.10 kernel.
>>
>> Sure, no problem.
>>
>> Actually, I'll send a v2 with DEFINE_FLEX(), instead.
> 
> I was just going through the patch tracker to make sure stuff got
> handled -- did a v2 of these ever get posted? I don't see anything in
> the tracker nor changes here in -next.

Yes, it's in linux-next already:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=1d717123bb1a7555a432e51a41709badf8545dba

--
Gustavo

Re: [PATCH 0/2][next] firewire: Avoid -Wflex-array-member-not-at-end warnings

[Kees Cook]

On Mon, Apr 29, 2024 at 12:42:23PM -0600, Gustavo A. R. Silva wrote:
> 
> 
> On 4/29/24 12:30, Kees Cook wrote:
> > On Wed, Mar 06, 2024 at 10:18:59AM -0600, Gustavo A. R. Silva wrote:
> > > 
> > > > Thanks for the improvements, however we are mostly at the end of
> > > > development period for v6.8 kernel. Let me postpone applying the patches
> > > > until closing the next merge window (for v6.9), since we need the term to
> > > > evaluate the change. I mean that it goes to v6.10 kernel.
> > > 
> > > Sure, no problem.
> > > 
> > > Actually, I'll send a v2 with DEFINE_FLEX(), instead.
> > 
> > I was just going through the patch tracker to make sure stuff got
> > handled -- did a v2 of these ever get posted? I don't see anything in
> > the tracker nor changes here in -next.
> 
> Yes, it's in linux-next already:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=1d717123bb1a7555a432e51a41709badf8545dba

Ah-ha! Thank you! :)

-- 
Kees Cook