Re: [PATCH] net/mlx5: Replace zero-length array with flexible-array

2020-05-10 Thread Saeed Mahameed
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

2020-05-09 Thread Jakub Kicinski
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

2020-05-09 Thread Saeed Mahameed
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

2020-05-09 Thread Saeed Mahameed
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

2020-05-08 Thread Jakub Kicinski
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.