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

Message ID 20190905093239.27187-6-amo@semihalf.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Add an option to use LTO for DPDK build |

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 --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;