[dpdk-dev] [PATCH v4] test: fix missing NULL pointer checks

[Daniel Mrzyglod]

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.

v4 changes:
-If test fails add cleanup routines
-Change Magic Numbers to dedicated Macro RTE_DIM

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 at intel.com>
---
 app/test/test_sched.c | 120 ++++++++++++++++++++++++++++++++++----------------
 1 file changed, 83 insertions(+), 37 deletions(-)

diff --git a/app/test/test_sched.c b/app/test/test_sched.c
index c957d80..6a97e2c 100644
--- a/app/test/test_sched.c
+++ b/app/test/test_sched.c
@@ -44,14 +44,7 @@
 #include <rte_ip.h>
 #include <rte_byteorder.h>
 #include <rte_sched.h>
-
-
-#define VERIFY(exp,fmt,args...)                    	                \
-		if (!(exp)) {                                               \
-			printf(fmt, ##args);                                    \
-			return -1;                                              \
-		}
-
+#include <rte_common.h>
 
 #define SUBPORT 	0
 #define PIPE 		1
@@ -92,6 +85,10 @@ static struct rte_sched_port_params port_param = {
 	.n_pipe_profiles = 1,
 };
 
+static struct rte_sched_port *port = NULL;
+static struct rte_mbuf *in_mbufs[10];
+static struct rte_mempool *mp = NULL;
+
 #define NB_MBUF          32
 #define MAX_PACKET_SZ    2048
 #define MBUF_SZ (MAX_PACKET_SZ + sizeof(struct rte_mbuf) + RTE_PKTMBUF_HEADROOM)
@@ -128,7 +125,6 @@ prepare_pkt(struct rte_mbuf *mbuf)
 	struct ether_hdr *eth_hdr;
 	struct vlan_hdr *vlan1, *vlan2;
 	struct ipv4_hdr *ip_hdr;
-
 	/* Simulate a classifier */
 	eth_hdr = rte_pktmbuf_mtod(mbuf, struct ether_hdr *);
 	vlan1 = (struct vlan_hdr *)(&eth_hdr->ether_type );
@@ -150,64 +146,71 @@ prepare_pkt(struct rte_mbuf *mbuf)
 }
 
 
+static void
+clean_environment(void)
+{
+	uint32_t i;
+
+	port = NULL;
+	for (i = 0; i < RTE_DIM(in_mbufs); i++) {
+		if (in_mbufs[i] != NULL) {
+			rte_pktmbuf_free(in_mbufs[i]);
+			in_mbufs[i] = NULL;
+		}
+	}
+}
+
 /**
  * test main entrance for library sched
  */
 static int
 test_sched(void)
 {
-	struct rte_mempool *mp = NULL;
-	struct rte_sched_port *port = NULL;
 	uint32_t pipe;
-	struct rte_mbuf *in_mbufs[10];
+	uint32_t i;
 	struct rte_mbuf *out_mbufs[10];
-	int i;
-
 	int err;
 
-	mp = create_mempool();
-
 	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++) {
+	for (i = 0; i < RTE_DIM(in_mbufs); 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, RTE_DIM(in_mbufs));
+	TEST_ASSERT_EQUAL(err, RTE_DIM(in_mbufs), "Wrong enqueue, err=%d\n", err);
 
-	err = rte_sched_port_enqueue(port, in_mbufs, 10);
-	VERIFY(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);
+	err = rte_sched_port_dequeue(port, out_mbufs, RTE_DIM(out_mbufs));
+	TEST_ASSERT_EQUAL(err, RTE_DIM(out_mbufs), "Wrong dequeue, err=%d\n", err);
 
-	for (i = 0; i < 10; i++) {
+	for (i = 0; i < RTE_DIM(out_mbufs); 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,20 +218,63 @@ 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
+	/* Test cleanning */
 	rte_sched_port_free(port);
+	port = NULL;
+	for (i = 0; i < RTE_DIM(in_mbufs); i++) {
+			rte_pktmbuf_free(in_mbufs[i]);
+			in_mbufs[i] = NULL;
+	}
+
+	return 0;
+}
+
+static int
+check_environment(void)
+{
+	uint32_t i;
+
+	TEST_ASSERT_NULL(port, "Port not cleaned by test\n");
+
+	for (i = 0; i < RTE_DIM(in_mbufs); i++) {
+		TEST_ASSERT_NULL(in_mbufs[i], "in_mbuff not cleaned by test\n");
+	}
 
 	return 0;
 }
 
+
+
+static int
+test_sched_start(void)
+{
+	int ret;
+
+	mp = create_mempool();
+	TEST_ASSERT_NOT_NULL(mp, "Error creating mempool\n");
+
+	if (check_environment() < 0)
+		return -1;
+	ret = test_sched();
+	if (ret == 0)
+		TEST_ASSERT_SUCCESS(check_environment(), "Bad Values after test execution");
+	else
+		clean_environment();
+
+	return ret;
+}
+
 static struct test_command sched_cmd = {
 	.command = "sched_autotest",
-	.callback = test_sched,
+	.callback = test_sched_start,
 };
 REGISTER_TEST_COMMAND(sched_cmd);
-- 
2.1.0