Re: [PATCH][next] rpmsg: glink: Avoid -Wflex-array-member-not-at-end warnings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi all,

Friendly ping: who can take this, please? 🙂

Thanks
-Gustavo

On 07/08/24 17:19, 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.

So, in order to avoid ending up with a flexible-array member in the
middle of multiple other structs, we use the `__struct_group()`
helper to create a new tagged `struct glink_msg_hdr`. This structure
groups together all the members of the flexible `struct glink_msg`
except the flexible array.

As a result, the array is effectively separated from the rest of the
members without modifying the memory layout of the flexible structure.
We then change the type of the middle struct members currently causing
trouble from `struct glink_msg` to `struct glink_msg_hdr`.

We also want to ensure that when new members need to be added to the
flexible structure, they are always included within the newly created
tagged struct. For this, we use `static_assert()`. This ensures that the
memory layout for both the flexible structure and the new tagged struct
is the same after any changes.

This approach avoids having to implement `struct glink_msg_hdr` as a
completely separate structure, thus preventing having to maintain two
independent but basically identical structures, closing the door to
potential bugs in the future.

We also use `container_of()` whenever we need to retrieve a pointer to
the flexible structure, through which we can access the flexible-array
member, if necessary.

Additionally, we use the `DEFINE_RAW_FLEX()` helper for an on-stack
definition of a flexible structure where the size for the flexible-array
member is known at compile-time.

So, with these changes, fix the following warnings:
drivers/rpmsg/qcom_glink_native.c:51:26: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/rpmsg/qcom_glink_native.c:459:34: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/rpmsg/qcom_glink_native.c:846:34: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/rpmsg/qcom_glink_native.c:968:34: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
drivers/rpmsg/qcom_glink_native.c:1380:34: 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@xxxxxxxxxx>
---
  drivers/rpmsg/qcom_glink_native.c | 42 +++++++++++++++++--------------
  1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/rpmsg/qcom_glink_native.c b/drivers/rpmsg/qcom_glink_native.c
index 82d460ff4777..ed89b810f262 100644
--- a/drivers/rpmsg/qcom_glink_native.c
+++ b/drivers/rpmsg/qcom_glink_native.c
@@ -30,11 +30,16 @@
  #define RPM_GLINK_CID_MAX	65536
struct glink_msg {
-	__le16 cmd;
-	__le16 param1;
-	__le32 param2;
+	/* New members MUST be added within the __struct_group() macro below. */
+	__struct_group(glink_msg_hdr, hdr, __packed,
+		__le16 cmd;
+		__le16 param1;
+		__le32 param2;
+	);
  	u8 data[];
  } __packed;
+static_assert(offsetof(struct glink_msg, data) == sizeof(struct glink_msg_hdr),
+	      "struct member likely outside of __struct_group()");
/**
   * struct glink_defer_cmd - deferred incoming control message
@@ -48,7 +53,7 @@ struct glink_msg {
  struct glink_defer_cmd {
  	struct list_head node;
- struct glink_msg msg;
+	struct glink_msg_hdr msg;
  	u8 data[];
  };
@@ -455,12 +460,9 @@ static void qcom_glink_intent_req_abort(struct glink_channel *channel)
  static int qcom_glink_send_open_req(struct qcom_glink *glink,
  				    struct glink_channel *channel)
  {
-	struct {
-		struct glink_msg msg;
-		u8 name[GLINK_NAME_SIZE];
-	} __packed req;
+	DEFINE_RAW_FLEX(struct glink_msg, req, data, GLINK_NAME_SIZE);
  	int name_len = strlen(channel->name) + 1;
-	int req_len = ALIGN(sizeof(req.msg) + name_len, 8);
+	int req_len = ALIGN(sizeof(*req) + name_len, 8);
  	int ret;
  	unsigned long flags;
@@ -476,12 +478,12 @@ static int qcom_glink_send_open_req(struct qcom_glink *glink, channel->lcid = ret; - req.msg.cmd = cpu_to_le16(GLINK_CMD_OPEN);
-	req.msg.param1 = cpu_to_le16(channel->lcid);
-	req.msg.param2 = cpu_to_le32(name_len);
-	strcpy(req.name, channel->name);
+	req->cmd = cpu_to_le16(GLINK_CMD_OPEN);
+	req->param1 = cpu_to_le16(channel->lcid);
+	req->param2 = cpu_to_le32(name_len);
+	strcpy(req->data, channel->name);
- ret = qcom_glink_tx(glink, &req, req_len, NULL, 0, true);
+	ret = qcom_glink_tx(glink, req, req_len, NULL, 0, true);
  	if (ret)
  		goto remove_idr;
@@ -826,7 +828,9 @@ static int qcom_glink_rx_defer(struct qcom_glink *glink, size_t extra) INIT_LIST_HEAD(&dcmd->node); - qcom_glink_rx_peek(glink, &dcmd->msg, 0, sizeof(dcmd->msg) + extra);
+	qcom_glink_rx_peek(glink,
+			   container_of(&dcmd->msg, struct glink_msg, hdr), 0,
+			   sizeof(dcmd->msg) + extra);
spin_lock(&glink->rx_lock);
  	list_add_tail(&dcmd->node, &glink->rx_queue);
@@ -843,7 +847,7 @@ static int qcom_glink_rx_data(struct qcom_glink *glink, size_t avail)
  	struct glink_core_rx_intent *intent;
  	struct glink_channel *channel;
  	struct {
-		struct glink_msg msg;
+		struct glink_msg_hdr msg;
  		__le32 chunk_size;
  		__le32 left_size;
  	} __packed hdr;
@@ -965,7 +969,7 @@ static void qcom_glink_handle_intent(struct qcom_glink *glink,
  	};
struct {
-		struct glink_msg msg;
+		struct glink_msg_hdr msg;
  		struct intent_pair intents[];
  	} __packed * msg;
@@ -1377,7 +1381,7 @@ static int __qcom_glink_send(struct glink_channel *channel,
  	struct glink_core_rx_intent *tmp;
  	int iid = 0;
  	struct {
-		struct glink_msg msg;
+		struct glink_msg_hdr msg;
  		__le32 chunk_size;
  		__le32 left_size;
  	} __packed req;
@@ -1685,7 +1689,7 @@ static void qcom_glink_work(struct work_struct *work)
  		list_del(&dcmd->node);
  		spin_unlock_irqrestore(&glink->rx_lock, flags);
- msg = &dcmd->msg;
+		msg = container_of(&dcmd->msg, struct glink_msg, hdr);
  		cmd = le16_to_cpu(msg->cmd);
  		param1 = le16_to_cpu(msg->param1);
  		param2 = le32_to_cpu(msg->param2);




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux