| Message ID | Z9dy43vUUh4goi-Q@kspp (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [next] iio: cros_ec: Avoid -Wflex-array-member-not-at-end warning | expand |
On Mon, Mar 17, 2025 at 11:24:59AM +1030, Gustavo A. R. Silva wrote: > static int cros_ec_get_host_cmd_version_mask(struct cros_ec_device *ec_dev, > u16 cmd_offset, u16 cmd, u32 *mask) > { > + DEFINE_RAW_FLEX(struct cros_ec_command, buf, data, > + sizeof(struct ec_response_get_cmd_versions)); max(sizeof(struct ec_params_get_cmd_versions), sizeof(struct ec_response_get_cmd_versions))?
On 17/03/25 12:02, Tzung-Bi Shih wrote: > On Mon, Mar 17, 2025 at 11:24:59AM +1030, Gustavo A. R. Silva wrote: >> static int cros_ec_get_host_cmd_version_mask(struct cros_ec_device *ec_dev, >> u16 cmd_offset, u16 cmd, u32 *mask) >> { >> + DEFINE_RAW_FLEX(struct cros_ec_command, buf, data, >> + sizeof(struct ec_response_get_cmd_versions)); > > max(sizeof(struct ec_params_get_cmd_versions), > sizeof(struct ec_response_get_cmd_versions))? I considered that, but DEFINE_RAW_FLEX() complains about it due to the _Static_assert(__builtin_constant_p(count), \ "onstack flex array members require compile-time const count"); -- Gustavo
On Mon, 17 Mar 2025 15:10:38 +1030 "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote: > On 17/03/25 12:02, Tzung-Bi Shih wrote: > > On Mon, Mar 17, 2025 at 11:24:59AM +1030, Gustavo A. R. Silva wrote: > >> static int cros_ec_get_host_cmd_version_mask(struct cros_ec_device *ec_dev, > >> u16 cmd_offset, u16 cmd, u32 *mask) > >> { > >> + DEFINE_RAW_FLEX(struct cros_ec_command, buf, data, > >> + sizeof(struct ec_response_get_cmd_versions)); > > > > max(sizeof(struct ec_params_get_cmd_versions), > > sizeof(struct ec_response_get_cmd_versions))? > > I considered that, but DEFINE_RAW_FLEX() complains about it due to the > > _Static_assert(__builtin_constant_p(count), \ > "onstack flex array members require compile-time const count"); > Maybe add an assert that you indeed have the larger of the two + a comment on why it matters? > -- > Gustavo >
On Mon, Mar 17, 2025 at 12:04:47PM +0000, Jonathan Cameron wrote: > On Mon, 17 Mar 2025 15:10:38 +1030 > "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote: > > > On 17/03/25 12:02, Tzung-Bi Shih wrote: > > > On Mon, Mar 17, 2025 at 11:24:59AM +1030, Gustavo A. R. Silva wrote: > > >> static int cros_ec_get_host_cmd_version_mask(struct cros_ec_device *ec_dev, > > >> u16 cmd_offset, u16 cmd, u32 *mask) > > >> { > > >> + DEFINE_RAW_FLEX(struct cros_ec_command, buf, data, > > >> + sizeof(struct ec_response_get_cmd_versions)); > > > > > > max(sizeof(struct ec_params_get_cmd_versions), > > > sizeof(struct ec_response_get_cmd_versions))? > > > > I considered that, but DEFINE_RAW_FLEX() complains about it due to the > > > > _Static_assert(__builtin_constant_p(count), \ > > "onstack flex array members require compile-time const count"); > > > Maybe add an assert that you indeed have the larger of the two + a comment > on why it matters? Or, is there a way for using compiler-time MAX()? I failed to find so.
On March 17, 2025 6:51:50 PM PDT, Tzung-Bi Shih <tzungbi@kernel.org> wrote: >On Mon, Mar 17, 2025 at 12:04:47PM +0000, Jonathan Cameron wrote: >> On Mon, 17 Mar 2025 15:10:38 +1030 >> "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote: >> >> > On 17/03/25 12:02, Tzung-Bi Shih wrote: >> > > On Mon, Mar 17, 2025 at 11:24:59AM +1030, Gustavo A. R. Silva wrote: >> > >> static int cros_ec_get_host_cmd_version_mask(struct cros_ec_device *ec_dev, >> > >> u16 cmd_offset, u16 cmd, u32 *mask) >> > >> { >> > >> + DEFINE_RAW_FLEX(struct cros_ec_command, buf, data, >> > >> + sizeof(struct ec_response_get_cmd_versions)); >> > > >> > > max(sizeof(struct ec_params_get_cmd_versions), >> > > sizeof(struct ec_response_get_cmd_versions))? >> > >> > I considered that, but DEFINE_RAW_FLEX() complains about it due to the >> > >> > _Static_assert(__builtin_constant_p(count), \ >> > "onstack flex array members require compile-time const count"); >> > >> Maybe add an assert that you indeed have the larger of the two + a comment >> on why it matters? > >Or, is there a way for using compiler-time MAX()? I failed to find so. Did MAX() not work? I would expect it to do compile time comparison of two sizeof()s. It can do other arithmetic no problem, e.g.: https://elixir.bootlin.com/linux/v6.13.7/source/lib/vsprintf.c#L1097
> Did MAX() not work? I would expect it to do compile time comparison of two sizeof()s. It can do other arithmetic no problem, e.g.: > https://elixir.bootlin.com/linux/v6.13.7/source/lib/vsprintf.c#L1097 Yep, that does the trick. :) Thanks -- Gustavo
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c index 7751d6f69b12..760a07f64390 100644 --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c @@ -34,25 +34,18 @@ static int cros_ec_get_host_cmd_version_mask(struct cros_ec_device *ec_dev, u16 cmd_offset, u16 cmd, u32 *mask) { + DEFINE_RAW_FLEX(struct cros_ec_command, buf, data, + sizeof(struct ec_response_get_cmd_versions)); int ret; - struct { - struct cros_ec_command msg; - union { - struct ec_params_get_cmd_versions params; - struct ec_response_get_cmd_versions resp; - }; - } __packed buf = { - .msg = { - .command = EC_CMD_GET_CMD_VERSIONS + cmd_offset, - .insize = sizeof(struct ec_response_get_cmd_versions), - .outsize = sizeof(struct ec_params_get_cmd_versions) - }, - .params = {.cmd = cmd} - }; - - ret = cros_ec_cmd_xfer_status(ec_dev, &buf.msg); + + buf->command = EC_CMD_GET_CMD_VERSIONS + cmd_offset; + buf->insize = sizeof(struct ec_response_get_cmd_versions); + buf->outsize = sizeof(struct ec_params_get_cmd_versions); + ((struct ec_params_get_cmd_versions *)buf->data)->cmd = cmd; + + ret = cros_ec_cmd_xfer_status(ec_dev, buf); if (ret >= 0) - *mask = buf.resp.version_mask; + *mask = ((struct ec_response_get_cmd_versions *)buf->data)->version_mask; return ret; }
-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/iio/common/cros_ec_sensors/cros_ec_sensors_core.c:39: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> --- This looks pretty much like the following patch for RTC: https://lore.kernel.org/linux-hardening/Z9PpPg06OK8ghNvm@kspp/ Thanks .../cros_ec_sensors/cros_ec_sensors_core.c | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-)