From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id B3BE4A0350; Sat, 6 Jun 2020 05:48:01 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 066E11C194; Sat, 6 Jun 2020 05:48:01 +0200 (CEST) Received: from huawei.com (szxga07-in.huawei.com [45.249.212.35]) by dpdk.org (Postfix) with ESMTP id DEC6B1C13A for ; Sat, 6 Jun 2020 05:47:59 +0200 (CEST) Received: from DGGEMS401-HUB.china.huawei.com (unknown [172.30.72.58]) by Forcepoint Email with ESMTP id 2D597EC3FABFF74A8721 for ; Sat, 6 Jun 2020 11:47:58 +0800 (CST) Received: from [10.69.31.206] (10.69.31.206) by DGGEMS401-HUB.china.huawei.com (10.3.19.201) with Microsoft SMTP Server id 14.3.487.0; Sat, 6 Jun 2020 11:47:51 +0800 To: Ferruh Yigit References: <1591251737-64598-1-git-send-email-xavier.huwei@huawei.com> <66ad25cf-1cdb-c470-ae8a-260929ff6fab@huawei.com> <0c7c5502-def3-2cc7-85a6-f7588b28b758@intel.com> CC: , From: "Wei Hu (Xavier)" Message-ID: <469aaa40-6794-5062-708b-437a3f9865c8@huawei.com> Date: Sat, 6 Jun 2020 11:47:52 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <0c7c5502-def3-2cc7-85a6-f7588b28b758@intel.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.69.31.206] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix passing negative parameter to strerror X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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) >>>> --- >>>> 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 >> >