[PATCH 0/2][next] firewire: Avoid -Wflex-array-member-not-at-end warnings
-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(-)
-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 */ };
-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; }
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
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
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
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/?...
-- Gustavo
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/?...
Ah-ha! Thank you! :)
participants (4)
-
Gustavo A. R. Silva
-
Gustavo A. R. Silva
-
Kees Cook
-
Takashi Sakamoto