Skip to contentSkip to conversation

rtc: Avoid a couple of -Wflex-array-member-not-at-end warnings

RFC In review Gustavo A. R. Silva 3 participants 3 messages Mar 10, 2025 – Mar 14, 2025

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

RFC 1/1 rtc: Avoid a couple of -Wflex-array-member-not-at-end warnings

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.

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":

drivers/rtc/rtc-cros-ec.c +2 -2
index 865c2e82c7a5..7e9bbab47e4c 100644
@@ -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:

drivers/rtc/rtc-cros-ec.c +10 -4
index 865c2e82c7a5..6dc815bdbcd9 100644
@@ -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;
include/linux/platform_data/cros_ec_proto.h +7 -5
index 3ec24f445c29..2a638c8c5ec2 100644
@@ -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[];
 };
 

What do you think?

Thanks!
--
Gustavo

#1 lore raw
Quoted 11 lines · Gustavo A. R. Silva

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

Quoted 19 lines

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":

drivers/rtc/rtc-cros-ec.c +1 -1
index 865c2e82c7a5..7e9bbab47e4c 100644
@@ -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));

drivers/rtc/rtc-cros-ec.c static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command, · line 66

Quoted 9 lines
drivers/rtc/rtc-cros-ec.c +1 -1
@@ -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

Quoted 1 line

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), ...);