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 731B5A00C4; Fri, 5 Jun 2020 04:57:50 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 62EC91D607; Fri, 5 Jun 2020 04:57:49 +0200 (CEST) Received: from huawei.com (szxga05-in.huawei.com [45.249.212.191]) by dpdk.org (Postfix) with ESMTP id 3447C1D5ED for ; Fri, 5 Jun 2020 04:57:47 +0200 (CEST) Received: from DGGEMS405-HUB.china.huawei.com (unknown [172.30.72.58]) by Forcepoint Email with ESMTP id 06652B16504048F2DC28 for ; Fri, 5 Jun 2020 10:57:46 +0800 (CST) Received: from [10.69.31.206] (10.69.31.206) by DGGEMS405-HUB.china.huawei.com (10.3.19.205) with Microsoft SMTP Server id 14.3.487.0; Fri, 5 Jun 2020 10:57:38 +0800 To: Ferruh Yigit References: <1591251737-64598-1-git-send-email-xavier.huwei@huawei.com> CC: , From: "Wei Hu (Xavier)" Message-ID: <66ad25cf-1cdb-c470-ae8a-260929ff6fab@huawei.com> Date: Fri, 5 Jun 2020 10:57:38 +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: 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 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) >> --- >> 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