Re: [dpdk-dev] [PATCH 10/10] test: fix global variable multiple definitions

[Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>]

<snip>

> 
> Multiple global variable are defined in multiple unit test files with same
> name, but all unit test files are linked into single executable, which means
> those variables share same storage which is not the intention, fixed by
> making global variables 'static'.
> 
> Issue has been detected by '-fno-common' gcc flag.
> 
> Fixes: fdeb30fa7102 ("test/bitrate: add unit tests for bitrate library")
> Fixes: c3eabff124e6 ("distributor: add unit tests")
> Fixes: 0e925aef2779 ("app/test: add EFD functional and perf tests")
> Fixes: 359e17bf081f ("app/test: improve hash unit tests")
> Fixes: c7eb0972e74b ("test/hash: add lock-free r/w concurrency")
> Fixes: 1e3676a06e4c ("test/latency: add unit tests for latencystats library")
> Fixes: 0cc67a96e486 ("test/member: add functional and perf tests")
> Fixes: e6a14121f4ae ("test/rcu: remove arbitrary limit on max core count")
> Fixes: 104dbec2081a ("test/rcu: increase size of core numbers")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>  app/test/test_bitratestats.c      |  6 +++---
>  app/test/test_distributor_perf.c  |  2 +-
>  app/test/test_efd.c               |  2 +-
>  app/test/test_efd_perf.c          |  6 +++---
>  app/test/test_hash_perf.c         | 12 ++++++------
>  app/test/test_hash_readwrite_lf.c |  8 ++++----
>  app/test/test_latencystats.c      |  6 +++---
>  app/test/test_member_perf.c       | 16 ++++++++--------
>  app/test/test_rcu_qsbr.c          | 10 +++++-----
>  9 files changed, 34 insertions(+), 34 deletions(-)
> 
> diff --git a/app/test/test_bitratestats.c b/app/test/test_bitratestats.c index
> 32b1b0fc0..3a7d9c037 100644
> --- a/app/test/test_bitratestats.c
> +++ b/app/test/test_bitratestats.c
> @@ -18,9 +18,9 @@
>  #define BIT_NUM_PACKETS 10
>  #define QUEUE_ID 0
> 
> -uint16_t portid;
> -struct rte_stats_bitrates *bitrate_data; -struct rte_ring *ring;
> +static uint16_t portid;
> +static struct rte_stats_bitrates *bitrate_data; static struct rte_ring
> +*ring;
> 
>  /* To test whether rte_stats_bitrate_create is successful */  static int diff --
> git a/app/test/test_distributor_perf.c b/app/test/test_distributor_perf.c
> index 664530ff9..f153bcf9b 100644
> --- a/app/test/test_distributor_perf.c
> +++ b/app/test/test_distributor_perf.c
> @@ -25,7 +25,7 @@ static volatile unsigned worker_idx;  struct
> worker_stats {
>  	volatile unsigned handled_packets;
>  } __rte_cache_aligned;
> -struct worker_stats worker_stats[RTE_MAX_LCORE];
> +static struct worker_stats worker_stats[RTE_MAX_LCORE];
> 
>  /*
>   * worker thread used for testing the time to do a round-trip of a cache diff -
> -git a/app/test/test_efd.c b/app/test/test_efd.c index 73b304431..a779a71f2
> 100644
> --- a/app/test/test_efd.c
> +++ b/app/test/test_efd.c
> @@ -94,7 +94,7 @@ static struct flow_key keys[5] = {
>  	}
>  };
>  /* Array to store the data */
> -efd_value_t data[5];
> +static efd_value_t data[5];
> 
>  static inline uint8_t efd_get_all_sockets_bitmask(void)  { diff --git
> a/app/test/test_efd_perf.c b/app/test/test_efd_perf.c index
> 1dcb992c6..d47622d5c 100644
> --- a/app/test/test_efd_perf.c
> +++ b/app/test/test_efd_perf.c
> @@ -71,13 +71,13 @@ static uint32_t hashtest_key_lens[] = {  };
> 
>  /* Array to store number of cycles per operation */ -uint64_t
> cycles[NUM_KEYSIZES][NUM_OPERATIONS];
> +static uint64_t cycles[NUM_KEYSIZES][NUM_OPERATIONS];
> 
>  /* Array to store the data */
> -efd_value_t data[KEYS_TO_ADD];
> +static efd_value_t data[KEYS_TO_ADD];
> 
>  /* Array to store all input keys */
> -uint8_t keys[KEYS_TO_ADD][MAX_KEYSIZE];
> +static uint8_t keys[KEYS_TO_ADD][MAX_KEYSIZE];
> 
>  /* Shuffle the keys that have been added, so lookups will be totally random
> */  static void diff --git a/app/test/test_hash_perf.c
> b/app/test/test_hash_perf.c index 5648fce02..a438eae6c 100644
> --- a/app/test/test_hash_perf.c
> +++ b/app/test/test_hash_perf.c
> @@ -53,22 +53,22 @@ static uint32_t hashtest_key_lens[] = {  struct
> rte_hash *h[NUM_KEYSIZES];
> 
>  /* Array that stores if a slot is full */ -uint8_t slot_taken[MAX_ENTRIES];
> +static uint8_t slot_taken[MAX_ENTRIES];
> 
>  /* Array to store number of cycles per operation */ -uint64_t
> cycles[NUM_KEYSIZES][NUM_OPERATIONS][2][2];
> +static uint64_t cycles[NUM_KEYSIZES][NUM_OPERATIONS][2][2];
> 
>  /* Array to store all input keys */
> -uint8_t keys[KEYS_TO_ADD][MAX_KEYSIZE];
> +static uint8_t keys[KEYS_TO_ADD][MAX_KEYSIZE];
> 
>  /* Array to store the precomputed hash for 'keys' */ -hash_sig_t
> signatures[KEYS_TO_ADD];
> +static hash_sig_t signatures[KEYS_TO_ADD];
> 
>  /* Array to store how many busy entries have each bucket */ -uint8_t
> buckets[NUM_BUCKETS];
> +static uint8_t buckets[NUM_BUCKETS];
> 
>  /* Array to store the positions where keys are added */ -int32_t
> positions[KEYS_TO_ADD];
> +static int32_t positions[KEYS_TO_ADD];
> 
>  /* Parameters used for hash table in unit test functions. */  static struct
> rte_hash_parameters ut_params = { diff --git
> a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c
> index 1f2fba41f..97c304054 100644
> --- a/app/test/test_hash_readwrite_lf.c
> +++ b/app/test/test_hash_readwrite_lf.c
> @@ -48,7 +48,7 @@
>  #define WRITE_EXT_BKT 2
> 
>  #define NUM_TEST 3
> -unsigned int rwc_core_cnt[NUM_TEST] = {1, 2, 4};
> +static unsigned int rwc_core_cnt[NUM_TEST] = {1, 2, 4};
> 
>  struct rwc_perf {
>  	uint32_t w_no_ks_r_hit[2][NUM_TEST];
> @@ -62,7 +62,7 @@ struct rwc_perf {
> 
>  static struct rwc_perf rwc_lf_results, rwc_non_lf_results;
> 
> -struct {
> +static struct {
>  	uint32_t *keys;
>  	uint32_t *keys_no_ks;
>  	uint32_t *keys_ks;
> @@ -87,9 +87,9 @@ static rte_atomic64_t greads;
> 
>  static volatile uint8_t writer_done;
> 
> -uint16_t enabled_core_ids[RTE_MAX_LCORE];
> +static uint16_t enabled_core_ids[RTE_MAX_LCORE];
> 
> -uint8_t *scanned_bkts;
> +static uint8_t *scanned_bkts;
> 
>  static inline uint16_t
>  get_short_sig(const hash_sig_t hash)
> diff --git a/app/test/test_latencystats.c b/app/test/test_latencystats.c index
> 8dd794be4..968e0bc47 100644
> --- a/app/test/test_latencystats.c
> +++ b/app/test/test_latencystats.c
> @@ -17,10 +17,10 @@
>  #define LATENCY_NUM_PACKETS 10
>  #define QUEUE_ID 0
> 
> -uint16_t portid;
> -struct rte_ring *ring;
> +static uint16_t portid;
> +static struct rte_ring *ring;
> 
> -struct rte_metric_name lat_stats_strings[] = {
> +static struct rte_metric_name lat_stats_strings[] = {
>  	{"min_latency_ns"},
>  	{"avg_latency_ns"},
>  	{"max_latency_ns"},
> diff --git a/app/test/test_member_perf.c b/app/test/test_member_perf.c
> index 564a2b3c1..e2840f12d 100644
> --- a/app/test/test_member_perf.c
> +++ b/app/test/test_member_perf.c
> @@ -65,18 +65,18 @@ static uint32_t hashtest_key_lens[] = {  };
> 
>  /* Array to store number of cycles per operation */ -uint64_t
> cycles[NUM_TYPE][NUM_KEYSIZES][NUM_OPERATIONS];
> -uint64_t false_data[NUM_TYPE][NUM_KEYSIZES];
> -uint64_t false_data_bulk[NUM_TYPE][NUM_KEYSIZES];
> -uint64_t false_data_multi[NUM_TYPE][NUM_KEYSIZES];
> -uint64_t false_data_multi_bulk[NUM_TYPE][NUM_KEYSIZES];
> +static uint64_t cycles[NUM_TYPE][NUM_KEYSIZES][NUM_OPERATIONS];
> +static uint64_t false_data[NUM_TYPE][NUM_KEYSIZES];
> +static uint64_t false_data_bulk[NUM_TYPE][NUM_KEYSIZES];
> +static uint64_t false_data_multi[NUM_TYPE][NUM_KEYSIZES];
> +static uint64_t false_data_multi_bulk[NUM_TYPE][NUM_KEYSIZES];
> 
> -uint64_t false_hit[NUM_TYPE][NUM_KEYSIZES];
> +static uint64_t false_hit[NUM_TYPE][NUM_KEYSIZES];
> 
> -member_set_t data[NUM_TYPE][/* Array to store the data */KEYS_TO_ADD];
> +static member_set_t data[NUM_TYPE][/* Array to store the data
> +*/KEYS_TO_ADD];
> 
>  /* Array to store all input keys */
> -uint8_t keys[KEYS_TO_ADD][MAX_KEYSIZE];
> +static uint8_t keys[KEYS_TO_ADD][MAX_KEYSIZE];
> 
>  /* Shuffle the keys that have been added, so lookups will be totally random
> */  static void diff --git a/app/test/test_rcu_qsbr.c b/app/test/test_rcu_qsbr.c
> index d1b9e46a2..53c46ccc1 100644
> --- a/app/test/test_rcu_qsbr.c
> +++ b/app/test/test_rcu_qsbr.c
> @@ -25,8 +25,8 @@
>  /* Make sure that this has the same value as __RTE_QSBR_CNT_INIT */
> #define TEST_RCU_QSBR_CNT_INIT 1
> 
> -uint16_t enabled_core_ids[RTE_MAX_LCORE]; -unsigned int num_cores;
> +static uint16_t enabled_core_ids[RTE_MAX_LCORE]; static unsigned int
> +num_cores;
> 
>  static uint32_t *keys;
>  #define TOTAL_ENTRY (1024 * 8)
> @@ -35,8 +35,8 @@ static uint32_t
> *hash_data[RTE_MAX_LCORE][TOTAL_ENTRY];
>  static uint8_t writer_done;
> 
>  static struct rte_rcu_qsbr *t[RTE_MAX_LCORE]; -struct rte_hash
> *h[RTE_MAX_LCORE]; -char hash_name[RTE_MAX_LCORE][8];
> +static struct rte_hash *h[RTE_MAX_LCORE]; static char
> +hash_name[RTE_MAX_LCORE][8];
> 
>  struct test_rcu_thread_info {
>  	/* Index in RCU array */
> @@ -46,7 +46,7 @@ struct test_rcu_thread_info {
>  	/* lcore IDs registered on the RCU variable */
>  	uint16_t r_core_ids[2];
>  };
> -struct test_rcu_thread_info thread_info[RTE_MAX_LCORE/4];
> +static struct test_rcu_thread_info thread_info[RTE_MAX_LCORE/4];
> 
>  static int
>  alloc_rcu(void)
RCU changes look fine.

> --
> 2.21.0