* [PATCH v4] test: fix missing NULL pointer checks @ 2015-01-30 11:14 Daniel Mrzyglod [not found] ` <1422616493-3192-1-git-send-email-danielx.t.mrzyglod-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 2+ messages in thread From: Daniel Mrzyglod @ 2015-01-30 11:14 UTC (permalink / raw) To: dev-VfR2kkLFssw 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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> --- 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 *)(ð_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 ^ permalink raw reply related [flat|nested] 2+ messages in thread
[parent not found: <1422616493-3192-1-git-send-email-danielx.t.mrzyglod-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v4] test: fix missing NULL pointer checks [not found] ` <1422616493-3192-1-git-send-email-danielx.t.mrzyglod-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2015-02-10 11:45 ` Bruce Richardson 0 siblings, 0 replies; 2+ messages in thread From: Bruce Richardson @ 2015-02-10 11:45 UTC (permalink / raw) To: Daniel Mrzyglod; +Cc: dev-VfR2kkLFssw On Fri, Jan 30, 2015 at 12:14: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. > > v4 changes: > -If test fails add cleanup routines > -Change Magic Numbers to dedicated Macro RTE_DIM This V4 adds in significant new changes to those described in the patch title. I think it needs to be split into separate patches, so that we can have a single simple patch adding in the additional null tests. The rework of the macros and the switch to have global variables and clean-up routines should be in separate patches. Check spelling of mbuf as "mbuff" a few times below. Also, can you please post follow-up patches within the original thread to make tracking the discussion easier. It allows us to quickly see how this patch has evolved from the original proposal. /Bruce > > 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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> > --- > 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 *)(ð_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 > ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-02-10 11:45 UTC | newest] Thread overview: 2+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-30 11:14 [PATCH v4] test: fix missing NULL pointer checks Daniel Mrzyglod [not found] ` <1422616493-3192-1-git-send-email-danielx.t.mrzyglod-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2015-02-10 11:45 ` Bruce Richardson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).