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 D76F05A3F for ; Tue, 27 Jan 2015 19:06:48 +0100 (CET) Received: from hmsreliant.think-freely.org ([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1YGAXB-0004j5-Ct; Tue, 27 Jan 2015 13:06:47 -0500 Date: Tue, 27 Jan 2015 13:06:40 -0500 From: Neil Horman To: Daniel Mrzyglod Message-ID: <20150127180640.GB20118@hmsreliant.think-freely.org> References: <1422373493-9816-1-git-send-email-danielx.t.mrzyglod@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1422373493-9816-1-git-send-email-danielx.t.mrzyglod@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v3] 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Jan 2015 18:06:49 -0000 On Tue, Jan 27, 2015 at 04:44:53PM +0100, Daniel Mrzyglod wrote: > In test_sched, we are missing NULL pointer checks after create_mempool() > and rte_pktmbuf_alloc(). Add in these checks using TEST_ASSERT_NOT_NULL macros. > > VERIFY macro was removed and replaced by standard test ASSERTS from "test.h" header. > This provides additional information to track when the failure occured. > > v3 changes: > - remove VERIFY macro > - fix spelling error. > - change unproper comment > > v2 changes: > - Replace all VERIFY macros instances by proper TEST_ASSERT* macros. > - fix description > > v1 changes: > - first iteration of patch using VERIFY macro. > > Signed-off-by: Daniel Mrzyglod > --- > app/test/test_sched.c | 39 ++++++++++++++++++--------------------- > 1 file changed, 18 insertions(+), 21 deletions(-) > > diff --git a/app/test/test_sched.c b/app/test/test_sched.c > index c957d80..60c62de 100644 > --- a/app/test/test_sched.c > +++ b/app/test/test_sched.c > @@ -46,13 +46,6 @@ > #include > > > -#define VERIFY(exp,fmt,args...) \ > - if (!(exp)) { \ > - printf(fmt, ##args); \ > - return -1; \ > - } > - > - > #define SUBPORT 0 > #define PIPE 1 > #define TC 2 > @@ -166,48 +159,49 @@ test_sched(void) > int err; > > mp = create_mempool(); > + TEST_ASSERT_NOT_NULL(mp, "Error creating mempool\n"); > > port_param.socket = 0; > port_param.rate = (uint64_t) 10000 * 1000 * 1000 / 8; > > port = rte_sched_port_config(&port_param); > - VERIFY(port != NULL, "Error config sched port\n"); > - > + TEST_ASSERT_NOT_NULL(port, "Error config sched port\n"); > > err = rte_sched_subport_config(port, SUBPORT, subport_param); > - VERIFY(err == 0, "Error config sched, err=%d\n", err); > + TEST_ASSERT_SUCCESS(err, "Error config sched, err=%d\n", err); > > for (pipe = 0; pipe < port_param.n_pipes_per_subport; pipe ++) { > err = rte_sched_pipe_config(port, SUBPORT, pipe, 0); > - VERIFY(err == 0, "Error config sched pipe %u, err=%d\n", pipe, err); > + TEST_ASSERT_SUCCESS(err, "Error config sched pipe %u, err=%d\n", pipe, err); > } > > for (i = 0; i < 10; i++) { > in_mbufs[i] = rte_pktmbuf_alloc(mp); > + TEST_ASSERT_NOT_NULL(in_mbufs[i], "Packet allocation failed\n"); > prepare_pkt(in_mbufs[i]); > } > > > err = rte_sched_port_enqueue(port, in_mbufs, 10); > - VERIFY(err == 10, "Wrong enqueue, err=%d\n", err); > + TEST_ASSERT_EQUAL(err, 10, "Wrong enqueue, err=%d\n", err); > > err = rte_sched_port_dequeue(port, out_mbufs, 10); > - VERIFY(err == 10, "Wrong dequeue, err=%d\n", err); > + TEST_ASSERT_EQUAL(err, 10, "Wrong dequeue, err=%d\n", err); > > for (i = 0; i < 10; i++) { > enum rte_meter_color color; > uint32_t subport, traffic_class, queue; > > color = rte_sched_port_pkt_read_color(out_mbufs[i]); > - VERIFY(color == e_RTE_METER_YELLOW, "Wrong color\n"); > + TEST_ASSERT_EQUAL(color, e_RTE_METER_YELLOW, "Wrong color\n"); > > rte_sched_port_pkt_read_tree_path(out_mbufs[i], > &subport, &pipe, &traffic_class, &queue); > > - VERIFY(subport == SUBPORT, "Wrong subport\n"); > - VERIFY(pipe == PIPE, "Wrong pipe\n"); > - VERIFY(traffic_class == TC, "Wrong traffic_class\n"); > - VERIFY(queue == QUEUE, "Wrong queue\n"); > + TEST_ASSERT_EQUAL(subport, SUBPORT, "Wrong subport\n"); > + TEST_ASSERT_EQUAL(pipe, PIPE, "Wrong pipe\n"); > + TEST_ASSERT_EQUAL(traffic_class, TC, "Wrong traffic_class\n"); > + TEST_ASSERT_EQUAL(queue, QUEUE, "Wrong queue\n"); > > } > > @@ -215,12 +209,15 @@ test_sched(void) > struct rte_sched_subport_stats subport_stats; > uint32_t tc_ov; > rte_sched_subport_read_stats(port, SUBPORT, &subport_stats, &tc_ov); > - //VERIFY(subport_stats.n_pkts_tc[TC-1] == 10, "Wrong subport stats\n"); > - > +#if 0 > + TEST_ASSERT_EQUAL(subport_stats.n_pkts_tc[TC-1], 10, "Wrong subport stats\n"); > +#endif > struct rte_sched_queue_stats queue_stats; > uint16_t qlen; > rte_sched_queue_read_stats(port, QUEUE, &queue_stats, &qlen); > - //VERIFY(queue_stats.n_pkts == 10, "Wrong queue stats\n"); > +#if 0 > + TEST_ASSERT_EQUAL(queue_stats.n_pkts, 10, "Wrong queue stats\n"); > +#endif > > rte_sched_port_free(port); > > -- > 2.1.0 > > These TEST_ASSERT macros are no better than the VERIFY macro, they contain exaxtly the same return issue that I outlined in my first post on the subject. Neil