* [dpdk-dev] [PATCH v3] test: fix missing NULL pointer checks
@ 2015-01-27 15:44 Daniel Mrzyglod
2015-01-27 18:06 ` Neil Horman
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Mrzyglod @ 2015-01-27 15:44 UTC (permalink / raw)
To: dev
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [dpdk-dev] [PATCH v3] test: fix missing NULL pointer checks
2015-01-27 15:44 [dpdk-dev] [PATCH v3] test: fix missing NULL pointer checks Daniel Mrzyglod
@ 2015-01-27 18:06 ` Neil Horman
2015-01-30 10:18 ` Thomas Monjalon
0 siblings, 1 reply; 5+ messages in thread
From: Neil Horman @ 2015-01-27 18:06 UTC (permalink / raw)
To: Daniel Mrzyglod; +Cc: dev
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [dpdk-dev] [PATCH v3] test: fix missing NULL pointer checks
2015-01-27 18:06 ` Neil Horman
@ 2015-01-30 10:18 ` Thomas Monjalon
2015-02-10 11:46 ` Bruce Richardson
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Monjalon @ 2015-01-30 10:18 UTC (permalink / raw)
To: Neil Horman; +Cc: dev
2015-01-27 13:06, Neil Horman:
> 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>
>
> 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, you are suggesting to rework the assert macros of the unit tests.
It should be another patch.
Here, Daniel is improving the sched test with existing macros.
I think it should be applied.
--
Thomas
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH v3] test: fix missing NULL pointer checks
2015-01-30 10:18 ` Thomas Monjalon
@ 2015-02-10 11:46 ` Bruce Richardson
2015-02-24 20:54 ` Thomas Monjalon
0 siblings, 1 reply; 5+ messages in thread
From: Bruce Richardson @ 2015-02-10 11:46 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
On Fri, Jan 30, 2015 at 11:18:19AM +0100, Thomas Monjalon wrote:
> 2015-01-27 13:06, Neil Horman:
> > 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>
> >
> > 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, you are suggesting to rework the assert macros of the unit tests.
> It should be another patch.
> Here, Daniel is improving the sched test with existing macros.
> I think it should be applied.
>
+1
I agree with Thomas here. Having looked at the V4 patch, I believe this V3 is
better, and that other cleanup should be done in separate patches.
/Bruce
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH v3] test: fix missing NULL pointer checks
2015-02-10 11:46 ` Bruce Richardson
@ 2015-02-24 20:54 ` Thomas Monjalon
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Monjalon @ 2015-02-24 20:54 UTC (permalink / raw)
To: Daniel Mrzyglod; +Cc: dev
2015-02-10 11:46, Bruce Richardson:
> On Fri, Jan 30, 2015 at 11:18:19AM +0100, Thomas Monjalon wrote:
> > 2015-01-27 13:06, Neil Horman:
> > > 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>
> > >
> > > 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, you are suggesting to rework the assert macros of the unit tests.
> > It should be another patch.
> > Here, Daniel is improving the sched test with existing macros.
> > I think it should be applied.
> >
>
> +1
> I agree with Thomas here. Having looked at the V4 patch, I believe this V3 is
> better, and that other cleanup should be done in separate patches.
Applied, thanks
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-02-24 20:55 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-27 15:44 [dpdk-dev] [PATCH v3] test: fix missing NULL pointer checks Daniel Mrzyglod
2015-01-27 18:06 ` Neil Horman
2015-01-30 10:18 ` Thomas Monjalon
2015-02-10 11:46 ` Bruce Richardson
2015-02-24 20:54 ` Thomas Monjalon
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).