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 83600A0C50; Fri, 23 Jul 2021 22:16:38 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EF8DD40DDA; Fri, 23 Jul 2021 22:16:37 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id B51CA406A3 for ; Fri, 23 Jul 2021 22:16:36 +0200 (CEST) Received: from [192.168.100.116] (unknown [37.139.99.76]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id E20617F553; Fri, 23 Jul 2021 23:16:35 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru E20617F553 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1627071396; bh=3hj/9aFFBdN8FVIVTjnVmqD+KRv64QUy9kb1aqHPCOg=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=aOKj4RUC8gWiarvXoH02Jfsk3gDt50VKGM03z/Vzg27TpC3hnx6ehe6wBWGdV3jnj LWNMsGLxzbIqpEXIWIigyXB43qHkQZyg3g+OdnZho50SCI59Dx2IfdHfOMFJDI9kzH oRgKMDH3pIm11/KiaUGoyojoV9XHc+F9nhU7fPnY= To: David Hunt , "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, Thomas Monjalon References: <1618835391-54882-1-git-send-email-humin29@huawei.com> <334051b8-9532-3059-ec00-3ed690701190@intel.com> From: Andrew Rybchenko Message-ID: <77c41302-9f93-8258-d42f-f3045d7d1f5f@oktetlabs.ru> Date: Fri, 23 Jul 2021 23:16:35 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <334051b8-9532-3059-ec00-3ed690701190@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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" 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.