* [PATCH][next] leds: Avoid -Wflex-array-member-not-at-end warning @ 2025-03-28 14:33 Gustavo A. R. Silva 2025-03-28 18:31 ` Thomas Weißschuh 0 siblings, 1 reply; 5+ messages in thread From: Gustavo A. R. Silva @ 2025-03-28 14:33 UTC (permalink / raw) To: Thomas Weißschuh, Lee Jones, Pavel Machek, Benson Leung, Guenter Roeck Cc: linux-leds, chrome-platform, linux-kernel, Gustavo A. R. Silva, linux-hardening -Wflex-array-member-not-at-end was introduced in GCC-14, and we are getting ready to enable it, globally. 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/leds/leds-cros_ec.c:70:40: 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/leds/leds-cros_ec.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/drivers/leds/leds-cros_ec.c b/drivers/leds/leds-cros_ec.c index 275522b81ea5..6eab0474f52d 100644 --- a/drivers/leds/leds-cros_ec.c +++ b/drivers/leds/leds-cros_ec.c @@ -66,24 +66,20 @@ static int cros_ec_led_send_cmd(struct cros_ec_device *cros_ec, union cros_ec_led_cmd_data *arg) { int ret; - struct { - struct cros_ec_command msg; - union cros_ec_led_cmd_data data; - } __packed buf = { - .msg = { - .version = 1, - .command = EC_CMD_LED_CONTROL, - .insize = sizeof(arg->resp), - .outsize = sizeof(arg->req), - }, - .data.req = arg->req - }; - - ret = cros_ec_cmd_xfer_status(cros_ec, &buf.msg); + DEFINE_RAW_FLEX(struct cros_ec_command, msg, data, + sizeof(union cros_ec_led_cmd_data)); + + msg->version = 1; + msg->command = EC_CMD_LED_CONTROL; + msg->insize = sizeof(arg->resp); + msg->outsize = sizeof(arg->req); + *(struct ec_params_led_control *)msg->data = arg->req; + + ret = cros_ec_cmd_xfer_status(cros_ec, msg); if (ret < 0) return ret; - arg->resp = buf.data.resp; + arg->resp = *(struct ec_response_led_control *)msg->data; return 0; } -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH][next] leds: Avoid -Wflex-array-member-not-at-end warning 2025-03-28 14:33 [PATCH][next] leds: Avoid -Wflex-array-member-not-at-end warning Gustavo A. R. Silva @ 2025-03-28 18:31 ` Thomas Weißschuh 2025-03-28 18:51 ` Gustavo A. R. Silva 0 siblings, 1 reply; 5+ messages in thread From: Thomas Weißschuh @ 2025-03-28 18:31 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Lee Jones, Pavel Machek, Benson Leung, Guenter Roeck, linux-leds, chrome-platform, linux-kernel, linux-hardening Hi Gustavo, On 2025-03-28 08:33:22-0600, 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. > > 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/leds/leds-cros_ec.c:70:40: 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/leds/leds-cros_ec.c | 26 +++++++++++--------------- > 1 file changed, 11 insertions(+), 15 deletions(-) > > diff --git a/drivers/leds/leds-cros_ec.c b/drivers/leds/leds-cros_ec.c > index 275522b81ea5..6eab0474f52d 100644 > --- a/drivers/leds/leds-cros_ec.c > +++ b/drivers/leds/leds-cros_ec.c > @@ -66,24 +66,20 @@ static int cros_ec_led_send_cmd(struct cros_ec_device *cros_ec, > union cros_ec_led_cmd_data *arg) > { > int ret; > - struct { > - struct cros_ec_command msg; > - union cros_ec_led_cmd_data data; > - } __packed buf = { > - .msg = { > - .version = 1, > - .command = EC_CMD_LED_CONTROL, > - .insize = sizeof(arg->resp), > - .outsize = sizeof(arg->req), > - }, > - .data.req = arg->req > - }; > - > - ret = cros_ec_cmd_xfer_status(cros_ec, &buf.msg); > + DEFINE_RAW_FLEX(struct cros_ec_command, msg, data, > + sizeof(union cros_ec_led_cmd_data)); > + > + msg->version = 1; > + msg->command = EC_CMD_LED_CONTROL; > + msg->insize = sizeof(arg->resp); > + msg->outsize = sizeof(arg->req); > + *(struct ec_params_led_control *)msg->data = arg->req; To be honest this looks really ugly and it's not at all obvious what is going on. We have the utility function cros_ec_cmd() which would be the nicer alternative. (Without having verified that it avoids the warning). While it is slightly more expensive, I don't think it matters. And if it does, the helper can be optimized. (The same goes for my other cros_ec drivers) > + > + ret = cros_ec_cmd_xfer_status(cros_ec, msg); > if (ret < 0) > return ret; > > - arg->resp = buf.data.resp; > + arg->resp = *(struct ec_response_led_control *)msg->data; > > return 0; > } Thomas ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][next] leds: Avoid -Wflex-array-member-not-at-end warning 2025-03-28 18:31 ` Thomas Weißschuh @ 2025-03-28 18:51 ` Gustavo A. R. Silva 2025-03-29 9:32 ` Thomas Weißschuh 0 siblings, 1 reply; 5+ messages in thread From: Gustavo A. R. Silva @ 2025-03-28 18:51 UTC (permalink / raw) To: Thomas Weißschuh, Gustavo A. R. Silva Cc: Lee Jones, Pavel Machek, Benson Leung, Guenter Roeck, linux-leds, chrome-platform, linux-kernel, linux-hardening Hi! On 28/03/25 12:31, Thomas Weißschuh wrote: > Hi Gustavo, > > On 2025-03-28 08:33:22-0600, 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. >> >> 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/leds/leds-cros_ec.c:70:40: 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/leds/leds-cros_ec.c | 26 +++++++++++--------------- >> 1 file changed, 11 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/leds/leds-cros_ec.c b/drivers/leds/leds-cros_ec.c >> index 275522b81ea5..6eab0474f52d 100644 >> --- a/drivers/leds/leds-cros_ec.c >> +++ b/drivers/leds/leds-cros_ec.c >> @@ -66,24 +66,20 @@ static int cros_ec_led_send_cmd(struct cros_ec_device *cros_ec, >> union cros_ec_led_cmd_data *arg) >> { >> int ret; >> - struct { >> - struct cros_ec_command msg; >> - union cros_ec_led_cmd_data data; >> - } __packed buf = { >> - .msg = { >> - .version = 1, >> - .command = EC_CMD_LED_CONTROL, >> - .insize = sizeof(arg->resp), >> - .outsize = sizeof(arg->req), >> - }, >> - .data.req = arg->req >> - }; >> - >> - ret = cros_ec_cmd_xfer_status(cros_ec, &buf.msg); >> + DEFINE_RAW_FLEX(struct cros_ec_command, msg, data, >> + sizeof(union cros_ec_led_cmd_data)); >> + >> + msg->version = 1; >> + msg->command = EC_CMD_LED_CONTROL; >> + msg->insize = sizeof(arg->resp); >> + msg->outsize = sizeof(arg->req); >> + *(struct ec_params_led_control *)msg->data = arg->req; > > To be honest this looks really ugly and it's not at all obvious what is We can do something like this, instead: diff --git a/drivers/leds/leds-cros_ec.c b/drivers/leds/leds-cros_ec.c index 275522b81ea5..c7235f4e577b 100644 --- a/drivers/leds/leds-cros_ec.c +++ b/drivers/leds/leds-cros_ec.c @@ -66,24 +66,24 @@ static int cros_ec_led_send_cmd(struct cros_ec_device *cros_ec, union cros_ec_led_cmd_data *arg) { int ret; - struct { - struct cros_ec_command msg; - union cros_ec_led_cmd_data data; - } __packed buf = { - .msg = { - .version = 1, - .command = EC_CMD_LED_CONTROL, - .insize = sizeof(arg->resp), - .outsize = sizeof(arg->req), - }, - .data.req = arg->req - }; - - ret = cros_ec_cmd_xfer_status(cros_ec, &buf.msg); + DEFINE_RAW_FLEX(struct cros_ec_command, msg, data, + sizeof(union cros_ec_led_cmd_data)); + struct ec_params_led_control *req = + (struct ec_params_led_control *)msg->data; + struct ec_response_led_control *resp = + (struct ec_response_led_control *)msg->data; + + msg->version = 1; + msg->command = EC_CMD_LED_CONTROL; + msg->insize = sizeof(arg->resp); + msg->outsize = sizeof(arg->req); + *req = arg->req; + + ret = cros_ec_cmd_xfer_status(cros_ec, msg); if (ret < 0) return ret; - arg->resp = buf.data.resp; + arg->resp = *resp; return 0; } as in other cases: https://lore.kernel.org/linux-hardening/Z-a4meHAy-t58bcE@kspp/ -Gustavo > going on. We have the utility function cros_ec_cmd() which would be the > nicer alternative. (Without having verified that it avoids the warning). > While it is slightly more expensive, I don't think it matters. > And if it does, the helper can be optimized. > > (The same goes for my other cros_ec drivers) > >> + >> + ret = cros_ec_cmd_xfer_status(cros_ec, msg); >> if (ret < 0) >> return ret; >> >> - arg->resp = buf.data.resp; >> + arg->resp = *(struct ec_response_led_control *)msg->data; >> >> return 0; >> } > > > Thomas ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH][next] leds: Avoid -Wflex-array-member-not-at-end warning 2025-03-28 18:51 ` Gustavo A. R. Silva @ 2025-03-29 9:32 ` Thomas Weißschuh 2025-03-31 17:04 ` Gustavo A. R. Silva 0 siblings, 1 reply; 5+ messages in thread From: Thomas Weißschuh @ 2025-03-29 9:32 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Gustavo A. R. Silva, Lee Jones, Pavel Machek, Benson Leung, Guenter Roeck, linux-leds, chrome-platform, linux-kernel, linux-hardening Hi! On 2025-03-28 12:51:02-0600, Gustavo A. R. Silva wrote: > On 28/03/25 12:31, Thomas Weißschuh wrote: > > On 2025-03-28 08:33:22-0600, 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. > > > > > > 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/leds/leds-cros_ec.c:70:40: 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/leds/leds-cros_ec.c | 26 +++++++++++--------------- > > > 1 file changed, 11 insertions(+), 15 deletions(-) > > > > > > diff --git a/drivers/leds/leds-cros_ec.c b/drivers/leds/leds-cros_ec.c > > > index 275522b81ea5..6eab0474f52d 100644 > > > --- a/drivers/leds/leds-cros_ec.c > > > +++ b/drivers/leds/leds-cros_ec.c > > > @@ -66,24 +66,20 @@ static int cros_ec_led_send_cmd(struct cros_ec_device *cros_ec, > > > union cros_ec_led_cmd_data *arg) > > > { > > > int ret; > > > - struct { > > > - struct cros_ec_command msg; > > > - union cros_ec_led_cmd_data data; > > > - } __packed buf = { > > > - .msg = { > > > - .version = 1, > > > - .command = EC_CMD_LED_CONTROL, > > > - .insize = sizeof(arg->resp), > > > - .outsize = sizeof(arg->req), > > > - }, > > > - .data.req = arg->req > > > - }; > > > - > > > - ret = cros_ec_cmd_xfer_status(cros_ec, &buf.msg); > > > + DEFINE_RAW_FLEX(struct cros_ec_command, msg, data, > > > + sizeof(union cros_ec_led_cmd_data)); > > > + > > > + msg->version = 1; > > > + msg->command = EC_CMD_LED_CONTROL; > > > + msg->insize = sizeof(arg->resp); > > > + msg->outsize = sizeof(arg->req); > > > + *(struct ec_params_led_control *)msg->data = arg->req; > > > > To be honest this looks really ugly and it's not at all obvious what is > > We can do something like this, instead: > > diff --git a/drivers/leds/leds-cros_ec.c b/drivers/leds/leds-cros_ec.c > index 275522b81ea5..c7235f4e577b 100644 > --- a/drivers/leds/leds-cros_ec.c > +++ b/drivers/leds/leds-cros_ec.c > @@ -66,24 +66,24 @@ static int cros_ec_led_send_cmd(struct cros_ec_device *cros_ec, > union cros_ec_led_cmd_data *arg) > { > int ret; > - struct { > - struct cros_ec_command msg; > - union cros_ec_led_cmd_data data; > - } __packed buf = { > - .msg = { > - .version = 1, > - .command = EC_CMD_LED_CONTROL, > - .insize = sizeof(arg->resp), > - .outsize = sizeof(arg->req), > - }, > - .data.req = arg->req > - }; > - > - ret = cros_ec_cmd_xfer_status(cros_ec, &buf.msg); > + DEFINE_RAW_FLEX(struct cros_ec_command, msg, data, > + sizeof(union cros_ec_led_cmd_data)); > + struct ec_params_led_control *req = > + (struct ec_params_led_control *)msg->data; > + struct ec_response_led_control *resp = > + (struct ec_response_led_control *)msg->data; > + > + msg->version = 1; > + msg->command = EC_CMD_LED_CONTROL; > + msg->insize = sizeof(arg->resp); > + msg->outsize = sizeof(arg->req); > + *req = arg->req; > + > + ret = cros_ec_cmd_xfer_status(cros_ec, msg); > if (ret < 0) > return ret; > > - arg->resp = buf.data.resp; > + arg->resp = *resp; > > return 0; > } My issue was with the general usage of DEFINE_RAW_FLEX(), the reply was placed badly. For somebody not knowing this construct it's not clear what is happening under the hood. It's probably fine in a regular header file with some explanation, but in a random driver it looks off. The following is what I had in mind. Now actually tested. --- a/drivers/leds/leds-cros_ec.c +++ b/drivers/leds/leds-cros_ec.c @@ -60,31 +60,18 @@ static inline struct cros_ec_led_priv *cros_ec_led_cdev_to_priv(struct led_class union cros_ec_led_cmd_data { struct ec_params_led_control req; struct ec_response_led_control resp; -} __packed; +}; static int cros_ec_led_send_cmd(struct cros_ec_device *cros_ec, union cros_ec_led_cmd_data *arg) { int ret; - struct { - struct cros_ec_command msg; - union cros_ec_led_cmd_data data; - } __packed buf = { - .msg = { - .version = 1, - .command = EC_CMD_LED_CONTROL, - .insize = sizeof(arg->resp), - .outsize = sizeof(arg->req), - }, - .data.req = arg->req - }; - - ret = cros_ec_cmd_xfer_status(cros_ec, &buf.msg); + + ret = cros_ec_cmd(cros_ec, 1, EC_CMD_LED_CONTROL, + &arg->req, sizeof(arg->req), &arg->resp, sizeof(arg->resp)); if (ret < 0) return ret; - arg->resp = buf.data.resp; - return 0; } > > as in other cases: > > https://lore.kernel.org/linux-hardening/Z-a4meHAy-t58bcE@kspp/ > > -Gustavo > > > going on. We have the utility function cros_ec_cmd() which would be the > > nicer alternative. (Without having verified that it avoids the warning). > > While it is slightly more expensive, I don't think it matters. > > And if it does, the helper can be optimized. > > > > (The same goes for my other cros_ec drivers) > > > > > + > > > + ret = cros_ec_cmd_xfer_status(cros_ec, msg); > > > if (ret < 0) > > > return ret; > > > - arg->resp = buf.data.resp; > > > + arg->resp = *(struct ec_response_led_control *)msg->data; > > > return 0; > > > } Thomas ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][next] leds: Avoid -Wflex-array-member-not-at-end warning 2025-03-29 9:32 ` Thomas Weißschuh @ 2025-03-31 17:04 ` Gustavo A. R. Silva 0 siblings, 0 replies; 5+ messages in thread From: Gustavo A. R. Silva @ 2025-03-31 17:04 UTC (permalink / raw) To: Thomas Weißschuh Cc: Gustavo A. R. Silva, Lee Jones, Pavel Machek, Benson Leung, Guenter Roeck, linux-leds, chrome-platform, linux-kernel, linux-hardening > My issue was with the general usage of DEFINE_RAW_FLEX(), the reply was > placed badly. For somebody not knowing this construct it's not clear > what is happening under the hood. > It's probably fine in a regular header file with some explanation, > but in a random driver it looks off. > > The following is what I had in mind. Now actually tested. OK, let's go with that :) https://lore.kernel.org/linux-hardening/Z-rKcgFjsyKvd58q@kspp/ Thanks! -- Gustavo ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-03-31 17:04 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-28 14:33 [PATCH][next] leds: Avoid -Wflex-array-member-not-at-end warning Gustavo A. R. Silva 2025-03-28 18:31 ` Thomas Weißschuh 2025-03-28 18:51 ` Gustavo A. R. Silva 2025-03-29 9:32 ` Thomas Weißschuh 2025-03-31 17:04 ` Gustavo A. R. Silva
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.