-Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
ready to enable it globally.
There are currently a couple of objects (`req` and `rsp`), in a couple
of structures, that contain flexible structures (`struct l2cap_ecred_conn_req`
and `struct l2cap_ecred_conn_rsp`), for example:
struct l2cap_ecred_rsp_data {
struct {
struct l2cap_ecred_conn_rsp rsp;
__le16 scid[L2CAP_ECRED_MAX_CID];
} __packed pdu;
int count;
};
in the struct above, `struct l2cap_ecred_conn_rsp` is a flexible
structure:
struct l2cap_ecred_conn_rsp {
__le16 mtu;
__le16 mps;
__le16 credits;
__le16 result;
__le16 dcid[];
};
So, in order to avoid ending up with a flexible-array member in the
middle of another structure, we use the `struct_group_tagged()` (and
`__struct_group()` when the flexible structure is `__packed`) helper
to separate the flexible array from the rest of the members in the
flexible structure:
struct l2cap_ecred_conn_rsp {
struct_group_tagged(l2cap_ecred_conn_rsp_hdr, hdr,
... the rest of members
);
__le16 dcid[];
};
With the change described above, we now declare objects of the type of
the tagged struct, in this example `struct l2cap_ecred_conn_rsp_hdr`,
without embedding flexible arrays in the middle of other structures:
struct l2cap_ecred_rsp_data {
struct {
struct l2cap_ecred_conn_rsp_hdr rsp;
__le16 scid[L2CAP_ECRED_MAX_CID];
} __packed pdu;
int count;
};
Also, when the flexible-array member needs to be accessed, we use
`container_of()` to retrieve a pointer to the flexible structure.
We also use the `DEFINE_RAW_FLEX()` helper for a couple of on-stack
definitions of a flexible structure where the size of the flexible-array
member is known at compile-time.
So, with these changes, fix the following warnings:
net/bluetooth/l2cap_core.c:1260:45: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
net/bluetooth/l2cap_core.c:3740:45: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
net/bluetooth/l2cap_core.c:4999:45: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
net/bluetooth/l2cap_core.c:7116:47: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
Link: https://github.com/KSPP/linux/issues/202
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
Hi!
I wonder if `struct l2cap_ecred_conn_rsp` should also be `__packed`.
Thanks
- Gustavo
include/net/bluetooth/l2cap.h | 20 +++++++++------
net/bluetooth/l2cap_core.c | 46 ++++++++++++++++-------------------
2 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index a4278aa618ab..92a143517d83 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -463,18 +463,22 @@ struct l2cap_le_credits {
#define L2CAP_ECRED_MAX_CID 5
struct l2cap_ecred_conn_req {
- __le16 psm;
- __le16 mtu;
- __le16 mps;
- __le16 credits;
+ __struct_group(l2cap_ecred_conn_req_hdr, hdr, __packed,
+ __le16 psm;
+ __le16 mtu;
+ __le16 mps;
+ __le16 credits;
+ );
__le16 scid[];
} __packed;
struct l2cap_ecred_conn_rsp {
- __le16 mtu;
- __le16 mps;
- __le16 credits;
- __le16 result;
+ struct_group_tagged(l2cap_ecred_conn_rsp_hdr, hdr,
+ __le16 mtu;
+ __le16 mps;
+ __le16 credits;
+ __le16 result;
+ );
__le16 dcid[];
};
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 467b242d8be0..bf087eca489e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1257,7 +1257,7 @@ static void l2cap_le_connect(struct l2cap_chan *chan)
struct l2cap_ecred_conn_data {
struct {
- struct l2cap_ecred_conn_req req;
+ struct l2cap_ecred_conn_req_hdr req;
__le16 scid[5];
} __packed pdu;
struct l2cap_chan *chan;
@@ -3737,7 +3737,7 @@ static void l2cap_ecred_list_defer(struct l2cap_chan *chan, void *data)
struct l2cap_ecred_rsp_data {
struct {
- struct l2cap_ecred_conn_rsp rsp;
+ struct l2cap_ecred_conn_rsp_hdr rsp;
__le16 scid[L2CAP_ECRED_MAX_CID];
} __packed pdu;
int count;
@@ -3746,6 +3746,8 @@ struct l2cap_ecred_rsp_data {
static void l2cap_ecred_rsp_defer(struct l2cap_chan *chan, void *data)
{
struct l2cap_ecred_rsp_data *rsp = data;
+ struct l2cap_ecred_conn_rsp *rsp_flex =
+ container_of(&rsp->pdu.rsp, struct l2cap_ecred_conn_rsp, hdr);
if (test_bit(FLAG_ECRED_CONN_REQ_SENT, &chan->flags))
return;
@@ -3755,7 +3757,7 @@ static void l2cap_ecred_rsp_defer(struct l2cap_chan *chan, void *data)
/* Include all channels pending with the same ident */
if (!rsp->pdu.rsp.result)
- rsp->pdu.rsp.dcid[rsp->count++] = cpu_to_le16(chan->scid);
+ rsp_flex->dcid[rsp->count++] = cpu_to_le16(chan->scid);
else
l2cap_chan_del(chan, ECONNRESET);
}
@@ -4995,10 +4997,7 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
u8 *data)
{
struct l2cap_ecred_conn_req *req = (void *) data;
- struct {
- struct l2cap_ecred_conn_rsp rsp;
- __le16 dcid[L2CAP_ECRED_MAX_CID];
- } __packed pdu;
+ DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
struct l2cap_chan *chan, *pchan;
u16 mtu, mps;
__le16 psm;
@@ -5017,7 +5016,7 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
cmd_len -= sizeof(*req);
num_scid = cmd_len / sizeof(u16);
- if (num_scid > ARRAY_SIZE(pdu.dcid)) {
+ if (num_scid > L2CAP_ECRED_MAX_CID) {
result = L2CAP_CR_LE_INVALID_PARAMS;
goto response;
}
@@ -5046,7 +5045,7 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
BT_DBG("psm 0x%2.2x mtu %u mps %u", __le16_to_cpu(psm), mtu, mps);
- memset(&pdu, 0, sizeof(pdu));
+ memset(pdu, 0, sizeof(*pdu));
/* Check if we have socket listening on psm */
pchan = l2cap_global_chan_by_psm(BT_LISTEN, psm, &conn->hcon->src,
@@ -5072,8 +5071,8 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
BT_DBG("scid[%d] 0x%4.4x", i, scid);
- pdu.dcid[i] = 0x0000;
- len += sizeof(*pdu.dcid);
+ pdu->dcid[i] = 0x0000;
+ len += sizeof(*pdu->dcid);
/* Check for valid dynamic CID range */
if (scid < L2CAP_CID_DYN_START || scid > L2CAP_CID_LE_DYN_END) {
@@ -5107,13 +5106,13 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
l2cap_ecred_init(chan, __le16_to_cpu(req->credits));
/* Init response */
- if (!pdu.rsp.credits) {
- pdu.rsp.mtu = cpu_to_le16(chan->imtu);
- pdu.rsp.mps = cpu_to_le16(chan->mps);
- pdu.rsp.credits = cpu_to_le16(chan->rx_credits);
+ if (!pdu->credits) {
+ pdu->mtu = cpu_to_le16(chan->imtu);
+ pdu->mps = cpu_to_le16(chan->mps);
+ pdu->credits = cpu_to_le16(chan->rx_credits);
}
- pdu.dcid[i] = cpu_to_le16(chan->scid);
+ pdu->dcid[i] = cpu_to_le16(chan->scid);
__set_chan_timer(chan, chan->ops->get_sndtimeo(chan));
@@ -5135,13 +5134,13 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
l2cap_chan_put(pchan);
response:
- pdu.rsp.result = cpu_to_le16(result);
+ pdu->result = cpu_to_le16(result);
if (defer)
return 0;
l2cap_send_cmd(conn, cmd->ident, L2CAP_ECRED_CONN_RSP,
- sizeof(pdu.rsp) + len, &pdu);
+ sizeof(*pdu) + len, pdu);
return 0;
}
@@ -7112,14 +7111,11 @@ EXPORT_SYMBOL_GPL(l2cap_chan_connect);
static void l2cap_ecred_reconfigure(struct l2cap_chan *chan)
{
struct l2cap_conn *conn = chan->conn;
- struct {
- struct l2cap_ecred_reconf_req req;
- __le16 scid;
- } pdu;
+ DEFINE_RAW_FLEX(struct l2cap_ecred_reconf_req, pdu, scid, 1);
- pdu.req.mtu = cpu_to_le16(chan->imtu);
- pdu.req.mps = cpu_to_le16(chan->mps);
- pdu.scid = cpu_to_le16(chan->scid);
+ pdu->mtu = cpu_to_le16(chan->imtu);
+ pdu->mps = cpu_to_le16(chan->mps);
+ pdu->scid[0] = cpu_to_le16(chan->scid);
chan->ident = l2cap_get_ident(conn);
--
2.34.1
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=838554
---Test result---
Test Summary:
CheckPatch FAIL 1.08 seconds
GitLint FAIL 0.60 seconds
SubjectPrefix PASS 0.12 seconds
BuildKernel FAIL 22.88 seconds
CheckAllWarning FAIL 24.75 seconds
CheckSparse FAIL 29.51 seconds
CheckSmatch FAIL 72.39 seconds
BuildKernel32 FAIL 22.78 seconds
TestRunnerSetup FAIL 487.10 seconds
TestRunner_l2cap-tester FAIL 0.14 seconds
TestRunner_iso-tester FAIL 0.14 seconds
TestRunner_bnep-tester FAIL 0.15 seconds
TestRunner_mgmt-tester FAIL 0.14 seconds
TestRunner_rfcomm-tester FAIL 0.14 seconds
TestRunner_sco-tester FAIL 0.15 seconds
TestRunner_ioctl-tester FAIL 0.15 seconds
TestRunner_mesh-tester FAIL 0.14 seconds
TestRunner_smp-tester FAIL 0.14 seconds
TestRunner_userchan-tester FAIL 0.42 seconds
IncrementalBuild FAIL 20.44 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[next] Bluetooth: L2CAP: Avoid -Wflex-array-member-not-at-end warnings
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#69:
of structures, that contain flexible structures (`struct l2cap_ecred_conn_req`
total: 0 errors, 1 warnings, 0 checks, 150 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13604949.patch has style problems, please review.
NOTE: Ignored message types: UNKNOWN_COMMIT_ID
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[next] Bluetooth: L2CAP: Avoid -Wflex-array-member-not-at-end warnings
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
38: B3 Line contains hard tab characters (\t): " ... the rest of members"
64: B1 Line exceeds max length (161>80): "net/bluetooth/l2cap_core.c:1260:45: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]"
65: B1 Line exceeds max length (161>80): "net/bluetooth/l2cap_core.c:3740:45: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]"
66: B1 Line exceeds max length (161>80): "net/bluetooth/l2cap_core.c:4999:45: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]"
67: B1 Line exceeds max length (161>80): "net/bluetooth/l2cap_core.c:7116:47: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]"
##############################
Test: BuildKernel - FAIL
Desc: Build Kernel for Bluetooth
Output:
net/bluetooth/l2cap_core.c: In function ‘l2cap_ecred_conn_req’:
net/bluetooth/l2cap_core.c:5000:2: error: implicit declaration of function ‘DEFINE_RAW_FLEX’; did you mean ‘DEFINE_FLEX’? [-Werror=implicit-function-declaration]
5000 | DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
| ^~~~~~~~~~~~~~~
| DEFINE_FLEX
net/bluetooth/l2cap_core.c:5000:18: error: expected expression before ‘struct’
5000 | DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
| ^~~~~~
net/bluetooth/l2cap_core.c:5048:9: error: ‘pdu’ undeclared (first use in this function)
5048 | memset(pdu, 0, sizeof(*pdu));
| ^~~
net/bluetooth/l2cap_core.c:5048:9: note: each undeclared identifier is reported only once for each function it appears in
net/bluetooth/l2cap_core.c: In function ‘l2cap_ecred_reconfigure’:
net/bluetooth/l2cap_core.c:7114:18: error: expected expression before ‘struct’
7114 | DEFINE_RAW_FLEX(struct l2cap_ecred_reconf_req, pdu, scid, 1);
| ^~~~~~
net/bluetooth/l2cap_core.c:7116:2: error: ‘pdu’ undeclared (first use in this function)
7116 | pdu->mtu = cpu_to_le16(chan->imtu);
| ^~~
cc1: some warnings being treated as errors
make[4]: *** [scripts/Makefile.build:243: net/bluetooth/l2cap_core.o] Error 1
make[3]: *** [scripts/Makefile.build:481: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:481: net] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/github/workspace/src/src/Makefile:1921: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2
##############################
Test: CheckAllWarning - FAIL
Desc: Run linux kernel with all warning enabled
Output:
net/bluetooth/l2cap_core.c: In function ‘l2cap_ecred_conn_req’:
net/bluetooth/l2cap_core.c:5000:2: error: implicit declaration of function ‘DEFINE_RAW_FLEX’; did you mean ‘DEFINE_FLEX’? [-Werror=implicit-function-declaration]
5000 | DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
| ^~~~~~~~~~~~~~~
| DEFINE_FLEX
net/bluetooth/l2cap_core.c:5000:18: error: expected expression before ‘struct’
5000 | DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
| ^~~~~~
net/bluetooth/l2cap_core.c:5048:9: error: ‘pdu’ undeclared (first use in this function)
5048 | memset(pdu, 0, sizeof(*pdu));
| ^~~
net/bluetooth/l2cap_core.c:5048:9: note: each undeclared identifier is reported only once for each function it appears in
net/bluetooth/l2cap_core.c: In function ‘l2cap_ecred_reconfigure’:
net/bluetooth/l2cap_core.c:7114:18: error: expected expression before ‘struct’
7114 | DEFINE_RAW_FLEX(struct l2cap_ecred_reconf_req, pdu, scid, 1);
| ^~~~~~
net/bluetooth/l2cap_core.c:7116:2: error: ‘pdu’ undeclared (first use in this function)
7116 | pdu->mtu = cpu_to_le16(chan->imtu);
| ^~~
cc1: some warnings being treated as errors
make[4]: *** [scripts/Makefile.build:243: net/bluetooth/l2cap_core.o] Error 1
make[3]: *** [scripts/Makefile.build:481: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:481: net] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/github/workspace/src/src/Makefile:1921: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2
##############################
Test: CheckSparse - FAIL
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/af_bluetooth.c:223:25: warning: context imbalance in 'bt_accept_enqueue' - different lock contexts for basic block
drivers/bluetooth/hci_ag6xx.c:257:24: warning: restricted __le32 degrades to integer
drivers/bluetooth/hci_mrvl.c:170:23: warning: restricted __le16 degrades to integer
drivers/bluetooth/hci_mrvl.c:203:23: warning: restricted __le16 degrades to integer
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
./include/net/bluetooth/hci.h:2691:47: warning: array of flexible structures
./include/net/bluetooth/hci.h:2777:43: warning: array of flexible structures
net/bluetooth/l2cap_core.c: In function ‘l2cap_ecred_conn_req’:
net/bluetooth/l2cap_core.c:5000:2: error: implicit declaration of function ‘DEFINE_RAW_FLEX’; did you mean ‘DEFINE_FLEX’? [-Werror=implicit-function-declaration]
5000 | DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
| ^~~~~~~~~~~~~~~
| DEFINE_FLEX
net/bluetooth/l2cap_core.c:5000:18: error: expected expression before ‘struct’
5000 | DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
| ^~~~~~
net/bluetooth/l2cap_core.c:5048:9: error: ‘pdu’ undeclared (first use in this function)
5048 | memset(pdu, 0, sizeof(*pdu));
| ^~~
net/bluetooth/l2cap_core.c:5048:9: note: each undeclared identifier is reported only once for each function it appears in
net/bluetooth/l2cap_core.c: In function ‘l2cap_ecred_reconfigure’:
net/bluetooth/l2cap_core.c:7114:18: error: expected expression before ‘struct’
7114 | DEFINE_RAW_FLEX(struct l2cap_ecred_reconf_req, pdu, scid, 1);
| ^~~~~~
net/bluetooth/l2cap_core.c:7116:2: error: ‘pdu’ undeclared (first use in this function)
7116 | pdu->mtu = cpu_to_le16(chan->imtu);
| ^~~
cc1: some warnings being treated as errors
make[4]: *** [scripts/Makefile.build:243: net/bluetooth/l2cap_core.o] Error 1
make[3]: *** [scripts/Makefile.build:481: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:481: net] Error 2
make[2]: *** Waiting for unfinished jobs....
drivers/bluetooth/hci_nokia.c:284:23: warning: incorrect type in assignment (different base types)
drivers/bluetooth/hci_nokia.c:284:23: expected unsigned short [usertype] baud
drivers/bluetooth/hci_nokia.c:284:23: got restricted __le16 [usertype]
drivers/bluetooth/hci_nokia.c:287:26: warning: incorrect type in assignment (different base types)
drivers/bluetooth/hci_nokia.c:287:26: expected unsigned short [usertype] sys_clk
drivers/bluetooth/hci_nokia.c:287:26: got restricted __le16 [usertype]
make[1]: *** [/github/workspace/src/src/Makefile:1921: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
./include/net/bluetooth/hci.h:2691:47: warning: array of flexible structures
./include/net/bluetooth/hci.h:2777:43: warning: array of flexible structures
net/bluetooth/l2cap_core.c: In function ‘l2cap_ecred_conn_req’:
net/bluetooth/l2cap_core.c:5000:2: error: implicit declaration of function ‘DEFINE_RAW_FLEX’; did you mean ‘DEFINE_FLEX’? [-Werror=implicit-function-declaration]
5000 | DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
| ^~~~~~~~~~~~~~~
| DEFINE_FLEX
net/bluetooth/l2cap_core.c:5000:18: error: expected expression before ‘struct’
5000 | DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
| ^~~~~~
net/bluetooth/l2cap_core.c:5048:9: error: ‘pdu’ undeclared (first use in this function)
5048 | memset(pdu, 0, sizeof(*pdu));
| ^~~
net/bluetooth/l2cap_core.c:5048:9: note: each undeclared identifier is reported only once for each function it appears in
net/bluetooth/l2cap_core.c: In function ‘l2cap_ecred_reconfigure’:
net/bluetooth/l2cap_core.c:7114:18: error: expected expression before ‘struct’
7114 | DEFINE_RAW_FLEX(struct l2cap_ecred_reconf_req, pdu, scid, 1);
| ^~~~~~
net/bluetooth/l2cap_core.c:7116:2: error: ‘pdu’ undeclared (first use in this function)
7116 | pdu->mtu = cpu_to_le16(chan->imtu);
| ^~~
cc1: some warnings being treated as errors
make[4]: *** [scripts/Makefile.build:243: net/bluetooth/l2cap_core.o] Error 1
make[3]: *** [scripts/Makefile.build:481: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:481: net] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/github/workspace/src/src/Makefile:1921: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2
##############################
Test: BuildKernel32 - FAIL
Desc: Build 32bit Kernel for Bluetooth
Output:
net/bluetooth/l2cap_core.c: In function ‘l2cap_ecred_conn_req’:
net/bluetooth/l2cap_core.c:5000:2: error: implicit declaration of function ‘DEFINE_RAW_FLEX’; did you mean ‘DEFINE_FLEX’? [-Werror=implicit-function-declaration]
5000 | DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
| ^~~~~~~~~~~~~~~
| DEFINE_FLEX
net/bluetooth/l2cap_core.c:5000:18: error: expected expression before ‘struct’
5000 | DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
| ^~~~~~
In file included from ./arch/x86/include/asm/string.h:3,
from ./arch/x86/include/asm/cpuid.h:9,
from ./arch/x86/include/asm/processor.h:19,
from ./arch/x86/include/asm/timex.h:5,
from ./include/linux/timex.h:67,
from ./include/linux/time32.h:13,
from ./include/linux/time.h:60,
from ./include/linux/stat.h:19,
from ./include/linux/module.h:13,
from net/bluetooth/l2cap_core.c:31:
net/bluetooth/l2cap_core.c:5048:9: error: ‘pdu’ undeclared (first use in this function)
5048 | memset(pdu, 0, sizeof(*pdu));
| ^~~
./arch/x86/include/asm/string_32.h:195:46: note: in definition of macro ‘memset’
195 | #define memset(s, c, count) __builtin_memset(s, c, count)
| ^
net/bluetooth/l2cap_core.c:5048:9: note: each undeclared identifier is reported only once for each function it appears in
5048 | memset(pdu, 0, sizeof(*pdu));
| ^~~
./arch/x86/include/asm/string_32.h:195:46: note: in definition of macro ‘memset’
195 | #define memset(s, c, count) __builtin_memset(s, c, count)
| ^
net/bluetooth/l2cap_core.c: In function ‘l2cap_ecred_reconfigure’:
net/bluetooth/l2cap_core.c:7114:18: error: expected expression before ‘struct’
7114 | DEFINE_RAW_FLEX(struct l2cap_ecred_reconf_req, pdu, scid, 1);
| ^~~~~~
net/bluetooth/l2cap_core.c:7116:2: error: ‘pdu’ undeclared (first use in this function)
7116 | pdu->mtu = cpu_to_le16(chan->imtu);
| ^~~
cc1: some warnings being treated as errors
make[4]: *** [scripts/Makefile.build:243: net/bluetooth/l2cap_core.o] Error 1
make[3]: *** [scripts/Makefile.build:481: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:481: net] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/github/workspace/src/src/Makefile:1921: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2
##############################
Test: TestRunnerSetup - FAIL
Desc: Setup kernel and bluez for test-runner
Output:
Kernel:
net/bluetooth/l2cap_core.c: In function ‘l2cap_ecred_conn_req’:
net/bluetooth/l2cap_core.c:5000:2: error: implicit declaration of function ‘DEFINE_RAW_FLEX’; did you mean ‘DEFINE_FLEX’? [-Werror=implicit-function-declaration]
5000 | DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
| ^~~~~~~~~~~~~~~
| DEFINE_FLEX
net/bluetooth/l2cap_core.c:5000:18: error: expected expression before ‘struct’
5000 | DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
| ^~~~~~
net/bluetooth/l2cap_core.c:5048:9: error: ‘pdu’ undeclared (first use in this function)
5048 | memset(pdu, 0, sizeof(*pdu));
| ^~~
net/bluetooth/l2cap_core.c:5048:9: note: each undeclared identifier is reported only once for each function it appears in
net/bluetooth/l2cap_core.c: In function ‘l2cap_ecred_reconfigure’:
net/bluetooth/l2cap_core.c:7114:18: error: expected expression before ‘struct’
7114 | DEFINE_RAW_FLEX(struct l2cap_ecred_reconf_req, pdu, scid, 1);
| ^~~~~~
net/bluetooth/l2cap_core.c:7116:2: error: ‘pdu’ undeclared (first use in this function)
7116 | pdu->mtu = cpu_to_le16(chan->imtu);
| ^~~
cc1: some warnings being treated as errors
make[4]: *** [scripts/Makefile.build:243: net/bluetooth/l2cap_core.o] Error 1
make[4]: *** Waiting for unfinished jobs....
make[3]: *** [scripts/Makefile.build:481: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:481: net] Error 2
make[1]: *** [/github/workspace/src/src/Makefile:1921: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2
##############################
Test: TestRunner_l2cap-tester - FAIL
Desc: Run l2cap-tester with test-runner
Output:
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_bnep-tester - FAIL
Desc: Run bnep-tester with test-runner
Output:
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_rfcomm-tester - FAIL
Desc: Run rfcomm-tester with test-runner
Output:
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_sco-tester - FAIL
Desc: Run sco-tester with test-runner
Output:
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_ioctl-tester - FAIL
Desc: Run ioctl-tester with test-runner
Output:
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_smp-tester - FAIL
Desc: Run smp-tester with test-runner
Output:
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: TestRunner_userchan-tester - FAIL
Desc: Run userchan-tester with test-runner
Output:
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
qemu: could not open kernel file '/github/workspace/src/src/arch/x86/boot/bzImage': No such file or directory
##############################
Test: IncrementalBuild - FAIL
Desc: Incremental build with the patches in the series
Output:
[next] Bluetooth: L2CAP: Avoid -Wflex-array-member-not-at-end warnings
net/bluetooth/l2cap_core.c: In function ‘l2cap_ecred_conn_req’:
net/bluetooth/l2cap_core.c:5000:2: error: implicit declaration of function ‘DEFINE_RAW_FLEX’; did you mean ‘DEFINE_FLEX’? [-Werror=implicit-function-declaration]
5000 | DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
| ^~~~~~~~~~~~~~~
| DEFINE_FLEX
net/bluetooth/l2cap_core.c:5000:18: error: expected expression before ‘struct’
5000 | DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
| ^~~~~~
net/bluetooth/l2cap_core.c:5048:9: error: ‘pdu’ undeclared (first use in this function)
5048 | memset(pdu, 0, sizeof(*pdu));
| ^~~
net/bluetooth/l2cap_core.c:5048:9: note: each undeclared identifier is reported only once for each function it appears in
net/bluetooth/l2cap_core.c: In function ‘l2cap_ecred_reconfigure’:
net/bluetooth/l2cap_core.c:7114:18: error: expected expression before ‘struct’
7114 | DEFINE_RAW_FLEX(struct l2cap_ecred_reconf_req, pdu, scid, 1);
| ^~~~~~
net/bluetooth/l2cap_core.c:7116:2: error: ‘pdu’ undeclared (first use in this function)
7116 | pdu->mtu = cpu_to_le16(chan->imtu);
| ^~~
cc1: some warnings being treated as errors
make[4]: *** [scripts/Makefile.build:243: net/bluetooth/l2cap_core.o] Error 1
make[3]: *** [scripts/Makefile.build:481: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:481: net] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/github/workspace/src/src/Makefile:1921: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2
---
Regards,
Linux Bluetooth
On 3/26/24 15:12, Luiz Augusto von Dentz wrote:
> Hi Gustavo,
>
> On Tue, Mar 26, 2024 at 4:02 PM Gustavo A. R. Silva
> <[email protected]> wrote:
>>
>> -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
>> ready to enable it globally.
>>
>> There are currently a couple of objects (`req` and `rsp`), in a couple
>> of structures, that contain flexible structures (`struct l2cap_ecred_conn_req`
>> and `struct l2cap_ecred_conn_rsp`), for example:
>>
>> struct l2cap_ecred_rsp_data {
>> struct {
>> struct l2cap_ecred_conn_rsp rsp;
>> __le16 scid[L2CAP_ECRED_MAX_CID];
>> } __packed pdu;
>> int count;
>> };
>>
>> in the struct above, `struct l2cap_ecred_conn_rsp` is a flexible
>> structure:
>>
>> struct l2cap_ecred_conn_rsp {
>> __le16 mtu;
>> __le16 mps;
>> __le16 credits;
>> __le16 result;
>> __le16 dcid[];
>> };
>>
>> So, in order to avoid ending up with a flexible-array member in the
>> middle of another structure, we use the `struct_group_tagged()` (and
>> `__struct_group()` when the flexible structure is `__packed`) helper
>> to separate the flexible array from the rest of the members in the
>> flexible structure:
>>
>> struct l2cap_ecred_conn_rsp {
>> struct_group_tagged(l2cap_ecred_conn_rsp_hdr, hdr,
>>
>> ... the rest of members
>>
>> );
>> __le16 dcid[];
>> };
>
> Wouldn't it be better, more readable at least, to not define the
> l2cap_ecred_conn_rsp_hdr inside thought? Instead just do:
>
> struct_group_tagged(l2cap_ecred_conn_rsp_hdr, hdr,
> ...
> };
>
> struct l2cap_ecred_conn_rsp {
> struct l2cap_ecred_conn_rsp_hdr hdr;
> __le16 dcid[];
> };
>
> Or was this done this way in order to maintain the same fields?
Exactly. But I can change it if people consider that's better.
>
>> With the change described above, we now declare objects of the type of
>> the tagged struct, in this example `struct l2cap_ecred_conn_rsp_hdr`,
>> without embedding flexible arrays in the middle of other structures:
>>
>> struct l2cap_ecred_rsp_data {
>> struct {
>> struct l2cap_ecred_conn_rsp_hdr rsp;
>> __le16 scid[L2CAP_ECRED_MAX_CID];
>> } __packed pdu;
>> int count;
>> };
>>
>> Also, when the flexible-array member needs to be accessed, we use
>> `container_of()` to retrieve a pointer to the flexible structure.
>>
>> We also use the `DEFINE_RAW_FLEX()` helper for a couple of on-stack
>> definitions of a flexible structure where the size of the flexible-array
>> member is known at compile-time.
>>
>> So, with these changes, fix the following warnings:
>> net/bluetooth/l2cap_core.c:1260:45: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>> net/bluetooth/l2cap_core.c:3740:45: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>> net/bluetooth/l2cap_core.c:4999:45: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>> net/bluetooth/l2cap_core.c:7116:47: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>>
>> Link: https://github.com/KSPP/linux/issues/202
>> Signed-off-by: Gustavo A. R. Silva <[email protected]>
>> ---
>>
>> Hi!
>>
>> I wonder if `struct l2cap_ecred_conn_rsp` should also be `__packed`.
>>
>> Thanks
>> - Gustavo
>>
>> include/net/bluetooth/l2cap.h | 20 +++++++++------
>> net/bluetooth/l2cap_core.c | 46 ++++++++++++++++-------------------
>> 2 files changed, 33 insertions(+), 33 deletions(-)
>>
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index a4278aa618ab..92a143517d83 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -463,18 +463,22 @@ struct l2cap_le_credits {
>> #define L2CAP_ECRED_MAX_CID 5
>>
>> struct l2cap_ecred_conn_req {
>> - __le16 psm;
>> - __le16 mtu;
>> - __le16 mps;
>> - __le16 credits;
>> + __struct_group(l2cap_ecred_conn_req_hdr, hdr, __packed,
>> + __le16 psm;
>> + __le16 mtu;
>> + __le16 mps;
>> + __le16 credits;
>> + );
>> __le16 scid[];
>> } __packed;
>>
>> struct l2cap_ecred_conn_rsp {
>> - __le16 mtu;
>> - __le16 mps;
>> - __le16 credits;
>> - __le16 result;
>> + struct_group_tagged(l2cap_ecred_conn_rsp_hdr, hdr,
>> + __le16 mtu;
>> + __le16 mps;
>> + __le16 credits;
>> + __le16 result;
>> + );
>> __le16 dcid[];
>> };
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index 467b242d8be0..bf087eca489e 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -1257,7 +1257,7 @@ static void l2cap_le_connect(struct l2cap_chan *chan)
>>
>> struct l2cap_ecred_conn_data {
>> struct {
>> - struct l2cap_ecred_conn_req req;
>> + struct l2cap_ecred_conn_req_hdr req;
>> __le16 scid[5];
>> } __packed pdu;
>
> Can't we just use DEFINE_RAW_FLEX for the pdu field above?
No; DEFINE_FLEX() and DEFINE_RAW_FLEX() are for on-stack code only.
Thanks
--
Gustavo
>
>> struct l2cap_chan *chan;
>> @@ -3737,7 +3737,7 @@ static void l2cap_ecred_list_defer(struct l2cap_chan *chan, void *data)
>>
>> struct l2cap_ecred_rsp_data {
>> struct {
>> - struct l2cap_ecred_conn_rsp rsp;
>> + struct l2cap_ecred_conn_rsp_hdr rsp;
>> __le16 scid[L2CAP_ECRED_MAX_CID];
>> } __packed pdu;
>
> Ditto.
>
>> int count;
>> @@ -3746,6 +3746,8 @@ struct l2cap_ecred_rsp_data {
>> static void l2cap_ecred_rsp_defer(struct l2cap_chan *chan, void *data)
>> {
>> struct l2cap_ecred_rsp_data *rsp = data;
>> + struct l2cap_ecred_conn_rsp *rsp_flex =
>> + container_of(&rsp->pdu.rsp, struct l2cap_ecred_conn_rsp, hdr);
>>
>> if (test_bit(FLAG_ECRED_CONN_REQ_SENT, &chan->flags))
>> return;
>> @@ -3755,7 +3757,7 @@ static void l2cap_ecred_rsp_defer(struct l2cap_chan *chan, void *data)
>>
>> /* Include all channels pending with the same ident */
>> if (!rsp->pdu.rsp.result)
>> - rsp->pdu.rsp.dcid[rsp->count++] = cpu_to_le16(chan->scid);
>> + rsp_flex->dcid[rsp->count++] = cpu_to_le16(chan->scid);
>> else
>> l2cap_chan_del(chan, ECONNRESET);
>> }
>> @@ -4995,10 +4997,7 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
>> u8 *data)
>> {
>> struct l2cap_ecred_conn_req *req = (void *) data;
>> - struct {
>> - struct l2cap_ecred_conn_rsp rsp;
>> - __le16 dcid[L2CAP_ECRED_MAX_CID];
>> - } __packed pdu;
>> + DEFINE_RAW_FLEX(struct l2cap_ecred_conn_rsp, pdu, dcid, L2CAP_ECRED_MAX_CID);
>> struct l2cap_chan *chan, *pchan;
>> u16 mtu, mps;
>> __le16 psm;
>> @@ -5017,7 +5016,7 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
>> cmd_len -= sizeof(*req);
>> num_scid = cmd_len / sizeof(u16);
>>
>> - if (num_scid > ARRAY_SIZE(pdu.dcid)) {
>> + if (num_scid > L2CAP_ECRED_MAX_CID) {
>> result = L2CAP_CR_LE_INVALID_PARAMS;
>> goto response;
>> }
>> @@ -5046,7 +5045,7 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
>>
>> BT_DBG("psm 0x%2.2x mtu %u mps %u", __le16_to_cpu(psm), mtu, mps);
>>
>> - memset(&pdu, 0, sizeof(pdu));
>> + memset(pdu, 0, sizeof(*pdu));
>>
>> /* Check if we have socket listening on psm */
>> pchan = l2cap_global_chan_by_psm(BT_LISTEN, psm, &conn->hcon->src,
>> @@ -5072,8 +5071,8 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
>>
>> BT_DBG("scid[%d] 0x%4.4x", i, scid);
>>
>> - pdu.dcid[i] = 0x0000;
>> - len += sizeof(*pdu.dcid);
>> + pdu->dcid[i] = 0x0000;
>> + len += sizeof(*pdu->dcid);
>>
>> /* Check for valid dynamic CID range */
>> if (scid < L2CAP_CID_DYN_START || scid > L2CAP_CID_LE_DYN_END) {
>> @@ -5107,13 +5106,13 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
>> l2cap_ecred_init(chan, __le16_to_cpu(req->credits));
>>
>> /* Init response */
>> - if (!pdu.rsp.credits) {
>> - pdu.rsp.mtu = cpu_to_le16(chan->imtu);
>> - pdu.rsp.mps = cpu_to_le16(chan->mps);
>> - pdu.rsp.credits = cpu_to_le16(chan->rx_credits);
>> + if (!pdu->credits) {
>> + pdu->mtu = cpu_to_le16(chan->imtu);
>> + pdu->mps = cpu_to_le16(chan->mps);
>> + pdu->credits = cpu_to_le16(chan->rx_credits);
>> }
>>
>> - pdu.dcid[i] = cpu_to_le16(chan->scid);
>> + pdu->dcid[i] = cpu_to_le16(chan->scid);
>>
>> __set_chan_timer(chan, chan->ops->get_sndtimeo(chan));
>>
>> @@ -5135,13 +5134,13 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
>> l2cap_chan_put(pchan);
>>
>> response:
>> - pdu.rsp.result = cpu_to_le16(result);
>> + pdu->result = cpu_to_le16(result);
>>
>> if (defer)
>> return 0;
>>
>> l2cap_send_cmd(conn, cmd->ident, L2CAP_ECRED_CONN_RSP,
>> - sizeof(pdu.rsp) + len, &pdu);
>> + sizeof(*pdu) + len, pdu);
>>
>> return 0;
>> }
>> @@ -7112,14 +7111,11 @@ EXPORT_SYMBOL_GPL(l2cap_chan_connect);
>> static void l2cap_ecred_reconfigure(struct l2cap_chan *chan)
>> {
>> struct l2cap_conn *conn = chan->conn;
>> - struct {
>> - struct l2cap_ecred_reconf_req req;
>> - __le16 scid;
>> - } pdu;
>> + DEFINE_RAW_FLEX(struct l2cap_ecred_reconf_req, pdu, scid, 1);
>>
>> - pdu.req.mtu = cpu_to_le16(chan->imtu);
>> - pdu.req.mps = cpu_to_le16(chan->mps);
>> - pdu.scid = cpu_to_le16(chan->scid);
>> + pdu->mtu = cpu_to_le16(chan->imtu);
>> + pdu->mps = cpu_to_le16(chan->mps);
>> + pdu->scid[0] = cpu_to_le16(chan->scid);
>>
>> chan->ident = l2cap_get_ident(conn);
>>
>> --
>> 2.34.1
>>
>
>