From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id C883C3772 for ; Tue, 16 Jan 2018 15:04:33 +0100 (CET) Received: from lfbn-lil-1-110-231.w90-45.abo.wanadoo.fr ([90.45.197.231] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1ebRqr-0000A7-Uz; Tue, 16 Jan 2018 15:04:35 +0100 Received: by droids-corp.org (sSMTP sendmail emulation); Tue, 16 Jan 2018 15:04:29 +0100 Date: Tue, 16 Jan 2018 15:04:29 +0100 From: Olivier Matz To: Keith Wiles Cc: dev@dpdk.org, stephen@networkplumber.org, jerin.jacob@caviumnetworks.com Message-ID: <20180116140429.pyab272uebox2gmn@platinum> References: <20180108153423.57648-1-keith.wiles@intel.com> <20180109142928.81687-1-keith.wiles@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180109142928.81687-1-keith.wiles@intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v2] mbuf:using sanity checks do not panic on null mbuf X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Jan 2018 14:04:33 -0000 On Tue, Jan 09, 2018 at 08:29:28AM -0600, Keith Wiles wrote: > The rte_pktmbuf_free() allows for NULL mbuf pointer, but > when sanity check is enabled it will panic with null pointer. > > Signed-off-by: Keith Wiles > --- > lib/librte_mbuf/rte_mbuf.c | 10 ++++++++-- > test/test/test_mbuf.c | 4 +--- > 2 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c > index 7543662f7..621679c92 100644 > --- a/lib/librte_mbuf/rte_mbuf.c > +++ b/lib/librte_mbuf/rte_mbuf.c > @@ -205,8 +205,9 @@ rte_mbuf_sanity_check(const struct rte_mbuf *m, int is_header) > const struct rte_mbuf *m_seg; > unsigned int nb_segs; > > - if (m == NULL) > - rte_panic("mbuf is NULL\n"); > + /* Calling with NULL is valid in the API */ > + if (!m) > + return; > > /* generic checks */ > if (m->pool == NULL) > @@ -243,6 +244,11 @@ rte_pktmbuf_dump(FILE *f, const struct rte_mbuf *m, unsigned dump_len) > > __rte_mbuf_sanity_check(m, 1); > > + if (!m) { > + fprintf(stderr, "MBUF pointer is NULL\n"); > + return; > + } > + > fprintf(f, "dump mbuf at %p, iova=%"PRIx64", buf_len=%u\n", > m, (uint64_t)m->buf_iova, (unsigned)m->buf_len); > fprintf(f, " pkt_len=%"PRIu32", ol_flags=%"PRIx64", nb_segs=%u, " > diff --git a/test/test/test_mbuf.c b/test/test/test_mbuf.c > index 9e82a20be..146eaf0e5 100644 > --- a/test/test/test_mbuf.c > +++ b/test/test/test_mbuf.c > @@ -864,10 +864,8 @@ test_failing_mbuf_sanity_check(struct rte_mempool *pktmbuf_pool) > > printf("Now checking for error conditions\n"); > > - if (verify_mbuf_check_panics(NULL)) { > - printf("Error with NULL mbuf test\n"); > + if (verify_mbuf_check_panics(NULL) != -1) > return -1; > - } > > badbuf = *buf; > badbuf.pool = NULL; > -- > 2.14.1 > The problem is a panic when rte_pktmbuf_free(NULL) when mbuf debug is enabled, right? A NULL mbuf is only valid in case of a free because it is convenient, but for most (all ?) other mbuf functions, the mbuf must not be NULL. What about this patch instead: --- a/lib/librte_mbuf/rte_mbuf.h +++ b/lib/librte_mbuf/rte_mbuf.h @@ -1413,13 +1413,14 @@ rte_pktmbuf_free_seg(struct rte_mbuf *m) * segment is added back into its original mempool. * * @param m - * The packet mbuf to be freed. + * The packet mbuf to be freed. If NULL, the function does nothing. */ static inline void rte_pktmbuf_free(struct rte_mbuf *m) { struct rte_mbuf *m_next; - __rte_mbuf_sanity_check(m, 1); + if (m != NULL) + __rte_mbuf_sanity_check(m, 1); while (m != NULL) { m_next = m->next;