| Message ID | Z9AA9tAbcIcz6BMO@kspp (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [next] firewire: core: avoid -Wflex-array-member-not-at-end warning | expand |
Hi, On Tue, Mar 11, 2025 at 07:53:02PM +1030, Gustavo A. R. Silva wrote: > Use the `DEFINE_RAW_FLEX()` helper for an on-stack definition of > a flexible structure where the size of the flexible-array member > is known at compile-time, and refactor the rest of the code, > accordingly. > > So, with these changes, fix the following warning: > > drivers/firewire/core-cdev.c:1141:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > --- > drivers/firewire/core-cdev.c | 42 ++++++++++++++++-------------------- > 1 file changed, 19 insertions(+), 23 deletions(-) Thanks for the improvement, however I think it includes a slight concern about the calculation for the length of array. > diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c > index b360dca2c69e..706b9037faac 100644 > --- a/drivers/firewire/core-cdev.c > +++ b/drivers/firewire/core-cdev.c > @@ -1137,10 +1137,7 @@ static int ioctl_queue_iso(struct client *client, union ioctl_arg *arg) > unsigned long payload, buffer_end, transmit_header_bytes = 0; > u32 control; > int count; > - struct { > - struct fw_iso_packet packet; > - u8 header[256]; > - } u; > + DEFINE_RAW_FLEX(struct fw_iso_packet, u, header, 8); The definition of 'struct fw_iso_packet' is in 'include/linux/firewire.h': ``` $ cat include/linux/firewire.h ... 460 struct fw_iso_packet { 461 u16 payload_length; /* Length of indirect payload */ 462 u32 interrupt:1; /* Generate interrupt on this packet */ 463 u32 skip:1; /* tx: Set to not send packet at all */ 464 /* rx: Sync bit, wait for matching sy */ 465 u32 tag:2; /* tx: Tag in packet header */ 466 u32 sy:4; /* tx: Sy in packet header */ 467 u32 header_length:8; /* Size of immediate header */ 468 u32 header[]; /* tx: Top of 1394 isoch. data_block */ 469 }; ``` The size of element of 'header' array is 4 byte (= 32 / 8). The original code keeps 256 bytes storage following to the structure. Therefore the 'COUNT' argument of DEFINE_RAW_FLEX() macro should be 64 (= 256 / 4). Although the header field is not rarely used by userspace applications actually and the reduction of kernel stack usage is preferable itself, it is preferable to keep the compatibility. Thanks Takashi Sakamoto
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index b360dca2c69e..706b9037faac 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -1137,10 +1137,7 @@ static int ioctl_queue_iso(struct client *client, union ioctl_arg *arg) unsigned long payload, buffer_end, transmit_header_bytes = 0; u32 control; int count; - struct { - struct fw_iso_packet packet; - u8 header[256]; - } u; + DEFINE_RAW_FLEX(struct fw_iso_packet, u, header, 8); if (ctx == NULL || a->handle != 0) return -EINVAL; @@ -1172,29 +1169,29 @@ static int ioctl_queue_iso(struct client *client, union ioctl_arg *arg) while (p < end) { if (get_user(control, &p->control)) return -EFAULT; - u.packet.payload_length = GET_PAYLOAD_LENGTH(control); - u.packet.interrupt = GET_INTERRUPT(control); - u.packet.skip = GET_SKIP(control); - u.packet.tag = GET_TAG(control); - u.packet.sy = GET_SY(control); - u.packet.header_length = GET_HEADER_LENGTH(control); + u->payload_length = GET_PAYLOAD_LENGTH(control); + u->interrupt = GET_INTERRUPT(control); + u->skip = GET_SKIP(control); + u->tag = GET_TAG(control); + u->sy = GET_SY(control); + u->header_length = GET_HEADER_LENGTH(control); switch (ctx->type) { case FW_ISO_CONTEXT_TRANSMIT: - if (u.packet.header_length & 3) + if (u->header_length & 3) return -EINVAL; - transmit_header_bytes = u.packet.header_length; + transmit_header_bytes = u->header_length; break; case FW_ISO_CONTEXT_RECEIVE: - if (u.packet.header_length == 0 || - u.packet.header_length % ctx->header_size != 0) + if (u->header_length == 0 || + u->header_length % ctx->header_size != 0) return -EINVAL; break; case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL: - if (u.packet.payload_length == 0 || - u.packet.payload_length & 3) + if (u->payload_length == 0 || + u->payload_length & 3) return -EINVAL; break; } @@ -1204,20 +1201,19 @@ 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)) + (u->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) + if (u->skip && ctx->type == FW_ISO_CONTEXT_TRANSMIT && + u->header_length + u->payload_length > 0) return -EINVAL; - if (payload + u.packet.payload_length > buffer_end) + if (payload + u->payload_length > buffer_end) return -EINVAL; - if (fw_iso_context_queue(ctx, &u.packet, - &client->buffer, payload)) + if (fw_iso_context_queue(ctx, u, &client->buffer, payload)) break; p = next; - payload += u.packet.payload_length; + payload += u->payload_length; count++; } fw_iso_context_queue_flush(ctx);
Use the `DEFINE_RAW_FLEX()` helper for an on-stack definition of a flexible structure where the size of the flexible-array member is known at compile-time, and refactor the rest of the code, accordingly. So, with these changes, fix the following warning: drivers/firewire/core-cdev.c:1141:38: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- drivers/firewire/core-cdev.c | 42 ++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 23 deletions(-)