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 812F4A0524; Mon, 19 Apr 2021 14:44:32 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5158D41283; Mon, 19 Apr 2021 14:44:32 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id 5CEAF41283 for ; Mon, 19 Apr 2021 14:44:30 +0200 (CEST) IronPort-SDR: 0z1QzmG8jtwKEECiBCRugdvn5kmNKSTNz4I+W1apYSOJX6Z5BWTprVoOzAK4cCjD53YnoeAJkY w1y/l1CbGZEA== X-IronPort-AV: E=McAfee;i="6200,9189,9958"; a="256629344" X-IronPort-AV: E=Sophos;i="5.82,234,1613462400"; d="scan'208";a="256629344" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Apr 2021 05:44:27 -0700 IronPort-SDR: 1ooprwXNj17yGYzwjLmfDkZVJdj/zFfmGFmvuGTOfy8OfyyqwmUsu8+YGJ2/nJmLeSVxdxUe29 4wlm+rzS2VFQ== X-IronPort-AV: E=Sophos;i="5.82,234,1613462400"; d="scan'208";a="523418289" Received: from dhunt5-mobl5.ger.corp.intel.com (HELO [10.252.8.120]) ([10.252.8.120]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Apr 2021 05:44:25 -0700 To: "Min Hu (Connor)" , dev@dpdk.org Cc: ferruh.yigit@intel.com, maryam.tahhan@intel.com, reshma.pattan@intel.com, xiaoyun.li@intel.com, jerinj@marvell.com References: <1618835391-54882-1-git-send-email-humin29@huawei.com> From: David Hunt Message-ID: <334051b8-9532-3059-ec00-3ed690701190@intel.com> Date: Mon, 19 Apr 2021 13:44:23 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.9.0 MIME-Version: 1.0 In-Reply-To: <1618835391-54882-1-git-send-email-humin29@huawei.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB 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" 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. Rgds, Dave.