[dpdk-dev] [PATCH] app/testpmd: fix passing negative parameter to strerror

[Wei Hu (Xavier)]

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 at dpdk.org
>>>>
>>>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei at 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
>>
>