[05/10] app/test: fix maybe-uninitialized warnings for LTO build
diff mbox series

Message ID 20190905093239.27187-6-amo@semihalf.com
State Superseded, archived
Delegated to: Thomas Monjalon
Headers show
Series
  • Add an option to use LTO for DPDK build
Related show

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Andrzej Ostruszka Sept. 5, 2019, 9:32 a.m. UTC
During LTO build compiler reports some 'false positive' warnings about
variables being possibly used uninitialized.  This patch silences these
warnings.

Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
---
 app/test/test_hash_readwrite.c     | 2 +-
 app/test/test_link_bonding_mode4.c | 6 ++++--
 app/test/test_memzone.c            | 3 ++-
 3 files changed, 7 insertions(+), 4 deletions(-)

Comments

Chas Williams Sept. 5, 2019, 1:25 p.m. UTC | #1
On 9/5/19 5:32 AM, Andrzej Ostruszka wrote:
> During LTO build compiler reports some 'false positive' warnings about
> variables being possibly used uninitialized.  This patch silences these
> warnings.
> 
> Signed-off-by: Andrzej Ostruszka <aostruszka@marvell.com>
> ---
>   app/test/test_hash_readwrite.c     | 2 +-
>   app/test/test_link_bonding_mode4.c | 6 ++++--
>   app/test/test_memzone.c            | 3 ++-
>   3 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c
> index 4376b099b..615767fb6 100644
> --- a/app/test/test_hash_readwrite.c
> +++ b/app/test/test_hash_readwrite.c
> @@ -298,7 +298,7 @@ test_rw_reader(void *arg)
>   
>   	begin = rte_rdtsc_precise();
>   	for (i = 0; i < read_cnt; i++) {
> -		void *data;
> +		void *data = arg;
>   		rte_hash_lookup_data(tbl_rw_test_param.h,
>   				tbl_rw_test_param.keys + i,
>   				&data);
> diff --git a/app/test/test_link_bonding_mode4.c b/app/test/test_link_bonding_mode4.c
> index bbb4e9cce..8ca376dda 100644
> --- a/app/test/test_link_bonding_mode4.c
> +++ b/app/test/test_link_bonding_mode4.c
> @@ -224,7 +224,7 @@ configure_ethdev(uint16_t port_id, uint8_t start)
>   static int
>   add_slave(struct slave_conf *slave, uint8_t start)
>   {
> -	struct rte_ether_addr addr, addr_check;
> +	struct rte_ether_addr addr, addr_check = { { 0 } };
>   
>   	/* Some sanity check */
>   	RTE_VERIFY(test_params.slave_ports <= slave &&
> @@ -578,7 +578,9 @@ bond_get_update_timeout_ms(void)
>   {
>   	struct rte_eth_bond_8023ad_conf conf;
>   
> -	rte_eth_bond_8023ad_conf_get(test_params.bonded_port_id, &conf);
> +	if (rte_eth_bond_8023ad_conf_get(test_params.bonded_port_id, &conf) < 0)
> +		return 0;
> +

This would return a delay of 0ms, which doesn't feel like it captures 
the intent of conf_get failing. Perhaps a TEST_ASSERT_FAILURE() instead?

>   	return conf.update_timeout_ms;
>   }
>   
> diff --git a/app/test/test_memzone.c b/app/test/test_memzone.c
> index 7501b63c5..c284dcb44 100644
> --- a/app/test/test_memzone.c
> +++ b/app/test/test_memzone.c
> @@ -476,7 +476,8 @@ find_max_block_free_size(unsigned int align, unsigned int socket_id)
>   	struct rte_malloc_socket_stats stats;
>   	size_t len, overhead;
>   
> -	rte_malloc_get_socket_stats(socket_id, &stats);
> +	if (rte_malloc_get_socket_stats(socket_id, &stats) < 0)
> +		return 0;
>   
>   	len = stats.greatest_free_size;
>   	overhead = MALLOC_ELEM_OVERHEAD;
>
Andrzej Ostruszka Sept. 17, 2019, 10:41 a.m. UTC | #2
On 9/5/19 3:25 PM, Chas Williams wrote:
[...]
>> @@ -578,7 +578,9 @@ bond_get_update_timeout_ms(void)
>>   {
>>   	struct rte_eth_bond_8023ad_conf conf;
>>   
>> -	rte_eth_bond_8023ad_conf_get(test_params.bonded_port_id, &conf);
>> +	if (rte_eth_bond_8023ad_conf_get(test_params.bonded_port_id, &conf) < 0)
>> +		return 0;
>> +
> 
> This would return a delay of 0ms, which doesn't feel like it captures 
> the intent of conf_get failing. Perhaps a TEST_ASSERT_FAILURE() instead?

Chas, I've sent today reworked version.  I've taken this remark into
account however TEST_ASSERT_FAILURE() is not really appropriate here
since this macro is not really an assert.  It returns -1, which does not
play well with the signature of this function (return is unsigned) and
how it is used.  So I've just inlined the logs.

Regards
Andrzej

Patch
diff mbox series

diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c
index 4376b099b..615767fb6 100644
--- a/app/test/test_hash_readwrite.c
+++ b/app/test/test_hash_readwrite.c
@@ -298,7 +298,7 @@  test_rw_reader(void *arg)
 
 	begin = rte_rdtsc_precise();
 	for (i = 0; i < read_cnt; i++) {
-		void *data;
+		void *data = arg;
 		rte_hash_lookup_data(tbl_rw_test_param.h,
 				tbl_rw_test_param.keys + i,
 				&data);
diff --git a/app/test/test_link_bonding_mode4.c b/app/test/test_link_bonding_mode4.c
index bbb4e9cce..8ca376dda 100644
--- a/app/test/test_link_bonding_mode4.c
+++ b/app/test/test_link_bonding_mode4.c
@@ -224,7 +224,7 @@  configure_ethdev(uint16_t port_id, uint8_t start)
 static int
 add_slave(struct slave_conf *slave, uint8_t start)
 {
-	struct rte_ether_addr addr, addr_check;
+	struct rte_ether_addr addr, addr_check = { { 0 } };
 
 	/* Some sanity check */
 	RTE_VERIFY(test_params.slave_ports <= slave &&
@@ -578,7 +578,9 @@  bond_get_update_timeout_ms(void)
 {
 	struct rte_eth_bond_8023ad_conf conf;
 
-	rte_eth_bond_8023ad_conf_get(test_params.bonded_port_id, &conf);
+	if (rte_eth_bond_8023ad_conf_get(test_params.bonded_port_id, &conf) < 0)
+		return 0;
+
 	return conf.update_timeout_ms;
 }
 
diff --git a/app/test/test_memzone.c b/app/test/test_memzone.c
index 7501b63c5..c284dcb44 100644
--- a/app/test/test_memzone.c
+++ b/app/test/test_memzone.c
@@ -476,7 +476,8 @@  find_max_block_free_size(unsigned int align, unsigned int socket_id)
 	struct rte_malloc_socket_stats stats;
 	size_t len, overhead;
 
-	rte_malloc_get_socket_stats(socket_id, &stats);
+	if (rte_malloc_get_socket_stats(socket_id, &stats) < 0)
+		return 0;
 
 	len = stats.greatest_free_size;
 	overhead = MALLOC_ELEM_OVERHEAD;