Re: [PATCH] net/mlx5: Replace zero-length array with flexible-array
On Thu, 2020-05-07 at 13:59 -0500, 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] > > sizeof(flexible-array-member) triggers a warning because flexible > array > members have incomplete type[1]. There are some instances of code in > which the sizeof operator is being incorrectly/erroneously applied to > zero-length arrays and the result is zero. Such instances may be > hiding > some bugs. So, this work (flexible-array member conversions) will > also > help to get completely rid of those sorts of issues. > > 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 > Applied to mlx5-next, thanks !
Re: [PATCH] net/mlx5: Replace zero-length array with flexible-array
On Sat, 9 May 2020 23:43:08 + Saeed Mahameed wrote: > > Saeed, I'm expecting you to take this and the mlx4 patch via your > > trees. > > Yes for the mlx5 patch, but usually Dave takes mlx4 patches directly. Ack, it said IB on it, but looks like the patch can as well be applied to net-next, so I took it.
Re: [PATCH] net/mlx5: Replace zero-length array with flexible-array
On Thu, 2020-05-07 at 13:59 -0500, 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] > > sizeof(flexible-array-member) triggers a warning because flexible > array > members have incomplete type[1]. There are some instances of code in > which the sizeof operator is being incorrectly/erroneously applied to > zero-length arrays and the result is zero. Such instances may be > hiding hmmm, we actually have some tooling that rely on this to identify such 0 length fields .. since the structs in this file are usually auto- generated from the hw sepcs .. now i see that these tools are broken in our CI with this patch applied. I guess we will need to fix them, and fix our code auto-generation tools. overall i am ok with this patch. I will apply it to mlx5-next. and submit it upstream soom. > some bugs. So, this work (flexible-array member conversions) will > also > help to get completely rid of those sorts of issues. > > 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 > --- > include/linux/mlx5/driver.h |2 - > include/linux/mlx5/mlx5_ifc.h | 66 + > - > include/linux/mlx5/qp.h |2 - > 3 files changed, 35 insertions(+), 35 deletions(-) > > diff --git a/include/linux/mlx5/driver.h > b/include/linux/mlx5/driver.h > index 6f8f79ef829b..1a4ba36275de 100644 > --- a/include/linux/mlx5/driver.h > +++ b/include/linux/mlx5/driver.h > @@ -200,7 +200,7 @@ struct mlx5_rsc_debug { > void *object; > enum dbg_rsc_type type; > struct dentry *root; > - struct mlx5_field_desc fields[0]; > + struct mlx5_field_desc fields[]; > }; > > enum mlx5_dev_event { > diff --git a/include/linux/mlx5/mlx5_ifc.h > b/include/linux/mlx5/mlx5_ifc.h > index 69b27c7dfc3e..c55686ff6504 100644 > --- a/include/linux/mlx5/mlx5_ifc.h > +++ b/include/linux/mlx5/mlx5_ifc.h > @@ -1677,7 +1677,7 @@ struct mlx5_ifc_wq_bits { > > u8 reserved_at_140[0x4c0]; > > - struct mlx5_ifc_cmd_pas_bits pas[0]; > + struct mlx5_ifc_cmd_pas_bits pas[]; > }; > > struct mlx5_ifc_rq_num_bits { > @@ -1895,7 +1895,7 @@ struct mlx5_ifc_resource_dump_menu_segment_bits > { > u8 reserved_at_20[0x10]; > u8 num_of_records[0x10]; > > - struct mlx5_ifc_resource_dump_menu_record_bits record[0]; > + struct mlx5_ifc_resource_dump_menu_record_bits record[]; > }; > > struct mlx5_ifc_resource_dump_resource_segment_bits { > @@ -1907,7 +1907,7 @@ struct > mlx5_ifc_resource_dump_resource_segment_bits { > > u8 index2[0x20]; > > - u8 payload[0][0x20]; > + u8 payload[][0x20]; > }; > > struct mlx5_ifc_resource_dump_terminate_segment_bits { > @@ -2984,7 +2984,7 @@ struct mlx5_ifc_flow_context_bits { > > u8 reserved_at_1200[0x600]; > > - union mlx5_ifc_dest_format_struct_flow_counter_list_auto_bits > destination[0]; > + union mlx5_ifc_dest_format_struct_flow_counter_list_auto_bits > destination[]; > }; > > enum { > @@ -3276,7 +3276,7 @@ struct mlx5_ifc_rqtc_bits { > > u8 reserved_at_e0[0x6a0]; > > - struct mlx5_ifc_rq_num_bits rq_num[0]; > + struct mlx5_ifc_rq_num_bits rq_num[]; > }; > > enum { > @@ -3388,7 +3388,7 @@ struct mlx5_ifc_nic_vport_context_bits { > > u8 reserved_at_7e0[0x20]; > > - u8 current_uc_mac_address[0][0x40]; > + u8 current_uc_mac_address[][0x40]; > }; > > enum { > @@ -4310,7 +4310,7 @@ struct mlx5_ifc_query_xrc_srq_out_bits { > > u8 reserved_at_280[0x600]; > > - u8 pas[0][0x40]; > + u8 pas[][0x40]; > }; > > struct mlx5_ifc_query_xrc_srq_in_bits { > @@ -4588,7 +4588,7 @@ struct mlx5_ifc_query_srq_out_bits { > > u8 reserved_at_280[0x600]; > > - u8 pas[0][0x40]; > + u8 pas[][0x40]; > }; > > struct
Re: [PATCH] net/mlx5: Replace zero-length array with flexible-array
On Fri, 2020-05-08 at 16:36 -0700, Jakub Kicinski wrote: > On Thu, 7 May 2020 13:59:35 -0500 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[]; > > }; > > Saeed, I'm expecting you to take this and the mlx4 patch via your > trees. Yes for the mlx5 patch, but usually Dave takes mlx4 patches directly. since the volume of mlx4 patches is very small, let's apply them directly to net-next, unless you want me to handle them from now on and make your life easier, then i don't have any issue with that. Saeed.
Re: [PATCH] net/mlx5: Replace zero-length array with flexible-array
On Thu, 7 May 2020 13:59:35 -0500 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[]; > }; Saeed, I'm expecting you to take this and the mlx4 patch via your trees.
[PATCH] net/mlx5: Replace zero-length array with flexible-array
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] sizeof(flexible-array-member) triggers a warning because flexible array members have incomplete type[1]. There are some instances of code in which the sizeof operator is being incorrectly/erroneously applied to zero-length arrays and the result is zero. Such instances may be hiding some bugs. So, this work (flexible-array member conversions) will also help to get completely rid of those sorts of issues. 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 --- include/linux/mlx5/driver.h |2 - include/linux/mlx5/mlx5_ifc.h | 66 +- include/linux/mlx5/qp.h |2 - 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h index 6f8f79ef829b..1a4ba36275de 100644 --- a/include/linux/mlx5/driver.h +++ b/include/linux/mlx5/driver.h @@ -200,7 +200,7 @@ struct mlx5_rsc_debug { void *object; enum dbg_rsc_type type; struct dentry *root; - struct mlx5_field_desc fields[0]; + struct mlx5_field_desc fields[]; }; enum mlx5_dev_event { diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h index 69b27c7dfc3e..c55686ff6504 100644 --- a/include/linux/mlx5/mlx5_ifc.h +++ b/include/linux/mlx5/mlx5_ifc.h @@ -1677,7 +1677,7 @@ struct mlx5_ifc_wq_bits { u8 reserved_at_140[0x4c0]; - struct mlx5_ifc_cmd_pas_bits pas[0]; + struct mlx5_ifc_cmd_pas_bits pas[]; }; struct mlx5_ifc_rq_num_bits { @@ -1895,7 +1895,7 @@ struct mlx5_ifc_resource_dump_menu_segment_bits { u8 reserved_at_20[0x10]; u8 num_of_records[0x10]; - struct mlx5_ifc_resource_dump_menu_record_bits record[0]; + struct mlx5_ifc_resource_dump_menu_record_bits record[]; }; struct mlx5_ifc_resource_dump_resource_segment_bits { @@ -1907,7 +1907,7 @@ struct mlx5_ifc_resource_dump_resource_segment_bits { u8 index2[0x20]; - u8 payload[0][0x20]; + u8 payload[][0x20]; }; struct mlx5_ifc_resource_dump_terminate_segment_bits { @@ -2984,7 +2984,7 @@ struct mlx5_ifc_flow_context_bits { u8 reserved_at_1200[0x600]; - union mlx5_ifc_dest_format_struct_flow_counter_list_auto_bits destination[0]; + union mlx5_ifc_dest_format_struct_flow_counter_list_auto_bits destination[]; }; enum { @@ -3276,7 +3276,7 @@ struct mlx5_ifc_rqtc_bits { u8 reserved_at_e0[0x6a0]; - struct mlx5_ifc_rq_num_bits rq_num[0]; + struct mlx5_ifc_rq_num_bits rq_num[]; }; enum { @@ -3388,7 +3388,7 @@ struct mlx5_ifc_nic_vport_context_bits { u8 reserved_at_7e0[0x20]; - u8 current_uc_mac_address[0][0x40]; + u8 current_uc_mac_address[][0x40]; }; enum { @@ -4310,7 +4310,7 @@ struct mlx5_ifc_query_xrc_srq_out_bits { u8 reserved_at_280[0x600]; - u8 pas[0][0x40]; + u8 pas[][0x40]; }; struct mlx5_ifc_query_xrc_srq_in_bits { @@ -4588,7 +4588,7 @@ struct mlx5_ifc_query_srq_out_bits { u8 reserved_at_280[0x600]; - u8 pas[0][0x40]; + u8 pas[][0x40]; }; struct mlx5_ifc_query_srq_in_bits { @@ -4799,7 +4799,7 @@ struct mlx5_ifc_query_qp_out_bits { u8 reserved_at_800[0x80]; - u8 pas[0][0x40]; + u8 pas[][0x40]; }; struct mlx5_ifc_query_qp_in_bits { @@ -5132,7 +5132,7 @@ struct mlx5_ifc_query_hca_vport_pkey_out_bits { u8 reserved_at_40[0x40]; - struct mlx5_ifc_pkey_bits pkey[0]; + struct mlx5_ifc_pkey_bits pkey[]; }; struct mlx5_ifc_query_hca_vport_pkey_in_bits { @@ -5168,7 +5168,7 @@ struct mlx5_ifc_query_hca_vport_gid_out_bits { u8 gids_num[0x10]; u8 reserved_at_70[0x10]; - struct mlx5_ifc_array128_auto_bits gid[0]; +