From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <nhorman@tuxdriver.com>
Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58])
by dpdk.org (Postfix) with ESMTP id D76F05A3F
for <dev@dpdk.org>; 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 <nhorman@tuxdriver.com>)
id 1YGAXB-0004j5-Ct; Tue, 27 Jan 2015 13:06:47 -0500
Date: Tue, 27 Jan 2015 13:06:40 -0500
From: Neil Horman <nhorman@tuxdriver.com>
To: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
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 <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>
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 <danielx.t.mrzyglod@intel.com>
> ---
> 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 <rte_sched.h>
>
>
> -#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