From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 1CCB711F5 for ; Sun, 28 Sep 2014 14:18:57 +0200 (CEST) Received: from [2001:470:8:a08:215:ff:fecc:4872] (helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1XYDXU-0004xO-Ks; Sun, 28 Sep 2014 08:25:26 -0400 Date: Sun, 28 Sep 2014 08:25:17 -0400 From: Neil Horman To: "Wiles, Roger Keith" Message-ID: <20140928122516.GA30445@localhost.localdomain> References: <20140928003719.GA27849@localhost.localdomain> <20140928015504.GA28263@localhost.localdomain> <831919DA-CEF0-4DA5-AF5D-5B1067F7A602@windriver.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <831919DA-CEF0-4DA5-AF5D-5B1067F7A602@windriver.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: "" Subject: Re: [dpdk-dev] [PATCH] rte_mempool_dump() crashes with NULL rte_mempool pointer. X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 28 Sep 2014 12:18:58 -0000 On Sun, Sep 28, 2014 at 05:38:06AM +0000, Wiles, Roger Keith wrote: > > On Sep 27, 2014, at 8:55 PM, Neil Horman wrote: > > > On Sun, Sep 28, 2014 at 01:14:05AM +0000, Wiles, Roger Keith wrote: > >> > >> On Sep 27, 2014, at 7:37 PM, Neil Horman wrote: > >> > >>> On Sat, Sep 27, 2014 at 06:35:01PM +0000, Wiles, Roger Keith wrote: > >>>> > >>>> Check the FILE *f and rte_mempool *mp pointers for NULL and > >>>> return plus print out a message if RTE_LIBRTE_MEMPOOL_DEBUG is enabled. > >>>> > >>>> Signed-off-by: Keith Wiles > >>>> --- > >>>> lib/librte_mempool/rte_mempool.c | 6 ++++++ > >>>> 1 file changed, 6 insertions(+) > >>>> > >>>> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c > >>>> index 332f469..efa6a6c 100644 > >>>> --- a/lib/librte_mempool/rte_mempool.c > >>>> +++ b/lib/librte_mempool/rte_mempool.c > >>>> @@ -765,6 +765,12 @@ rte_mempool_dump(FILE *f, const struct rte_mempool *mp) > >>>> unsigned common_count; > >>>> unsigned cache_count; > >>>> > >>>> + if ( (f == NULL) || (mp == NULL) ) { > >>>> +#ifdef RTE_LIBRTE_MEMPOOL_DEBUG > >>>> + fprintf(stderr, "*** Called rte_mempool_dump(%p, %p) with NULL argument\n", f, mp); > >>>> +#endif /* RTE_LIBRTE_MEMPOOL_DEBUG */ > >>>> + return; > >>>> + } > >>>> fprintf(f, "mempool <%s>@%p\n", mp->name, mp); > >>>> fprintf(f, " flags=%x\n", mp->flags); > >>>> fprintf(f, " ring=<%s>@%p\n", mp->ring->name, mp->ring); > >>>> -- > >>>> 2.1.0 > >>>> > >>>> > >>> Maybe use RTE_VERIFY instead? > >>> Neil > >>> > >> I did not think it needs to panic as it is just a debug function and returning would be fine by me, comments? > >> Do we have a similar RTE_VERIFY like function that does not panic? > >> > > If we don't, it would seem useful to make one. It beats having to do specific > > condition checking/error reporting. RTE_VERIFY_WARN or some such. > > Neil > > I decided to just use RTE_VERIFY() instead of creating a new macro for now, it seems this maybe an isolated case. I agree having RTE_VERIFY_WARN() would be nice, but as I was writing the macro I wanted to return from the function. For this routine ‘return’ would work as it returns (void), but for other routines a value may need to be returned. > Thats fine, you can do exactly what you need to do, just write the macro to assert !!condition at the end, like this: #define RTE_VERIFY_WARN(condition) do { \ int ret = !!condition; \ if (ret) \ printf(); \ ret;\ } Then, you can use the macro as a conditional itself anywhere you want: int function(void *arguments) { if (RTE_VERIFY(arguments == NULL)) return 1 .... } > Need a clean way to exit the routine without causing the macro to understand its return values. Just seem to become a bit messy at this point. Multiple macros for different return types or make the macros return a boolean value to be tested seemed to more complex then needed. See above, thats how all the Linux WARN_ON macros work. Neil > > > >> Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533 > > Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533 > >