wireless: realtek: Replace zero-length array with flexible-array member
Subsystems (5)
PATCH 1/1 wireless: realtek: Replace zero-length array with flexible-array member
The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member[1][2], introduced in C99:
struct foo {
int stuff;
struct boo array[];
};By making use of the mechanism above, we will get a compiler warning in case the flexible array does not occur last in the structure, which will help us prevent some kind of undefined behavior bugs from being inadvertently introduced[3] to the codebase from now on.
Also, notice that, dynamic memory allocations won't be affected by this change:
"Flexible array members have incomplete type, and so the sizeof operator may not be applied. As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."[1]
This issue was found with the help of Coccinelle.
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
--- drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h | 2 +- drivers/net/wireless/realtek/rtlwifi/wifi.h | 6 +++--- drivers/net/wireless/realtek/rtw88/fw.h | 2 +- drivers/net/wireless/realtek/rtw88/main.h | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-)
drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h +1 -1
@@ -627,7 +627,7 @@ struct rtl8xxxu_firmware_header {u32 reserved4;u32 reserved5; -u8 data[0 ]; +u8 data[]; };/*
drivers/net/wireless/realtek/rtlwifi/wifi.h +3 -3
@@ -1051,13 +1051,13 @@ struct rtl_hdr_3addr {u8 addr2[ETH_ALEN ];u8 addr3[ETH_ALEN ];__le16 seq_ctl; -u8 payload[0 ]; +u8 payload[]; } __packed;struct rtl_info_element {u8 id;u8 len; -u8 data[0 ]; +u8 data[]; } __packed;struct rtl_probe_rsp {
@@ -1068,7 +1068,7 @@ struct rtl_probe_rsp {/*SSID, supported rates, FH params, DS params, * CF params, IBSS params, TIM (if beacon), RSN */ -struct rtl_info_element info_element[0 ]; +struct rtl_info_element info_element[]; } __packed;/*LED related.*/
drivers/net/wireless/realtek/rtw88/fw.h +1 -1
@@ -36,7 +36,7 @@ enum rtw_c2h_cmd_id_ext {struct rtw_c2h_cmd {u8 id;u8 seq; -u8 payload[0 ]; +u8 payload[]; } __packed;enum rtw_rsvd_packet_type {
drivers/net/wireless/realtek/rtw88/main.h +1 -1
@@ -1641,7 +1641,7 @@ struct rtw_dev {struct rtw_wow_param wow;/* hci related data, must be last */ -u8 priv[0 ]__aligned(sizeof(void *)) ; +u8 priv[]__aligned(sizeof(void *)) ; };#include "hci.h"
Signature · 1 line
2.25.0
Quoted 15 lines · Gustavo A. R. Silva
The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member[1][2], introduced in C99:
struct foo { int stuff; struct boo array[]; };By making use of the mechanism above, we will get a compiler warning in case the flexible array does not occur last in the structure, which will help us prevent some kind of undefined behavior bugs from being inadvertently introduced[3] to the codebase from now on.
Quoted 8 lines
Also, notice that, dynamic memory allocations won't be affected by this change:
"Flexible array members have incomplete type, and so the sizeof operator may not be applied. As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."[1]
This issue was found with the help of Coccinelle.
Hi Gustavo,
I really don't think this improves the code in any way for the drivers you are modifying. If we really want to address this corner case, it seems like fixing the compiler to address [0] arrays the same as [] arrays is the right solution.
Cheers,
Jes
Quoted 10 lines · Gustavo A. R. Silva
The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member[1][2], introduced in C99:
struct foo { int stuff; struct boo array[]; };
Quoted 4 lines
By making use of the mechanism above, we will get a compiler warning in case the flexible array does not occur last in the structure, which will help us prevent some kind of undefined behavior bugs from being inadvertently introduced[3] to the codebase from now on.
Preferred by who exactly?
Signature · 1 line
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Quoted 22 lines · Gustavo A. R. Silva
The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member[1][2], introduced in C99:
struct foo { int stuff; struct boo array[]; };By making use of the mechanism above, we will get a compiler warning in case the flexible array does not occur last in the structure, which will help us prevent some kind of undefined behavior bugs from being inadvertently introduced[3] to the codebase from now on.
Also, notice that, dynamic memory allocations won't be affected by this change:
"Flexible array members have incomplete type, and so the sizeof operator may not be applied. As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."[1]
Quoted 7 lines
This issue was found with the help of Coccinelle.
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit76497732932f("cxgb3/l2t: Fix undefined behaviour")Signed-off-by Gustavo A. R. Silva <[email protected]>
Patch applied to wireless-drivers-next.git, thanks.
a1b7714b72fd wireless: realtek: Replace zero-length array with flexible-array member
Signature · 3 lines
https://patchwork.kernel.org/patch/11402315/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches