From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id EB77FA0C50; Sat, 24 Jul 2021 02:35:35 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 679E740687; Sat, 24 Jul 2021 02:35:35 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id D5A2B40041 for ; Sat, 24 Jul 2021 02:35:33 +0200 (CEST) Received: from dggeme756-chm.china.huawei.com (unknown [172.30.72.57]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4GWnFL2cH3zZsD1; Sat, 24 Jul 2021 08:32:06 +0800 (CST) Received: from [10.67.103.128] (10.67.103.128) by dggeme756-chm.china.huawei.com (10.3.19.102) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2176.2; Sat, 24 Jul 2021 08:35:30 +0800 To: Andrew Rybchenko , David Hunt , CC: , , , , , Thomas Monjalon References: <1618835391-54882-1-git-send-email-humin29@huawei.com> <334051b8-9532-3059-ec00-3ed690701190@intel.com> <77c41302-9f93-8258-d42f-f3045d7d1f5f@oktetlabs.ru> From: "Min Hu (Connor)" Message-ID: <91596745-5cca-73d7-3a01-043bfe55c70d@huawei.com> Date: Sat, 24 Jul 2021 08:35:30 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 MIME-Version: 1.0 In-Reply-To: <77c41302-9f93-8258-d42f-f3045d7d1f5f@oktetlabs.ru> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.128] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To dggeme756-chm.china.huawei.com (10.3.19.102) X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH] app: fix redundant comparison X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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" Agreed, This patch can be rejected, thanks. 在 2021/7/24 4:16, Andrew Rybchenko 写道: > On 4/19/21 3:44 PM, David Hunt wrote: >> Hi Connor, >> >> On 19/4/2021 1:29 PM, Min Hu (Connor) wrote: >>> The return value of 'rte_eal_cleanup' is always zero, so comparison >>> with zero is redundant. >>> >>> This patch fixed it by deleting the redundant comparison. >>> >>> Fixes: 67684d1e87b6 ("app/procinfo: call EAL cleanup before exit") >>> Fixes: b68a82425da4 ("app/compress-perf: add performance measurement") >>> Fixes: 5e516c89830a ("app/testpmd: call cleanup on exit") >>> Fixes: 613ce6691c0d ("examples/l3fwd-power: implement proper shutdown") >>> Fixes: 48be180de631 ("eal: move OS common debug functions to single >>> file") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Min Hu (Connor) >>> --- >>>   app/pdump/main.c                         | 4 +--- >>>   app/proc-info/main.c                     | 4 +--- >>>   app/test-compress-perf/main.c            | 7 +------ >>>   app/test-pmd/testpmd.c                   | 5 +---- >>>   examples/l3fwd-power/main.c              | 3 +-- >>>   lib/librte_eal/common/eal_common_debug.c | 4 +--- >>>   6 files changed, 6 insertions(+), 21 deletions(-) >>> >>> diff --git a/app/pdump/main.c b/app/pdump/main.c >>> index 63bbe65..33523c5 100644 >>> --- a/app/pdump/main.c >>> +++ b/app/pdump/main.c >>> @@ -1014,9 +1014,7 @@ main(int argc, char **argv) >>>       /* dump debug stats */ >>>       print_pdump_stats(); >>> -    ret = rte_eal_cleanup(); >>> -    if (ret) >>> -        printf("Error from rte_eal_cleanup(), %d\n", ret); >>> +    rte_eal_cleanup(); >>>       return 0; >>>   } >>> diff --git a/app/proc-info/main.c b/app/proc-info/main.c >>> index b9587f7..9498490 100644 >>> --- a/app/proc-info/main.c >>> +++ b/app/proc-info/main.c >>> @@ -1458,9 +1458,7 @@ main(int argc, char **argv) >>>       RTE_ETH_FOREACH_DEV(i) >>>           rte_eth_dev_close(i); >>> -    ret = rte_eal_cleanup(); >>> -    if (ret) >>> -        printf("Error from rte_eal_cleanup(), %d\n", ret); >>> +    rte_eal_cleanup(); >>>       return 0; >>>   } >>> diff --git a/app/test-compress-perf/main.c >>> b/app/test-compress-perf/main.c >>> index cc9951a..03ee385 100644 >>> --- a/app/test-compress-perf/main.c >>> +++ b/app/test-compress-perf/main.c >>> @@ -474,12 +474,7 @@ main(int argc, char **argv) >>>           /* fallthrough */ >>>       case ST_CLEAR: >>>       default: >>> -        i = rte_eal_cleanup(); >>> -        if (i) { >>> -            RTE_LOG(ERR, USER1, >>> -                "Error from rte_eal_cleanup(), %d\n", i); >>> -            ret = i; >>> -        } >>> +        rte_eal_cleanup(); >>>           break; >>>       } >>>       return ret; >>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c >>> index d4be23f..1f8a7b8 100644 >>> --- a/app/test-pmd/testpmd.c >>> +++ b/app/test-pmd/testpmd.c >>> @@ -3972,10 +3972,7 @@ main(int argc, char** argv) >>>               return 1; >>>       } >>> -    ret = rte_eal_cleanup(); >>> -    if (ret != 0) >>> -        rte_exit(EXIT_FAILURE, >>> -             "EAL cleanup failed: %s\n", strerror(-ret)); >>> +    rte_eal_cleanup(); >>>       return EXIT_SUCCESS; >>>   } >>> diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c >>> index 0af8810..84464d5 100644 >>> --- a/examples/l3fwd-power/main.c >>> +++ b/examples/l3fwd-power/main.c >>> @@ -2914,8 +2914,7 @@ main(int argc, char **argv) >>>               deinit_power_library()) >>>           rte_exit(EXIT_FAILURE, "deinit_power_library failed\n"); >>> -    if (rte_eal_cleanup() < 0) >>> -        RTE_LOG(ERR, L3FWD_POWER, "EAL cleanup failed\n"); >>> +    rte_eal_cleanup(); >>>       return 0; >>>   } >>> diff --git a/lib/librte_eal/common/eal_common_debug.c >>> b/lib/librte_eal/common/eal_common_debug.c >>> index 15418e9..04b1fed 100644 >>> --- a/lib/librte_eal/common/eal_common_debug.c >>> +++ b/lib/librte_eal/common/eal_common_debug.c >>> @@ -37,8 +37,6 @@ rte_exit(int exit_code, const char *format, ...) >>>       rte_vlog(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, format, ap); >>>       va_end(ap); >>> -    if (rte_eal_cleanup() != 0) >>> -        RTE_LOG(CRIT, EAL, >>> -            "EAL could not release all resources\n"); >>> +    rte_eal_cleanup(); >>>       exit(exit_code); >>>   } >> >> >> Since the function is defined as "int rte_eal_cleanup(void)" and the >> documentation says: >> >>   *  - 0 Successfully released all internal EAL resources. >>   *  - -EFAULT There was an error in releasing all resources. >> >> We really should check for non zero values. Just because it's >> currently coded to only return zero may not hold in the future. The >> code may change in the future to return non-zero values. > > +1 > > I think the patch should be rejected. > .