| Message ID | Z851qvkycepdNlBd@kspp (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] rtc: Avoid a couple of -Wflex-array-member-not-at-end warnings | expand |
On Mon, Mar 10, 2025 at 03:46:26PM +1030, Gustavo A. R. Silva wrote: > Hi all, > > As part of the efforts to globally enable -Wflex-array-member-not-at-end, > I'm currently trying to fix the following warnings: > > drivers/rtc/rtc-cros-ec.c:62:40: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] > drivers/rtc/rtc-cros-ec.c:40:40: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end] > > The issue is that `struct cros_ec_command` is a flexible structure (which > means that it contains a flexible-array member), and there is an object > of this type (msg) declared within another structure but at the end. ^ not > It seems that the following patch would suffice, as long as the flex-array > member in `struct cros_ec_command` is not expected to be accessed and > overlap with `struct ec_response_rtc data` in a "controlled manner": > > diff --git a/drivers/rtc/rtc-cros-ec.c b/drivers/rtc/rtc-cros-ec.c > index 865c2e82c7a5..7e9bbab47e4c 100644 > --- a/drivers/rtc/rtc-cros-ec.c > +++ b/drivers/rtc/rtc-cros-ec.c > @@ -37,8 +37,8 @@ static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command, > { > int ret; > struct { > - struct cros_ec_command msg; > struct ec_response_rtc data; > + struct cros_ec_command msg; > } __packed msg; > > memset(&msg, 0, sizeof(msg)); > @@ -59,8 +59,8 @@ static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command, > { > int ret; > struct { > - struct cros_ec_command msg; > struct ec_response_rtc data; > + struct cros_ec_command msg; > } __packed msg; > > memset(&msg, 0, sizeof(msg)); This doesn't work. The header (i.e. struct cros_ec_command) is expected to allocate right before the payload (e.g. struct ec_response_rtc)[1][2]. [1]: https://elixir.bootlin.com/linux/v6.14-rc1/source/drivers/platform/chrome/cros_ec_proto.c#L82 [2]: https://elixir.bootlin.com/linux/v6.14-rc1/source/drivers/platform/chrome/cros_ec_i2c.c#L166 > Otherwise, we probably need to use struct_group_tagged() as follows: I'm not a big fan of the solution. How about using something similar to: struct cros_ec_command *msg = kzalloc(sizeof(*msg) + sizeof(struct ec_response_rtc), ...);
diff --git a/drivers/rtc/rtc-cros-ec.c b/drivers/rtc/rtc-cros-ec.c index 865c2e82c7a5..7e9bbab47e4c 100644 --- a/drivers/rtc/rtc-cros-ec.c +++ b/drivers/rtc/rtc-cros-ec.c @@ -37,8 +37,8 @@ static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command, { int ret; struct { - struct cros_ec_command msg; struct ec_response_rtc data; + struct cros_ec_command msg; } __packed msg; memset(&msg, 0, sizeof(msg)); @@ -59,8 +59,8 @@ static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command, { int ret; struct { - struct cros_ec_command msg; struct ec_response_rtc data; + struct cros_ec_command msg; } __packed msg; memset(&msg, 0, sizeof(msg)); Otherwise, we probably need to use struct_group_tagged() as follows: diff --git a/drivers/rtc/rtc-cros-ec.c b/drivers/rtc/rtc-cros-ec.c index 865c2e82c7a5..6dc815bdbcd9 100644 --- a/drivers/rtc/rtc-cros-ec.c +++ b/drivers/rtc/rtc-cros-ec.c @@ -37,7 +37,7 @@ static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command, { int ret; struct { - struct cros_ec_command msg; + struct cros_ec_command_hdr msg; struct ec_response_rtc data; } __packed msg; @@ -45,7 +45,10 @@ static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command, msg.msg.command = command; msg.msg.insize = sizeof(msg.data); - ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg); + ret = cros_ec_cmd_xfer_status(cros_ec, + container_of(&msg.msg, + struct cros_ec_command, + __hdr)); if (ret < 0) return ret; @@ -59,7 +62,7 @@ static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command, { int ret; struct { - struct cros_ec_command msg; + struct cros_ec_command_hdr msg; struct ec_response_rtc data; } __packed msg; @@ -68,7 +71,10 @@ static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command, msg.msg.outsize = sizeof(msg.data); msg.data.time = param; - ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg); + ret = cros_ec_cmd_xfer_status(cros_ec, + container_of(&msg.msg, + struct cros_ec_command, + __hdr)); if (ret < 0) return ret; return 0; diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h index 3ec24f445c29..2a638c8c5ec2 100644 --- a/include/linux/platform_data/cros_ec_proto.h +++ b/include/linux/platform_data/cros_ec_proto.h @@ -80,11 +80,13 @@ enum { * @data: Where to put the incoming data from EC and outgoing data to EC. */ struct cros_ec_command { - uint32_t version; - uint32_t command; - uint32_t outsize; - uint32_t insize; - uint32_t result; + struct_group_tagged(cros_ec_command_hdr, __hdr, + uint32_t version; + uint32_t command; + uint32_t outsize; + uint32_t insize; + uint32_t result; + ); uint8_t data[]; };