Antw: [EXT] [PATCH] scsi: Replace zero-length array with flexible-array member
Hi! Personally I think variable-sized structures are a bad thing, independent of the syntax being used. 30 years ago saving one indirection would have been an argument for such structures, but nowadays? Regards, Ulrich >>> "Gustavo A. R. Silva" schrieb am 24.02.2020 um >>> 17:14 in Nachricht <32123_1582744382_5E56C33B_32123_188_1_20200224161406.GA21454@embeddedor>: > 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/scsi/fnic/vnic_devcmd.h | 2 +- > drivers/scsi/ipr.h | 6 +++--- > drivers/scsi/isci/sas.h | 2 +- > drivers/scsi/mpt3sas/mpt3sas_scsih.c | 2 +- > drivers/scsi/mvsas/mv_sas.h | 2 +- > drivers/scsi/mvumi.h | 4 ++-- > drivers/scsi/pmcraid.h | 2 +- > drivers/scsi/snic/vnic_devcmd.h | 2 +- > drivers/scsi/stex.c | 2 +- > include/scsi/iscsi_if.h | 10 +- > include/scsi/scsi_bsg_iscsi.h| 2 +- > include/scsi/scsi_device.h | 4 ++-- > include/scsi/scsi_host.h | 2 +- > include/scsi/scsi_ioctl.h| 2 +- > include/scsi/srp.h | 8 > include/uapi/scsi/scsi_bsg_fc.h | 2 +- > 16 files changed, 27 insertions(+), 27 deletions(-) > > diff --git a/drivers/scsi/fnic/vnic_devcmd.h > b/drivers/scsi/fnic/vnic_devcmd.h > index c5dde556dc7c..c20d30e36dfc 100644 > --- a/drivers/scsi/fnic/vnic_devcmd.h > +++ b/drivers/scsi/fnic/vnic_devcmd.h > @@ -442,7 +442,7 @@ struct vnic_devcmd_notify { > struct vnic_devcmd_provinfo { > u8 oui[3]; > u8 type; > - u8 data[0]; > + u8 data[]; > }; > > /* > diff --git a/drivers/scsi/ipr.h b/drivers/scsi/ipr.h > index a67baeb36d1f..fd3929a19ab5 100644 > --- a/drivers/scsi/ipr.h > +++ b/drivers/scsi/ipr.h > @@ -451,12 +451,12 @@ struct ipr_config_table_hdr64 { > > struct ipr_config_table { > struct ipr_config_table_hdr hdr; > - struct ipr_config_table_entry dev[0]; > + struct ipr_config_table_entry dev[]; > }__attribute__((packed, aligned (4))); > > struct ipr_config_table64 { > struct ipr_config_table_hdr64 hdr64; > - struct ipr_config_table_entry64 dev[0]; > + struct ipr_config_table_entry64 dev[]; > }__attribute__((packed, aligned (8))); > > struct ipr_config_table_entry_wrapper { > @@ -792,7 +792,7 @@ struct ipr_mode_page28 { > struct ipr_mode_page_hdr hdr; > u8 num_entries; > u8 entry_length; > - struct ipr_dev_bus_entry bus[0]; > + struct ipr_dev_bus_entry bus[]; > }__attribute__((packed)); > > struct ipr_mode_page24 { > diff --git a/drivers/scsi/isci/sas.h b/drivers/scsi/isci/sas.h > index dc26b4aea99e..15d8f3631ab7 100644 > --- a/drivers/scsi/isci/sas.h > +++ b/drivers/scsi/isci/sas.h > @@ -201,7 +201,7 @@ struct smp_req { > u8 func;/* byte 1 */ > u8 alloc_resp_len; /* byte 2 */ > u8 req_len; /* byte 3 */ > - u8 req_data[0]; > + u8 req_data[]; > } __packed; > > /* > diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c > b/drivers/scsi/mpt3sas/mpt3sas_scsih.c > index c597d544eb39..778d5e6ce385 100644 > --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c > +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c > @@ -207,7 +207,7 @@ struct fw_event_work { > u8 ignore; > u16 event; > struct kref refcount; > - charevent_data[0] __aligned(4); > + charevent_data[] __aligned(4); > }; > > static void fw_event_work_free(struct kref *r) > diff --git a/drivers/scsi/mvsas/mv_sas.h b/drivers/scsi/mvsas/mv_sas.h > index 519edc796691..327fdd5ee962 100644 > --- a/drivers/scsi/mvsas/mv_sas.h > +++ b/drivers/scsi/mvsas/mv_sas.h > @@ -394,7 +394,7 @@ struct mvs_info { > dma_addr_t bulk_buffer_dma1; > #define TRASH_BUCKET_SIZE
[PATCH] scsi: 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/scsi/fnic/vnic_devcmd.h | 2 +- drivers/scsi/ipr.h | 6 +++--- drivers/scsi/isci/sas.h | 2 +- drivers/scsi/mpt3sas/mpt3sas_scsih.c | 2 +- drivers/scsi/mvsas/mv_sas.h | 2 +- drivers/scsi/mvumi.h | 4 ++-- drivers/scsi/pmcraid.h | 2 +- drivers/scsi/snic/vnic_devcmd.h | 2 +- drivers/scsi/stex.c | 2 +- include/scsi/iscsi_if.h | 10 +- include/scsi/scsi_bsg_iscsi.h| 2 +- include/scsi/scsi_device.h | 4 ++-- include/scsi/scsi_host.h | 2 +- include/scsi/scsi_ioctl.h| 2 +- include/scsi/srp.h | 8 include/uapi/scsi/scsi_bsg_fc.h | 2 +- 16 files changed, 27 insertions(+), 27 deletions(-) diff --git a/drivers/scsi/fnic/vnic_devcmd.h b/drivers/scsi/fnic/vnic_devcmd.h index c5dde556dc7c..c20d30e36dfc 100644 --- a/drivers/scsi/fnic/vnic_devcmd.h +++ b/drivers/scsi/fnic/vnic_devcmd.h @@ -442,7 +442,7 @@ struct vnic_devcmd_notify { struct vnic_devcmd_provinfo { u8 oui[3]; u8 type; - u8 data[0]; + u8 data[]; }; /* diff --git a/drivers/scsi/ipr.h b/drivers/scsi/ipr.h index a67baeb36d1f..fd3929a19ab5 100644 --- a/drivers/scsi/ipr.h +++ b/drivers/scsi/ipr.h @@ -451,12 +451,12 @@ struct ipr_config_table_hdr64 { struct ipr_config_table { struct ipr_config_table_hdr hdr; - struct ipr_config_table_entry dev[0]; + struct ipr_config_table_entry dev[]; }__attribute__((packed, aligned (4))); struct ipr_config_table64 { struct ipr_config_table_hdr64 hdr64; - struct ipr_config_table_entry64 dev[0]; + struct ipr_config_table_entry64 dev[]; }__attribute__((packed, aligned (8))); struct ipr_config_table_entry_wrapper { @@ -792,7 +792,7 @@ struct ipr_mode_page28 { struct ipr_mode_page_hdr hdr; u8 num_entries; u8 entry_length; - struct ipr_dev_bus_entry bus[0]; + struct ipr_dev_bus_entry bus[]; }__attribute__((packed)); struct ipr_mode_page24 { diff --git a/drivers/scsi/isci/sas.h b/drivers/scsi/isci/sas.h index dc26b4aea99e..15d8f3631ab7 100644 --- a/drivers/scsi/isci/sas.h +++ b/drivers/scsi/isci/sas.h @@ -201,7 +201,7 @@ struct smp_req { u8 func;/* byte 1 */ u8 alloc_resp_len; /* byte 2 */ u8 req_len; /* byte 3 */ - u8 req_data[0]; + u8 req_data[]; } __packed; /* diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c index c597d544eb39..778d5e6ce385 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c @@ -207,7 +207,7 @@ struct fw_event_work { u8 ignore; u16 event; struct kref refcount; - charevent_data[0] __aligned(4); + charevent_data[] __aligned(4); }; static void fw_event_work_free(struct kref *r) diff --git a/drivers/scsi/mvsas/mv_sas.h b/drivers/scsi/mvsas/mv_sas.h index 519edc796691..327fdd5ee962 100644 --- a/drivers/scsi/mvsas/mv_sas.h +++ b/drivers/scsi/mvsas/mv_sas.h @@ -394,7 +394,7 @@ struct mvs_info { dma_addr_t bulk_buffer_dma1; #define TRASH_BUCKET_SIZE 0x2 void *dma_pool; - struct mvs_slot_info slot_info[0]; + struct mvs_slot_info slot_info[]; }; struct mvs_prv_info{ diff --git a/drivers/scsi/mvumi.h b/drivers/scsi/mvumi.h index ec8cc2207536..60d5691fc4ab 100644 --- a/drivers/scsi/mvumi.h +++ b/drivers/scsi/mvumi.h @@ -130,7 +130,7 @@ enum { struct mvumi_hotplug_event { u16 size; u8 dummy[2]; - u8 bitmap[0]; + u8 bitmap[]; }; struct mvumi_driver_event { @@ -290,7 +290,7 @@ struct mvumi_rsp_frame { struct mvumi_ob_data { struct list_head list; -
Re: [PATCH] scsi: Replace zero-length array with flexible-array member
On 2/24/20 10:30, Lee Duncan wrote: > On 2/24/20 8:14 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 >> --- >> ... >> diff --git a/include/scsi/iscsi_if.h b/include/scsi/iscsi_if.h >> index 92b11c7e0b4f..b0e240b10bf9 100644 >> --- a/include/scsi/iscsi_if.h >> +++ b/include/scsi/iscsi_if.h >> @@ -311,7 +311,7 @@ enum iscsi_param_type { >> struct iscsi_param_info { >> uint32_t len; /* Actual length of the param value */ >> uint16_t param; /* iscsi param */ >> -uint8_t value[0]; /* length sized value follows */ >> +uint8_t value[];/* length sized value follows */ >> } __packed; >> >> struct iscsi_iface_param_info { >> @@ -320,7 +320,7 @@ struct iscsi_iface_param_info { >> uint16_t param; /* iscsi param value */ >> uint8_t iface_type; /* IPv4 or IPv6 */ >> uint8_t param_type; /* iscsi_param_type */ >> -uint8_t value[0]; /* length sized value follows */ >> +uint8_t value[];/* length sized value follows */ >> } __packed; >> >> /* >> @@ -697,7 +697,7 @@ enum iscsi_flashnode_param { >> struct iscsi_flashnode_param_info { >> uint32_t len; /* Actual length of the param */ >> uint16_t param; /* iscsi param value */ >> -uint8_t value[0]; /* length sized value follows */ >> +uint8_t value[];/* length sized value follows */ >> } __packed; >> >> enum iscsi_discovery_parent_type { >> @@ -815,7 +815,7 @@ struct iscsi_stats { >> * up to ISCSI_STATS_CUSTOM_MAX >> */ >> uint32_t custom_length; >> -struct iscsi_stats_custom custom[0] >> +struct iscsi_stats_custom custom[] >> __attribute__ ((aligned (sizeof(uint64_t; >> }; >> >> @@ -946,7 +946,7 @@ struct iscsi_offload_host_stats { >> * up to ISCSI_HOST_STATS_CUSTOM_MAX >> */ >> uint32_t custom_length; >> -struct iscsi_host_stats_custom custom[0] >> +struct iscsi_host_stats_custom custom[] >> __aligned(sizeof(uint64_t)); >> }; >> >> diff --git a/include/scsi/scsi_bsg_iscsi.h b/include/scsi/scsi_bsg_iscsi.h >> index fa0c820a1663..6b8128005af8 100644 >> --- a/include/scsi/scsi_bsg_iscsi.h >> +++ b/include/scsi/scsi_bsg_iscsi.h >> @@ -52,7 +52,7 @@ struct iscsi_bsg_host_vendor { >> uint64_t vendor_id; >> >> /* start of vendor command area */ >> -uint32_t vendor_cmd[0]; >> +uint32_t vendor_cmd[]; >> }; >> >> /* Response: >> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h >> index f8312a3e5b42..4dc158cf09b8 100644 >> --- a/include/scsi/scsi_device.h >> +++ b/include/scsi/scsi_device.h >> @@ -231,7 +231,7 @@ struct scsi_device { >> struct mutexstate_mutex; >> enum scsi_device_state sdev_state; >> struct task_struct *quiesced_by; >> -unsigned long sdev_data[0]; >> +unsigned long sdev_data[]; >> } __attribute__((aligned(sizeof(unsigned long; >> >> #define to_scsi_device(d) \ >> @@ -315,7 +315,7 @@ struct scsi_target { >> charscsi_level; >> enum scsi_target_state state; >> void*hostdata; /* available to low-level driver */ >> -unsigned long starget_data[0]; /* for the transport */ >> +unsigned long starget_data[]; /* for the transport */ >> /* starget_data must be the last element */ >> } __attribute__((aligned(sizeof(unsigned long; >> >> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h >> index 7a97fb8104cf..e6811ea8f984 100644 >> --- a/include/scsi/scsi_host.h >> +++ b/include/scsi/scsi_host.h >> @@ -682,7 +682,7 @@ struct Scsi_Host { >> * and also because some compilers (m68k) don't automatically force >> * alignment to a long boundary. >> */ >> -
Re: [PATCH] scsi: Replace zero-length array with flexible-array member
Reviewed-by: Satish Kharat From: Gustavo A. R. Silva Sent: Monday, February 24, 2020 8:42 AM To: Lee Duncan ; Satish Kharat (satishkh) ; Sesidhar Baddela (sebaddel) ; Karan Tilak Kumar (kartilak) ; James E.J. Bottomley ; Martin K. Petersen ; Brian King ; Intel SCU Linux support ; Artur Paszkiewicz ; Sathya Prakash ; Chaitra P B ; Suganath Prabu Subramani ; Chris Leech ; Bart Van Assche Cc: linux-s...@vger.kernel.org ; linux-ker...@vger.kernel.org ; mpt-fusionlinux@broadcom.com ; open-iscsi@googlegroups.com ; linux-r...@vger.kernel.org Subject: Re: [PATCH] scsi: Replace zero-length array with flexible-array member On 2/24/20 10:30, Lee Duncan wrote: > On 2/24/20 8:14 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 >> --- >> ... >> diff --git a/include/scsi/iscsi_if.h b/include/scsi/iscsi_if.h >> index 92b11c7e0b4f..b0e240b10bf9 100644 >> --- a/include/scsi/iscsi_if.h >> +++ b/include/scsi/iscsi_if.h >> @@ -311,7 +311,7 @@ enum iscsi_param_type { >> struct iscsi_param_info { >> uint32_t len; /* Actual length of the param value */ >> uint16_t param; /* iscsi param */ >> -uint8_t value[0]; /* length sized value follows */ >> +uint8_t value[];/* length sized value follows */ >> } __packed; >> >> struct iscsi_iface_param_info { >> @@ -320,7 +320,7 @@ struct iscsi_iface_param_info { >> uint16_t param; /* iscsi param value */ >> uint8_t iface_type; /* IPv4 or IPv6 */ >> uint8_t param_type; /* iscsi_param_type */ >> -uint8_t value[0]; /* length sized value follows */ >> +uint8_t value[];/* length sized value follows */ >> } __packed; >> >> /* >> @@ -697,7 +697,7 @@ enum iscsi_flashnode_param { >> struct iscsi_flashnode_param_info { >> uint32_t len; /* Actual length of the param */ >> uint16_t param; /* iscsi param value */ >> -uint8_t value[0]; /* length sized value follows */ >> +uint8_t value[];/* length sized value follows */ >> } __packed; >> >> enum iscsi_discovery_parent_type { >> @@ -815,7 +815,7 @@ struct iscsi_stats { >>* up to ISCSI_STATS_CUSTOM_MAX >>*/ >> uint32_t custom_length; >> -struct iscsi_stats_custom custom[0] >> +struct iscsi_stats_custom custom[] >> __attribute__ ((aligned (sizeof(uint64_t; >> }; >> >> @@ -946,7 +946,7 @@ struct iscsi_offload_host_stats { >>* up to ISCSI_HOST_STATS_CUSTOM_MAX >>*/ >> uint32_t custom_length; >> -struct iscsi_host_stats_custom custom[0] >> +struct iscsi_host_stats_custom custom[] >> __aligned(sizeof(uint64_t)); >> }; >> >> diff --git a/include/scsi/scsi_bsg_iscsi.h b/include/scsi/scsi_bsg_iscsi.h >> index fa0c820a1663..6b8128005af8 100644 >> --- a/include/scsi/scsi_bsg_iscsi.h >> +++ b/include/scsi/scsi_bsg_iscsi.h >> @@ -52,7 +52,7 @@ struct iscsi_bsg_host_vendor { >> uint64_t vendor_id; >> >> /* start of vendor command area */ >> -uint32_t vendor_cmd[0]; >> +uint32_t vendor_cmd[]; >> }; >> >> /* Response: >> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h >> index f8312a3e5b42..4dc158cf09b8 100644 >> --- a/include/scsi/scsi_device.h >> +++ b/include/scsi/scsi_device.h >> @@ -231,7 +231,7 @@ struct scsi_device { >> struct mutexstate_mutex; >> enum scsi_device_state sdev_state; >> struct task_struct *quiesced_by; >> -unsigned long sdev_data[0]; >> +unsigned long sdev_data[]; >> } __attribute__((aligned(sizeof(unsigned long; >> >> #define to_scsi_device(d) \ >> @@ -315,7 +315,7 @@ struct scsi_target { >> charscsi_level; >> enum scsi_target_state state; >> void
Tagging version 2.1.1 of open-iscsi/open-iscsi
Hi All: Just a heads up (for those that don't hang out on github) that I'm tagging version 2.1.1 of open-iscsi today, if there are no objections. -- You received this message because you are subscribed to the Google Groups "open-iscsi" group. To unsubscribe from this group and stop receiving emails from it, send an email to open-iscsi+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/open-iscsi/54aefe03-21e2-4b91-8593-7d178bbf50c7%40googlegroups.com.