Re: [PATCH] staging: Replace zero-length array with flexible-array member
Hi,
On 2/20/20 13:04, adham.aboza...@microchip.com wrote:
> Hi Gustavo
>
> On 2/20/20 6:29 AM, 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.
>>
>> [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
>>
>> static void cfg_scan_result(enum scan_event scan_event,
>> diff --git a/drivers/staging/wilc1000/spi.c b/drivers/staging/wilc1000/spi.c
>> index 44f7d48851b5..11653ac118cd 100644
>> --- a/drivers/staging/wilc1000/spi.c
>> +++ b/drivers/staging/wilc1000/spi.c
>> @@ -139,7 +139,7 @@ struct wilc_spi_read_rsp_data {
>> u8 status;
>> u8 resp_header;
>> u8 resp_data[4];
>> - u8 crc[0];
>> + u8 crc[];
>> } __packed;
> more zero-length arrays in wilc1000, spi.c, struct wilc_spi_cmd, and in fw.h
>
Oh wow, I hadn't thought about cases like this:
struct wilc_spi_cmd {
u8 cmd_type;
union {
struct {
u8 addr[3];
u8 crc[0];
} __packed simple_cmd;
struct {
u8 addr[3];
u8 size[2];
u8 crc[0];
} __packed dma_cmd;
struct {
u8 addr[3];
u8 size[3];
u8 crc[0];
} __packed dma_cmd_ext;
struct {
u8 addr[2];
__be32 data;
u8 crc[0];
} __packed internal_w_cmd;
struct {
u8 addr[3];
__be32 data;
u8 crc[0];
} __packed w_cmd;
} u;
} __packed;
Thanks for the feedback.
--
Gustavo
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Replace zero-length array with flexible-array member
Hi Gustavo
On 2/20/20 6:29 AM, 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.
>
> [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
>
> static void cfg_scan_result(enum scan_event scan_event,
> diff --git a/drivers/staging/wilc1000/spi.c b/drivers/staging/wilc1000/spi.c
> index 44f7d48851b5..11653ac118cd 100644
> --- a/drivers/staging/wilc1000/spi.c
> +++ b/drivers/staging/wilc1000/spi.c
> @@ -139,7 +139,7 @@ struct wilc_spi_read_rsp_data {
> u8 status;
> u8 resp_header;
> u8 resp_data[4];
> - u8 crc[0];
> + u8 crc[];
> } __packed;
more zero-length arrays in wilc1000, spi.c, struct wilc_spi_cmd, and in fw.h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: 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")
Signed-off-by: Gustavo A. R. Silva
---
drivers/staging/gdm724x/gdm_mux.h | 2 +-
drivers/staging/gdm724x/hci_packet.h | 6 ++--
drivers/staging/greybus/audio_apbridgea.h | 2 +-
drivers/staging/ks7010/ks_hostif.h| 4 +--
.../staging/media/allegro-dvt/allegro-core.c | 2 +-
drivers/staging/octeon-usb/octeon-hcd.c | 2 +-
drivers/staging/rtl8192e/rtllib.h | 30 +--
.../staging/rtl8192u/ieee80211/ieee80211.h| 28 -
drivers/staging/rtl8712/ieee80211.h | 4 +--
drivers/staging/rtl8712/rtl871x_mp_ioctl.h| 4 +--
drivers/staging/wilc1000/cfg80211.c | 10 +++
drivers/staging/wilc1000/spi.c| 2 +-
drivers/staging/wlan-ng/hfa384x.h | 4 +--
drivers/staging/wlan-ng/p80211types.h | 4 +--
14 files changed, 52 insertions(+), 52 deletions(-)
diff --git a/drivers/staging/gdm724x/gdm_mux.h
b/drivers/staging/gdm724x/gdm_mux.h
index 51c22e3d8aeb..87b8d921fdc8 100644
--- a/drivers/staging/gdm724x/gdm_mux.h
+++ b/drivers/staging/gdm724x/gdm_mux.h
@@ -29,7 +29,7 @@ struct mux_pkt_header {
__le32 seq_num;
__le32 payload_size;
__le16 packet_type;
- unsigned char data[0];
+ unsigned char data[];
};
struct mux_tx {
diff --git a/drivers/staging/gdm724x/hci_packet.h
b/drivers/staging/gdm724x/hci_packet.h
index 6dea3694afdd..faecdfbc664f 100644
--- a/drivers/staging/gdm724x/hci_packet.h
+++ b/drivers/staging/gdm724x/hci_packet.h
@@ -28,7 +28,7 @@
struct hci_packet {
__dev16 cmd_evt;
__dev16 len;
- u8 data[0];
+ u8 data[];
} __packed;
struct tlv {
@@ -51,7 +51,7 @@ struct sdu {
__dev32 dft_eps_ID;
__dev32 bearer_ID;
__dev32 nic_type;
- u8 data[0];
+ u8 data[];
} __packed;
struct multi_sdu {
@@ -59,7 +59,7 @@ struct multi_sdu {
__dev16 len;
__dev16 num_packet;
__dev16 reserved;
- u8 data[0];
+ u8 data[];
} __packed;
struct hci_pdn_table_ind {
diff --git a/drivers/staging/greybus/audio_apbridgea.h
b/drivers/staging/greybus/audio_apbridgea.h
index 3f1f4dd2c61a..efec0f815efd 100644
--- a/drivers/staging/greybus/audio_apbridgea.h
+++ b/drivers/staging/greybus/audio_apbridgea.h
@@ -65,7 +65,7 @@
struct audio_apbridgea_hdr {
__u8type;
__le16 i2s_port;
- __u8data[0];
+ __u8data[];
} __packed;
struct audio_apbridgea_set_config_request {
diff --git a/drivers/staging/ks7010/ks_hostif.h
b/drivers/staging/ks7010/ks_hostif.h
index ca7dc8f5166c..39138191a556 100644
--- a/drivers/staging/ks7010/ks_hostif.h
+++ b/drivers/staging/ks7010/ks_hostif.h
@@ -70,7 +70,7 @@ struct hostif_data_request {
#define TYPE_DATA 0x
#define TYPE_AUTH 0x0001
__le16 reserved;
- u8 data[0];
+ u8 data[];
} __packed;
#define TYPE_PMK1 0x0001
@@ -194,7 +194,7 @@ enum mib_data_type {
struct hostif_mib_value {
__le16 size;
__le16 type;
- u8 body[0];
+ u8 body[];
} __packed;
struct hostif_mib_get_confirm_t {
diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c
b/drivers/staging/media/allegro-dvt/allegro-core.c
index 3be41698df4c..0a09b3622e78 100644
--- a/drivers/staging/media/allegro-dvt/allegro-core.c
+++ b/drivers/staging/media/allegro-dvt/allegro-core.c
@@ -434,7 +434,7 @@ struct mcu_msg_push_buffers_internal_buffer {
struct mcu_msg_push_buffers_internal {
struct mcu_msg_header header;
u32 channel_id;
- struct mcu_msg_push_buffers_internal_buffer buffer[0];
+ struct mcu_msg_push_buffers_internal_buffer buffer[];
} __attribute__ ((__packed__));
struct mcu_msg_put_stream_buffer {
diff --git a/drivers/staging/octeon-usb/octeon-hcd.c
b/drivers/staging/octeon-usb/octeon-hcd.c
index 582c9187559d..61471a19d4e6 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++
