Message ID | 1418895707-468-1-git-send-email-danielx.t.mrzyglod@intel.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 46199803F; Thu, 18 Dec 2014 15:10:07 +0100 (CET) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id B36DC1F5 for <dev@dpdk.org>; Thu, 18 Dec 2014 15:10:04 +0100 (CET) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 18 Dec 2014 06:05:57 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,601,1413270000"; d="scan'208";a="656389400" Received: from irvmail001.ir.intel.com ([163.33.26.43]) by orsmga002.jf.intel.com with ESMTP; 18 Dec 2014 06:08:17 -0800 Received: from sivswdev01.ir.intel.com (sivswdev01.ir.intel.com [10.237.217.45]) by irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id sBI9foMP005414; Thu, 18 Dec 2014 09:41:50 GMT Received: from sivswdev01.ir.intel.com (localhost [127.0.0.1]) by sivswdev01.ir.intel.com with ESMTP id sBI9fo3N000528; Thu, 18 Dec 2014 09:41:50 GMT Received: (from dtmrzglx@localhost) by sivswdev01.ir.intel.com with id sBI9foj3000521; Thu, 18 Dec 2014 09:41:50 GMT From: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com> To: dev@dpdk.org Date: Thu, 18 Dec 2014 09:41:47 +0000 Message-Id: <1418895707-468-1-git-send-email-danielx.t.mrzyglod@intel.com> X-Mailer: git-send-email 1.7.4.1 Subject: [dpdk-dev] [PATCH] test: fix missing NULL pointer checks X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Daniel Mrzyglod
Dec. 18, 2014, 9:41 a.m. UTC
In test_sched, we are missing NULL pointer checks after calls to create the
mempool and to allocate an mbuf. Add in these checks using VERIFY macros.
Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
---
app/test/test_sched.c | 2 ++
1 file changed, 2 insertions(+)
Comments
2014-12-18 09:41, Daniel Mrzyglod: > In test_sched, we are missing NULL pointer checks after calls to create the > mempool and to allocate an mbuf. Add in these checks using VERIFY macros. > > Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com> > --- > app/test/test_sched.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/app/test/test_sched.c b/app/test/test_sched.c > index c957d80..9b6e037 100644 > --- a/app/test/test_sched.c > +++ b/app/test/test_sched.c > @@ -166,6 +166,7 @@ test_sched(void) > int err; > > mp = create_mempool(); > + VERIFY(mp != NULL,"Error create mempool\n"); A space is missing after the comma. Is "Error creating mempool" more correct? > port_param.socket = 0; > port_param.rate = (uint64_t) 10000 * 1000 * 1000 / 8; > @@ -184,6 +185,7 @@ test_sched(void) > > for (i = 0; i < 10; i++) { > in_mbufs[i] = rte_pktmbuf_alloc(mp); > + VERIFY(in_mbufs[i] != NULL, "Bad packet allocation"); An \n is missing. "Packet allocation failed" seems more appropriate.
On Thu, Dec 18, 2014 at 09:41:47AM +0000, Daniel Mrzyglod wrote: > In test_sched, we are missing NULL pointer checks after calls to create the > mempool and to allocate an mbuf. Add in these checks using VERIFY macros. > > Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com> > --- > app/test/test_sched.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/app/test/test_sched.c b/app/test/test_sched.c > index c957d80..9b6e037 100644 > --- a/app/test/test_sched.c > +++ b/app/test/test_sched.c > @@ -166,6 +166,7 @@ test_sched(void) > int err; > > mp = create_mempool(); > + VERIFY(mp != NULL,"Error create mempool\n"); > > port_param.socket = 0; > port_param.rate = (uint64_t) 10000 * 1000 * 1000 / 8; > @@ -184,6 +185,7 @@ test_sched(void) > > for (i = 0; i < 10; i++) { > in_mbufs[i] = rte_pktmbuf_alloc(mp); > + VERIFY(in_mbufs[i] != NULL, "Bad packet allocation"); > prepare_pkt(in_mbufs[i]); > } > > -- > 2.1.0 > > Looking at the VERIFY macro, its defined as: #define VERIFY(exp,fmt,args...) \ if (!(exp)) { \ printf(fmt, ##args); return -1; \ } Thats really bad practice, as it embodies a return into the VERIFY macro, creating hidden function exit points that the programmer can't clean up within. Every use of the VERIFY macro in test_sched causes the program to return without freeing any of the memory allocated in the function (not that the function is any good at cleaning up after itself anyway), but I would recommend that you modify the macro as such: #define VERIFY(exp, fmt, args...) \ if (!(exp)) { \ printf(fmt, ##args); \ 0;\ } else \ 1;\ } That way you can use the macro like this: if (VERIFY(in_mbufs[i] != NULL, "Bad packet allocation") { //Insert cleanup code here } Neil
Ping. What next for this patch? 2014-12-18 22:05, Thomas Monjalon: > 2014-12-18 09:41, Daniel Mrzyglod: > > In test_sched, we are missing NULL pointer checks after calls to create the > > mempool and to allocate an mbuf. Add in these checks using VERIFY macros. > > > > Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com> > > --- > > app/test/test_sched.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/app/test/test_sched.c b/app/test/test_sched.c > > index c957d80..9b6e037 100644 > > --- a/app/test/test_sched.c > > +++ b/app/test/test_sched.c > > @@ -166,6 +166,7 @@ test_sched(void) > > int err; > > > > mp = create_mempool(); > > + VERIFY(mp != NULL,"Error create mempool\n"); > > A space is missing after the comma. > Is "Error creating mempool" more correct? > > > port_param.socket = 0; > > port_param.rate = (uint64_t) 10000 * 1000 * 1000 / 8; > > @@ -184,6 +185,7 @@ test_sched(void) > > > > for (i = 0; i < 10; i++) { > > in_mbufs[i] = rte_pktmbuf_alloc(mp); > > + VERIFY(in_mbufs[i] != NULL, "Bad packet allocation"); > > An \n is missing. > "Packet allocation failed" seems more appropriate.
diff --git a/app/test/test_sched.c b/app/test/test_sched.c index c957d80..9b6e037 100644 --- a/app/test/test_sched.c +++ b/app/test/test_sched.c @@ -166,6 +166,7 @@ test_sched(void) int err; mp = create_mempool(); + VERIFY(mp != NULL,"Error create mempool\n"); port_param.socket = 0; port_param.rate = (uint64_t) 10000 * 1000 * 1000 / 8; @@ -184,6 +185,7 @@ test_sched(void) for (i = 0; i < 10; i++) { in_mbufs[i] = rte_pktmbuf_alloc(mp); + VERIFY(in_mbufs[i] != NULL, "Bad packet allocation"); prepare_pkt(in_mbufs[i]); }