DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] app/testpmd: fix passing negative parameter to strerror
@ 2020-06-04  6:22 Wei Hu (Xavier)
  2020-06-04  8:45 ` Phil Yang
  2020-06-04 16:30 ` Ferruh Yigit
  0 siblings, 2 replies; 7+ messages in thread
From: Wei Hu (Xavier) @ 2020-06-04  6:22 UTC (permalink / raw)
  To: dev; +Cc: xavier.huwei

Currently, there are coverity defect warnings as blow:
Coverity issue:
  In nic_stats_clear function:
    CID 290021 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
    5. negative_returns: ret is passed to a parameter that cannot be
       negative.

    CID 289974 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
    6. negative_returns: ret is passed to a parameter that cannot be
       negative.

  In nic_xstats_clear function:
    CID 289985 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
    5. negative_returns: ret is passed to a parameter that cannot be
       negative.

    CID 289850 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
    6. negative_returns: ret is passed to a parameter that cannot be
       negative.

This patch fixes them by passing '-ret' to the function strerror() when ret
is negative.

Fixes: af75078fece3 ("first public release")
Fixes: 9eb974221f44 ("app/testpmd: fix statistics after reset")
Cc: stable@dpdk.org

Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
 app/test-pmd/config.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 5381207..356d0d2 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -244,14 +244,14 @@ nic_stats_clear(portid_t port_id)
 	ret = rte_eth_stats_reset(port_id);
 	if (ret != 0) {
 		printf("%s: Error: failed to reset stats (port %u): %s",
-		       __func__, port_id, strerror(ret));
+		       __func__, port_id, strerror(-ret));
 		return;
 	}
 
 	ret = rte_eth_stats_get(port_id, &ports[port_id].stats);
 	if (ret != 0) {
 		printf("%s: Error: failed to get stats (port %u): %s",
-		       __func__, port_id, strerror(ret));
+		       __func__, port_id, strerror(-ret));
 		return;
 	}
 	printf("\n  NIC statistics for port %d cleared\n", port_id);
@@ -333,14 +333,14 @@ nic_xstats_clear(portid_t port_id)
 	ret = rte_eth_xstats_reset(port_id);
 	if (ret != 0) {
 		printf("%s: Error: failed to reset xstats (port %u): %s",
-		       __func__, port_id, strerror(ret));
+		       __func__, port_id, strerror(-ret));
 		return;
 	}
 
 	ret = rte_eth_stats_get(port_id, &ports[port_id].stats);
 	if (ret != 0) {
 		printf("%s: Error: failed to get stats (port %u): %s",
-		       __func__, port_id, strerror(ret));
+		       __func__, port_id, strerror(-ret));
 		return;
 	}
 }
-- 
2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] app/testpmd: fix passing negative parameter to strerror
  2020-06-04  6:22 [dpdk-dev] [PATCH] app/testpmd: fix passing negative parameter to strerror Wei Hu (Xavier)
@ 2020-06-04  8:45 ` Phil Yang
  2020-06-04 10:51   ` Kevin Traynor
  2020-06-04 16:30 ` Ferruh Yigit
  1 sibling, 1 reply; 7+ messages in thread
From: Phil Yang @ 2020-06-04  8:45 UTC (permalink / raw)
  To: Wei Hu (Xavier), dev; +Cc: nd, nd

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Wei Hu (Xavier)
> Sent: Thursday, June 4, 2020 2:22 PM
> To: dev@dpdk.org
> Cc: xavier.huwei@huawei.com
> Subject: [dpdk-dev] [PATCH] app/testpmd: fix passing negative parameter to
> strerror
> 
> Currently, there are coverity defect warnings as blow:
> Coverity issue:
>   In nic_stats_clear function:
>     CID 290021 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>     5. negative_returns: ret is passed to a parameter that cannot be
>        negative.
> 
>     CID 289974 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>     6. negative_returns: ret is passed to a parameter that cannot be
>        negative.
> 
>   In nic_xstats_clear function:
>     CID 289985 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>     5. negative_returns: ret is passed to a parameter that cannot be
>        negative.
> 
>     CID 289850 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>     6. negative_returns: ret is passed to a parameter that cannot be
>        negative.
> 
> This patch fixes them by passing '-ret' to the function strerror() when ret
> is negative.
> 
> Fixes: af75078fece3 ("first public release")

It was the commit da328f7f115a ("ethdev: change xstats reset function to return int") introduced this defect in nic_xstats_clear() function.
Not the "first public release" commit. :)

The patch looks good to me.

Thanks,
Phil

> Fixes: 9eb974221f44 ("app/testpmd: fix statistics after reset")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> ---
>  app/test-pmd/config.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 5381207..356d0d2 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -244,14 +244,14 @@ nic_stats_clear(portid_t port_id)
>  	ret = rte_eth_stats_reset(port_id);
>  	if (ret != 0) {
>  		printf("%s: Error: failed to reset stats (port %u): %s",
> -		       __func__, port_id, strerror(ret));
> +		       __func__, port_id, strerror(-ret));
>  		return;
>  	}
> 
>  	ret = rte_eth_stats_get(port_id, &ports[port_id].stats);
>  	if (ret != 0) {
>  		printf("%s: Error: failed to get stats (port %u): %s",
> -		       __func__, port_id, strerror(ret));
> +		       __func__, port_id, strerror(-ret));
>  		return;
>  	}
>  	printf("\n  NIC statistics for port %d cleared\n", port_id);
> @@ -333,14 +333,14 @@ nic_xstats_clear(portid_t port_id)
>  	ret = rte_eth_xstats_reset(port_id);
>  	if (ret != 0) {
>  		printf("%s: Error: failed to reset xstats (port %u): %s",
> -		       __func__, port_id, strerror(ret));
> +		       __func__, port_id, strerror(-ret));
>  		return;
>  	}
> 
>  	ret = rte_eth_stats_get(port_id, &ports[port_id].stats);
>  	if (ret != 0) {
>  		printf("%s: Error: failed to get stats (port %u): %s",
> -		       __func__, port_id, strerror(ret));
> +		       __func__, port_id, strerror(-ret));
>  		return;
>  	}
>  }
> --
> 2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] app/testpmd: fix passing negative parameter to strerror
  2020-06-04  8:45 ` Phil Yang
@ 2020-06-04 10:51   ` Kevin Traynor
  0 siblings, 0 replies; 7+ messages in thread
From: Kevin Traynor @ 2020-06-04 10:51 UTC (permalink / raw)
  To: Phil Yang, Wei Hu (Xavier), dev; +Cc: nd

On 04/06/2020 09:45, Phil Yang wrote:
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Wei Hu (Xavier)
>> Sent: Thursday, June 4, 2020 2:22 PM
>> To: dev@dpdk.org
>> Cc: xavier.huwei@huawei.com
>> Subject: [dpdk-dev] [PATCH] app/testpmd: fix passing negative parameter to
>> strerror
>>
>> Currently, there are coverity defect warnings as blow:
>> Coverity issue:
>>   In nic_stats_clear function:
>>     CID 290021 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>     5. negative_returns: ret is passed to a parameter that cannot be
>>        negative.
>>
>>     CID 289974 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>     6. negative_returns: ret is passed to a parameter that cannot be
>>        negative.
>>
>>   In nic_xstats_clear function:
>>     CID 289985 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>     5. negative_returns: ret is passed to a parameter that cannot be
>>        negative.
>>
>>     CID 289850 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>     6. negative_returns: ret is passed to a parameter that cannot be
>>        negative.
>>
>> This patch fixes them by passing '-ret' to the function strerror() when ret
>> is negative.
>>
>> Fixes: af75078fece3 ("first public release")
> 
> It was the commit da328f7f115a ("ethdev: change xstats reset function to return int") introduced this defect in nic_xstats_clear() function.
> Not the "first public release" commit. :)
> 
> The patch looks good to me.
> 

+1

> Thanks,
> Phil
> 
>> Fixes: 9eb974221f44 ("app/testpmd: fix statistics after reset")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>> ---
>>  app/test-pmd/config.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index 5381207..356d0d2 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -244,14 +244,14 @@ nic_stats_clear(portid_t port_id)
>>  	ret = rte_eth_stats_reset(port_id);
>>  	if (ret != 0) {
>>  		printf("%s: Error: failed to reset stats (port %u): %s",
>> -		       __func__, port_id, strerror(ret));
>> +		       __func__, port_id, strerror(-ret));
>>  		return;
>>  	}
>>
>>  	ret = rte_eth_stats_get(port_id, &ports[port_id].stats);
>>  	if (ret != 0) {
>>  		printf("%s: Error: failed to get stats (port %u): %s",
>> -		       __func__, port_id, strerror(ret));
>> +		       __func__, port_id, strerror(-ret));
>>  		return;
>>  	}
>>  	printf("\n  NIC statistics for port %d cleared\n", port_id);
>> @@ -333,14 +333,14 @@ nic_xstats_clear(portid_t port_id)
>>  	ret = rte_eth_xstats_reset(port_id);
>>  	if (ret != 0) {
>>  		printf("%s: Error: failed to reset xstats (port %u): %s",
>> -		       __func__, port_id, strerror(ret));
>> +		       __func__, port_id, strerror(-ret));
>>  		return;
>>  	}
>>
>>  	ret = rte_eth_stats_get(port_id, &ports[port_id].stats);
>>  	if (ret != 0) {
>>  		printf("%s: Error: failed to get stats (port %u): %s",
>> -		       __func__, port_id, strerror(ret));
>> +		       __func__, port_id, strerror(-ret));
>>  		return;
>>  	}
>>  }
>> --
>> 2.7.4
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] app/testpmd: fix passing negative parameter to strerror
  2020-06-04  6:22 [dpdk-dev] [PATCH] app/testpmd: fix passing negative parameter to strerror Wei Hu (Xavier)
  2020-06-04  8:45 ` Phil Yang
@ 2020-06-04 16:30 ` Ferruh Yigit
  2020-06-05  2:57   ` Wei Hu (Xavier)
  1 sibling, 1 reply; 7+ messages in thread
From: Ferruh Yigit @ 2020-06-04 16:30 UTC (permalink / raw)
  To: Wei Hu (Xavier), dev

On 6/4/2020 7:22 AM, Wei Hu (Xavier) wrote:
> Currently, there are coverity defect warnings as blow:
> Coverity issue:
>   In nic_stats_clear function:
>     CID 290021 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>     5. negative_returns: ret is passed to a parameter that cannot be
>        negative.
> 
>     CID 289974 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>     6. negative_returns: ret is passed to a parameter that cannot be
>        negative.
> 
>   In nic_xstats_clear function:
>     CID 289985 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>     5. negative_returns: ret is passed to a parameter that cannot be
>        negative.
> 
>     CID 289850 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>     6. negative_returns: ret is passed to a parameter that cannot be
>        negative.

I guess these coverity IDs are from the internal coverity, because I can't find
them in the public coverity.
If it is internal, not sure about the benefit of documenting them in the commit
log, since no one except huawei can access them. What do you think to remove all
above reference?

> 
> This patch fixes them by passing '-ret' to the function strerror() when ret
> is negative.
> 
> Fixes: af75078fece3 ("first public release")
> Fixes: 9eb974221f44 ("app/testpmd: fix statistics after reset")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> ---
>  app/test-pmd/config.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 5381207..356d0d2 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -244,14 +244,14 @@ nic_stats_clear(portid_t port_id)
>  	ret = rte_eth_stats_reset(port_id);
>  	if (ret != 0) {
>  		printf("%s: Error: failed to reset stats (port %u): %s",
> -		       __func__, port_id, strerror(ret));
> +		       __func__, port_id, strerror(-ret));
>  		return;
>  	}
>  
>  	ret = rte_eth_stats_get(port_id, &ports[port_id].stats);
>  	if (ret != 0) {
>  		printf("%s: Error: failed to get stats (port %u): %s",
> -		       __func__, port_id, strerror(ret));
> +		       __func__, port_id, strerror(-ret));

Although in practice this may be the case, the 'rte_eth_stats_get()' function
documentation doesn't guarantee that return value will be negative, it says:

"
 * @return
 *   Zero if successful. Non-zero otherwise
"

To be accurate, what do you think to adding a negative check for 'ret' before
doing '-ret'?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] app/testpmd: fix passing negative parameter to strerror
  2020-06-04 16:30 ` Ferruh Yigit
@ 2020-06-05  2:57   ` Wei Hu (Xavier)
  2020-06-05  9:25     ` Ferruh Yigit
  0 siblings, 1 reply; 7+ messages in thread
From: Wei Hu (Xavier) @ 2020-06-05  2:57 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev, xavier.huwei



On 2020/6/5 0:30, Ferruh Yigit wrote:
> On 6/4/2020 7:22 AM, Wei Hu (Xavier) wrote:
>> Currently, there are coverity defect warnings as blow:
>> Coverity issue:
>>    In nic_stats_clear function:
>>      CID 290021 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>      5. negative_returns: ret is passed to a parameter that cannot be
>>         negative.
>>
>>      CID 289974 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>      6. negative_returns: ret is passed to a parameter that cannot be
>>         negative.
>>
>>    In nic_xstats_clear function:
>>      CID 289985 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>      5. negative_returns: ret is passed to a parameter that cannot be
>>         negative.
>>
>>      CID 289850 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>      6. negative_returns: ret is passed to a parameter that cannot be
>>         negative.
> I guess these coverity IDs are from the internal coverity, because I can't find
> them in the public coverity.
> If it is internal, not sure about the benefit of documenting them in the commit
> log, since no one except huawei can access them. What do you think to remove all
> above reference?
Yes, these are internal covertiy defects information. Maybe we can remove
internal coverity CID xxxx and reserve the description of the defect in the
commit log.

By the way,  when we access the "View defects" on the page of the public 
coverity
https://scan.coverity.com/projects/dpdk-data-plane-development-kit?tab=overview 

, The browser prompts HTTP ERROR 403.
>> This patch fixes them by passing '-ret' to the function strerror() when ret
>> is negative.
>>
>> Fixes: af75078fece3 ("first public release")
>> Fixes: 9eb974221f44 ("app/testpmd: fix statistics after reset")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>> ---
>>   app/test-pmd/config.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index 5381207..356d0d2 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -244,14 +244,14 @@ nic_stats_clear(portid_t port_id)
>>   	ret = rte_eth_stats_reset(port_id);
>>   	if (ret != 0) {
>>   		printf("%s: Error: failed to reset stats (port %u): %s",
>> -		       __func__, port_id, strerror(ret));
>> +		       __func__, port_id, strerror(-ret));
>>   		return;
>>   	}
>>   
>>   	ret = rte_eth_stats_get(port_id, &ports[port_id].stats);
>>   	if (ret != 0) {
>>   		printf("%s: Error: failed to get stats (port %u): %s",
>> -		       __func__, port_id, strerror(ret));
>> +		       __func__, port_id, strerror(-ret));
> Although in practice this may be the case, the 'rte_eth_stats_get()' function
> documentation doesn't guarantee that return value will be negative, it says:
>
> "
>   * @return
>   *   Zero if successful. Non-zero otherwise
> "
>
> To be accurate, what do you think to adding a negative check for 'ret' before
> doing '-ret'?
OK, we will add a negative check in V2.

Thanks, Xavier


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] app/testpmd: fix passing negative parameter to strerror
  2020-06-05  2:57   ` Wei Hu (Xavier)
@ 2020-06-05  9:25     ` Ferruh Yigit
  2020-06-06  3:47       ` Wei Hu (Xavier)
  0 siblings, 1 reply; 7+ messages in thread
From: Ferruh Yigit @ 2020-06-05  9:25 UTC (permalink / raw)
  To: Wei Hu (Xavier); +Cc: dev

On 6/5/2020 3:57 AM, Wei Hu (Xavier) wrote:
> 
> 
> On 2020/6/5 0:30, Ferruh Yigit wrote:
>> On 6/4/2020 7:22 AM, Wei Hu (Xavier) wrote:
>>> Currently, there are coverity defect warnings as blow:
>>> Coverity issue:
>>>    In nic_stats_clear function:
>>>      CID 290021 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>>      5. negative_returns: ret is passed to a parameter that cannot be
>>>         negative.
>>>
>>>      CID 289974 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>>      6. negative_returns: ret is passed to a parameter that cannot be
>>>         negative.
>>>
>>>    In nic_xstats_clear function:
>>>      CID 289985 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>>      5. negative_returns: ret is passed to a parameter that cannot be
>>>         negative.
>>>
>>>      CID 289850 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>>      6. negative_returns: ret is passed to a parameter that cannot be
>>>         negative.
>> I guess these coverity IDs are from the internal coverity, because I can't find
>> them in the public coverity.
>> If it is internal, not sure about the benefit of documenting them in the commit
>> log, since no one except huawei can access them. What do you think to remove all
>> above reference?
> Yes, these are internal covertiy defects information. Maybe we can remove
> internal coverity CID xxxx and reserve the description of the defect in the
> commit log.
> 
> By the way,  when we access the "View defects" on the page of the public 
> coverity
> https://scan.coverity.com/projects/dpdk-data-plane-development-kit?tab=overview 
> 
> , The browser prompts HTTP ERROR 403.

Can you please try a few times, I am also getting same error time to time but it
works after some tr.

And when it is successful, I have following URL, not sure if it works but you
can try this too:
https://scan4.coverity.com/reports.htm#v22369/p10075


>>> This patch fixes them by passing '-ret' to the function strerror() when ret
>>> is negative.
>>>
>>> Fixes: af75078fece3 ("first public release")
>>> Fixes: 9eb974221f44 ("app/testpmd: fix statistics after reset")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>>> ---
>>>   app/test-pmd/config.c | 8 ++++----
>>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>> index 5381207..356d0d2 100644
>>> --- a/app/test-pmd/config.c
>>> +++ b/app/test-pmd/config.c
>>> @@ -244,14 +244,14 @@ nic_stats_clear(portid_t port_id)
>>>   	ret = rte_eth_stats_reset(port_id);
>>>   	if (ret != 0) {
>>>   		printf("%s: Error: failed to reset stats (port %u): %s",
>>> -		       __func__, port_id, strerror(ret));
>>> +		       __func__, port_id, strerror(-ret));
>>>   		return;
>>>   	}
>>>   
>>>   	ret = rte_eth_stats_get(port_id, &ports[port_id].stats);
>>>   	if (ret != 0) {
>>>   		printf("%s: Error: failed to get stats (port %u): %s",
>>> -		       __func__, port_id, strerror(ret));
>>> +		       __func__, port_id, strerror(-ret));
>> Although in practice this may be the case, the 'rte_eth_stats_get()' function
>> documentation doesn't guarantee that return value will be negative, it says:
>>
>> "
>>   * @return
>>   *   Zero if successful. Non-zero otherwise
>> "
>>
>> To be accurate, what do you think to adding a negative check for 'ret' before
>> doing '-ret'?
> OK, we will add a negative check in V2.
> 
> Thanks, Xavier
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-dev] [PATCH] app/testpmd: fix passing negative parameter to strerror
  2020-06-05  9:25     ` Ferruh Yigit
@ 2020-06-06  3:47       ` Wei Hu (Xavier)
  0 siblings, 0 replies; 7+ messages in thread
From: Wei Hu (Xavier) @ 2020-06-06  3:47 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev, xavier.huwei



On 2020/6/5 17:25, Ferruh Yigit wrote:
> On 6/5/2020 3:57 AM, Wei Hu (Xavier) wrote:
>>
>> On 2020/6/5 0:30, Ferruh Yigit wrote:
>>> On 6/4/2020 7:22 AM, Wei Hu (Xavier) wrote:
>>>> Currently, there are coverity defect warnings as blow:
>>>> Coverity issue:
>>>>     In nic_stats_clear function:
>>>>       CID 290021 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>>>       5. negative_returns: ret is passed to a parameter that cannot be
>>>>          negative.
>>>>
>>>>       CID 289974 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>>>       6. negative_returns: ret is passed to a parameter that cannot be
>>>>          negative.
>>>>
>>>>     In nic_xstats_clear function:
>>>>       CID 289985 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>>>       5. negative_returns: ret is passed to a parameter that cannot be
>>>>          negative.
>>>>
>>>>       CID 289850 (#1 of 1): Argument cannot be negative (NEGATIVE_RETURNS)
>>>>       6. negative_returns: ret is passed to a parameter that cannot be
>>>>          negative.
>>> I guess these coverity IDs are from the internal coverity, because I can't find
>>> them in the public coverity.
>>> If it is internal, not sure about the benefit of documenting them in the commit
>>> log, since no one except huawei can access them. What do you think to remove all
>>> above reference?
>> Yes, these are internal covertiy defects information. Maybe we can remove
>> internal coverity CID xxxx and reserve the description of the defect in the
>> commit log.
>>
>> By the way,  when we access the "View defects" on the page of the public
>> coverity
>> https://scan.coverity.com/projects/dpdk-data-plane-development-kit?tab=overview
>>
>> , The browser prompts HTTP ERROR 403.
> Can you please try a few times, I am also getting same error time to time but it
> works after some tr.
>
> And when it is successful, I have following URL, not sure if it works but you
> can try this too:
> https://scan4.coverity.com/reports.htm#v22369/p10075
I found the defects information after several retries,
and will send V3 using the coverity IDs from the public coverity.

Thanks, Xavier
>
>>>> This patch fixes them by passing '-ret' to the function strerror() when ret
>>>> is negative.
>>>>
>>>> Fixes: af75078fece3 ("first public release")
>>>> Fixes: 9eb974221f44 ("app/testpmd: fix statistics after reset")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>>>> ---
>>>>    app/test-pmd/config.c | 8 ++++----
>>>>    1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>>> index 5381207..356d0d2 100644
>>>> --- a/app/test-pmd/config.c
>>>> +++ b/app/test-pmd/config.c
>>>> @@ -244,14 +244,14 @@ nic_stats_clear(portid_t port_id)
>>>>    	ret = rte_eth_stats_reset(port_id);
>>>>    	if (ret != 0) {
>>>>    		printf("%s: Error: failed to reset stats (port %u): %s",
>>>> -		       __func__, port_id, strerror(ret));
>>>> +		       __func__, port_id, strerror(-ret));
>>>>    		return;
>>>>    	}
>>>>    
>>>>    	ret = rte_eth_stats_get(port_id, &ports[port_id].stats);
>>>>    	if (ret != 0) {
>>>>    		printf("%s: Error: failed to get stats (port %u): %s",
>>>> -		       __func__, port_id, strerror(ret));
>>>> +		       __func__, port_id, strerror(-ret));
>>> Although in practice this may be the case, the 'rte_eth_stats_get()' function
>>> documentation doesn't guarantee that return value will be negative, it says:
>>>
>>> "
>>>    * @return
>>>    *   Zero if successful. Non-zero otherwise
>>> "
>>>
>>> To be accurate, what do you think to adding a negative check for 'ret' before
>>> doing '-ret'?
>> OK, we will add a negative check in V2.
>>
>> Thanks, Xavier
>>
>


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-06-06  3:48 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-04  6:22 [dpdk-dev] [PATCH] app/testpmd: fix passing negative parameter to strerror Wei Hu (Xavier)
2020-06-04  8:45 ` Phil Yang
2020-06-04 10:51   ` Kevin Traynor
2020-06-04 16:30 ` Ferruh Yigit
2020-06-05  2:57   ` Wei Hu (Xavier)
2020-06-05  9:25     ` Ferruh Yigit
2020-06-06  3:47       ` Wei Hu (Xavier)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).