netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH][next] wireless: realtek: Replace zero-length array with flexible-array member
@ 2020-02-25  0:27 Gustavo A. R. Silva
  2020-02-25 16:14 ` Jes Sorensen
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Gustavo A. R. Silva @ 2020-02-25  0:27 UTC (permalink / raw)
  To: Jes Sorensen, Kalle Valo, David S. Miller, Ping-Ke Shih,
	Yan-Hsuan Chuang
  Cc: linux-wireless, netdev, linux-kernel, 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]

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

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 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(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
index 6598c8d786ea..440d164443bc 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
@@ -627,7 +627,7 @@ struct rtl8xxxu_firmware_header {
 	u32	reserved4;
 	u32	reserved5;
 
-	u8	data[0];
+	u8	data[];
 };
 
 /*
diff --git a/drivers/net/wireless/realtek/rtlwifi/wifi.h b/drivers/net/wireless/realtek/rtlwifi/wifi.h
index 1cff9f07c9e9..13421cf2d201 100644
--- a/drivers/net/wireless/realtek/rtlwifi/wifi.h
+++ b/drivers/net/wireless/realtek/rtlwifi/wifi.h
@@ -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.*/
diff --git a/drivers/net/wireless/realtek/rtw88/fw.h b/drivers/net/wireless/realtek/rtw88/fw.h
index ccd27bd45775..414827800a5f 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.h
+++ b/drivers/net/wireless/realtek/rtw88/fw.h
@@ -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 {
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index c074cef22120..46c0ebceb177 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -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"
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH][next] wireless: realtek: Replace zero-length array with flexible-array member
  2020-02-25  0:27 [PATCH][next] wireless: realtek: Replace zero-length array with flexible-array member Gustavo A. R. Silva
@ 2020-02-25 16:14 ` Jes Sorensen
  2020-03-02 13:28 ` Kalle Valo
  2020-03-23 16:54 ` Kalle Valo
  2 siblings, 0 replies; 4+ messages in thread
From: Jes Sorensen @ 2020-02-25 16:14 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Kalle Valo, David S. Miller, Ping-Ke Shih,
	Yan-Hsuan Chuang
  Cc: linux-wireless, netdev, linux-kernel

On 2/24/20 7:27 PM, Gustavo A. R. Silva wrote:
> 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.

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH][next] wireless: realtek: Replace zero-length array with flexible-array member
  2020-02-25  0:27 [PATCH][next] wireless: realtek: Replace zero-length array with flexible-array member Gustavo A. R. Silva
  2020-02-25 16:14 ` Jes Sorensen
@ 2020-03-02 13:28 ` Kalle Valo
  2020-03-23 16:54 ` Kalle Valo
  2 siblings, 0 replies; 4+ messages in thread
From: Kalle Valo @ 2020-03-02 13:28 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Jes Sorensen, David S. Miller, Ping-Ke Shih, Yan-Hsuan Chuang,
	linux-wireless, netdev, linux-kernel

"Gustavo A. R. Silva" <gustavo@embeddedor.com> writes:

> 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.

Preferred by who exactly?

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH][next] wireless: realtek: Replace zero-length array with flexible-array member
  2020-02-25  0:27 [PATCH][next] wireless: realtek: Replace zero-length array with flexible-array member Gustavo A. R. Silva
  2020-02-25 16:14 ` Jes Sorensen
  2020-03-02 13:28 ` Kalle Valo
@ 2020-03-23 16:54 ` Kalle Valo
  2 siblings, 0 replies; 4+ messages in thread
From: Kalle Valo @ 2020-03-23 16:54 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Jes Sorensen, David S. Miller, Ping-Ke Shih, Yan-Hsuan Chuang,
	linux-wireless, netdev, linux-kernel, Gustavo A. R. Silva

"Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:

> 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")
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Patch applied to wireless-drivers-next.git, thanks.

a1b7714b72fd wireless: realtek: Replace zero-length array with flexible-array member

-- 
https://patchwork.kernel.org/patch/11402315/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-03-23 16:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-25  0:27 [PATCH][next] wireless: realtek: Replace zero-length array with flexible-array member Gustavo A. R. Silva
2020-02-25 16:14 ` Jes Sorensen
2020-03-02 13:28 ` Kalle Valo
2020-03-23 16:54 ` Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).