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 9F135A00C3; Sun, 27 Feb 2022 06:25:25 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2ACC34068A; Sun, 27 Feb 2022 06:25:25 +0100 (CET) Received: from out162-62-57-137.mail.qq.com (out162-62-57-137.mail.qq.com [162.62.57.137]) by mails.dpdk.org (Postfix) with ESMTP id DBF7B40683 for ; Sun, 27 Feb 2022 06:25:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foxmail.com; s=s201512; t=1645939520; bh=g2hRg9KAh6CUbuRRBpM5P6xIUAHN38GBZFnPjkXFYMY=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=HtYMM3L7LfJW63+WtY1zUd8TQXJMy4lDTSp6CVPqcPLC/bFmncvYtuTohJsHd3YUy hMRQd2okS5zUhAySS/sGHVZAvynUBu+tdgup6KDWO2Ej/fvnOBCQQilO4azvGCwGMg k95QlBnbHflpRLNWrj0wioAN6mHZE0nxEXWep2m4= Received: from localhost.localdomain ([111.193.130.237]) by newxmesmtplogicsvrszb6.qq.com (NewEsmtp) with SMTP id 5F5B4E20; Sun, 27 Feb 2022 13:23:53 +0800 X-QQ-mid: xmsmtpt1645939433tfnvvljlq Message-ID: X-QQ-XMAILINFO: MZmsIF9YmPJxYbJBXGZGWoElI0K6HVml3H3v2rrCB+FrM81zOACjnD0FP5xXxj FDnOKSvE6gRwz06vFdfkP/CrXCjlV8In/2B0pUvIh8JhnYVbwqdJTbTj/Wyu6j4zTYZTyNyEjetM VIm1oTwCp4LIjVfB41d6xx+c5pAokpqvcjtFfkv8T49woY48PcO2tCyorbJY2HkF8s3s2VYNrv7U EX1omARgyqTyPE2e2ztXB7uEutCnhMyH2/dd24C5OUnheNzFb+Eue9BQyIvVVYNoK2f8kfELm29u oZ7C4mWadZ7BafqUIUBKGdLf7X6mSmFHWthX4iw3kKZFq3YFmyDLmRPfZ2y0iX5pgGhcl+4EOGKu SUxVXvDMnpbPswX/UFJ9mCy7L/lyhxT5uMA3RdqURrZBmQeA0VZpYl2iEuFxI50ckxdoLEOj4jrL SF6VbTWINnxq+FgWqylRiRqJZICBDmnr6htRXH0btHAos3NtSImKxDG7MGVwrgvsWaYJzYYP7aGF iU1woIxZzSoBuRBGRkgDHr9F5k+SaNrMpfArKctQwSKDLdN0U+nsVxnB+8Ym+4lWHHiB9R8Eh4ey uuCxVhmlpLFmNCmRCPSpv834MOZFPMdVRq8ZVOKFBkWVe9OXlimAiCSvxfWH+nGsqBIl+SveiZx6 Fgxlvih6UbA92HmNct1fNkYjQGJ9lk06SvdIkSOCNe8zMyvtjdKsdnWkW27UurU3SVGp862r5b7n 5g/pTMj5PJrswlixmmcNO/lU2mBSzqWmuDQfCDfbKjtRaOJ4zIS3znUTpwbhrFd1VqKA9mkBMvM4 4Cihp+FbmxsTnEjSsRE3OrJBFpTNDxzeRKvNz4ew4eW+xA9zQYOiFia4iYqNVcRW4CZykWoBkQsb f9Z1j+cG3pqv09G8W2Q+hMCQLfnzy6Al9Rl15V9qGIyrIZ+6k8RHE/OyRts7bBvg== From: Weiguo Li To: stephen@networkplumber.org Cc: cristian.dumitrescu@intel.com, wojciechx.liguzinski@intel.com, dev@dpdk.org Subject: Re: [PATCH] sched: add parentheses to if clause Date: Sun, 27 Feb 2022 13:23:53 +0800 X-OQ-MSGID: <20220227052353.375603-1-liwg06@foxmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220226093137.3723b20f@hermes.local> References: <20220226093137.3723b20f@hermes.local> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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 On Sat, 26 Feb 2022 09:31:37 -0800, Stephen Hemminger wrote: > > Add parentheses to 'if' clause, otherwise will enlarged the > > chance of error return. > > > > Fixes: 44c730b0e37971 ("sched: add PIE based congestion management") > > > > Signed-off-by: Weiguo Li > > --- > > lib/sched/rte_pie.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/lib/sched/rte_pie.c b/lib/sched/rte_pie.c > > index cdb7bab697..51df403a25 100644 > > --- a/lib/sched/rte_pie.c > > +++ b/lib/sched/rte_pie.c > > @@ -18,10 +18,10 @@ rte_pie_rt_data_init(struct rte_pie *pie) > > /* Allocate memory to use the PIE data structure */ > > pie = rte_malloc(NULL, sizeof(struct rte_pie), 0); > > > > - if (pie == NULL) > > + if (pie == NULL) { > > RTE_LOG(ERR, SCHED, "%s: Memory allocation fails\n", __func__); > > - > > - return -1; > > + return -1; > > + } > > } > > > > pie->active = 0; > > This will make the test in test_pie.c fail. > > The concept of passing NULL to the routine and expecting allocation > is bad idea because the allocated structure is never initialized. > > Since rte_pie_rt_data_init(NULL) always returned -1. > It would make more sense to take out the rte_malloc(). > And document it. > > P.s: the routing should return a negative rte_errno instead of -1 > as well. > Hi Stephen, The 'rte_malloc' and null check is really misleading at the first sight... Thanks for your suggestion! -Weiguo