mlxsw: Replace zero-length array with flexible-array member
Subsystems (3)
PATCH 1/1 mlxsw: 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")
--- drivers/net/ethernet/mellanox/mlxsw/core.c | 2 +- drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_keys.c | 2 +- drivers/net/ethernet/mellanox/mlxsw/spectrum1_kvdl.c | 2 +- drivers/net/ethernet/mellanox/mlxsw/spectrum2_kvdl.c | 2 +- drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c | 4 ++-- .../net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c | 2 +- drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c | 4 ++-- drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.h | 4 ++-- drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c | 2 +- drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c | 2 +- drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c | 2 +- 11 files changed, 14 insertions(+), 14 deletions(-)
drivers/net/ethernet/mellanox/mlxsw/core.c +1 -1
@@ -82,7 +82,7 @@ struct mlxsw_core {struct mlxsw_core_port *ports;unsigned int max_ports;bool fw_flash_in_progress; -unsigned long driver_priv[0 ]; +unsigned long driver_priv[];/* driver_priv has to be always the last item */ };
drivers/net/ethernet/mellanox/mlxsw/core_acl_flex_keys.c +1 -1
@@ -72,7 +72,7 @@ struct mlxsw_afk_key_info {* is index inside "blocks" */ struct mlxsw_afk_element_usage elusage; -const struct mlxsw_afk_block *blocks[0 ]; +const struct mlxsw_afk_block *blocks[]; }; static bool
drivers/net/ethernet/mellanox/mlxsw/spectrum1_kvdl.c +1 -1
@@ -60,7 +60,7 @@ static const struct mlxsw_sp1_kvdl_part_info mlxsw_sp1_kvdl_parts_info[] = {struct mlxsw_sp1_kvdl_part {struct mlxsw_sp1_kvdl_part_info info; -unsigned long usage[0 ];/* Entries */ +unsigned long usage[];/* Entries */ };struct mlxsw_sp1_kvdl {
drivers/net/ethernet/mellanox/mlxsw/spectrum2_kvdl.c +1 -1
@@ -45,7 +45,7 @@ struct mlxsw_sp2_kvdl_part {unsigned int usage_bit_count;unsigned int indexes_per_usage_bit;unsigned int last_allocated_bit; -unsigned long usage[0 ];/* Usage bits */ +unsigned long usage[];/* Usage bits */ };struct mlxsw_sp2_kvdl {
drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c +2 -2
@@ -58,7 +58,7 @@ struct mlxsw_sp_acl_ruleset {struct mlxsw_sp_acl_ruleset_ht_key ht_key;struct rhashtable rule_ht;unsigned int ref_count; -unsigned long priv[0 ]; +unsigned long priv[];/* priv has to be always the last item */ };
@@ -71,7 +71,7 @@ struct mlxsw_sp_acl_rule {u64 last_used;u64 last_packets;u64 last_bytes; -unsigned long priv[0 ]; +unsigned long priv[];/* priv has to be always the last item */ };
drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_bloom_filter.c +1 -1
@@ -13,7 +13,7 @@struct mlxsw_sp_acl_bf {struct mutex lock;/* Protects Bloom Filter updates. */ unsigned int bank_size; -refcount_t refcnt[0 ]; +refcount_t refcnt[]; };/* Bloom filter uses a crc-16 hash over chunks of data which contain 4 key
drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c +2 -2
@@ -224,7 +224,7 @@ struct mlxsw_sp_acl_tcam_vchunk;struct mlxsw_sp_acl_tcam_chunk {struct mlxsw_sp_acl_tcam_vchunk *vchunk;struct mlxsw_sp_acl_tcam_region *region; -unsigned long priv[0 ]; +unsigned long priv[];/* priv has to be always the last item */ };
@@ -243,7 +243,7 @@ struct mlxsw_sp_acl_tcam_vchunk {struct mlxsw_sp_acl_tcam_entry {struct mlxsw_sp_acl_tcam_ventry *ventry;struct mlxsw_sp_acl_tcam_chunk *chunk; -unsigned long priv[0 ]; +unsigned long priv[];/* priv has to be always the last item */ };
drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.h +2 -2
@@ -20,7 +20,7 @@ struct mlxsw_sp_acl_tcam {struct mutex lock;/* guards vregion list */ struct list_head vregion_list;u32 vregion_rehash_intrvl;/* ms */ -unsigned long priv[0 ]; +unsigned long priv[];/* priv has to be always the last item */ };
@@ -86,7 +86,7 @@ struct mlxsw_sp_acl_tcam_region {char tcam_region_info[MLXSW_REG_PXXX_TCAM_REGION_INFO_LEN ];struct mlxsw_afk_key_info *key_info;struct mlxsw_sp *mlxsw_sp; -unsigned long priv[0 ]; +unsigned long priv[];/* priv has to be always the last item */ };
drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c +1 -1
@@ -8,7 +8,7 @@struct mlxsw_sp_kvdl {const struct mlxsw_sp_kvdl_ops *kvdl_ops; -unsigned long priv[0 ]; +unsigned long priv[];/* priv has to be always the last item */ };
drivers/net/ethernet/mellanox/mlxsw/spectrum_mr.c +1 -1
@@ -68,7 +68,7 @@ struct mlxsw_sp_mr_table {struct list_head route_list;struct rhashtable route_ht;const struct mlxsw_sp_mr_table_ops *ops; -char catchall_route_priv[0 ]; +char catchall_route_priv[];/* catchall_route_priv has to be always the last item */ };
drivers/net/ethernet/mellanox/mlxsw/spectrum_nve.c +1 -1
@@ -67,7 +67,7 @@ struct mlxsw_sp_nve_mc_record {struct mlxsw_sp_nve_mc_list *mc_list;const struct mlxsw_sp_nve_mc_record_ops *ops;u32 kvdl_index; -struct mlxsw_sp_nve_mc_entry entries[0 ]; +struct mlxsw_sp_nve_mc_entry entries[]; };struct mlxsw_sp_nve_mc_list {
Signature · 1 line
2.25.0
Quoted 22 lines · 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]
Quoted 7 lines
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] commit76497732932f("cxgb3/l2t: Fix undefined behaviour")Signed-off-by Gustavo A. R. Silva <[email protected]>
Thanks, Gustavo. Looks good to me. Ran a few tests with a debug config and nothing exploded.
I was just about to submit some patches myself, but they will conflict with this patch, so I will wait :)
Quoted 25 lines · Ido Schimmel
Quoted 24 lines · 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.
Quoted 9 lines
Quoted 5 lines
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit76497732932f("cxgb3/l2t: Fix undefined behaviour")Signed-off-by Gustavo A. R. Silva <[email protected]>Thanks, Gustavo. Looks good to me. Ran a few tests with a debug config and nothing exploded.
haha great!
Quoted 3 lines
I was just about to submit some patches myself, but they will conflict with this patch, so I will wait :)
:)
Quoted 3 lines
Reviewed-by Ido Schimmel <[email protected]>Tested-by Ido Schimmel <[email protected]>
Thanks!
--
Gustavo
From: "Gustavo A. R. Silva" <[email protected]>
Date: Tue, 18 Feb 2020 14:57:05 -0600
Quoted 22 lines
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]
Quoted 7 lines
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] commit76497732932f("cxgb3/l2t: Fix undefined behaviour")Signed-off-by Gustavo A. R. Silva <[email protected]>
Applied, thanks.