Re: [dpdk-dev] [PATCH v4 2/4] mempool: add non-IO flag

[Dmitry Kozlyuk <dkozlyuk@oss.nvidia.com>]

> [...]
> > +static int
> > +test_mempool_flag_non_io_set_when_no_iova_contig_set(void)
> > +{
> > +     struct rte_mempool *mp;
> > +     int ret;
> > +
> > +     mp = rte_mempool_create_empty("empty", MEMPOOL_SIZE,
> > +                                   MEMPOOL_ELT_SIZE, 0, 0,
> > +                                   SOCKET_ID_ANY,
> MEMPOOL_F_NO_IOVA_CONTIG);
> > +     RTE_TEST_ASSERT_NOT_NULL(mp, "Cannot create mempool: %s",
> > +                              rte_strerror(rte_errno));
> > +     rte_mempool_set_ops_byname(mp, rte_mbuf_best_mempool_ops(), NULL);
> > +     ret = rte_mempool_populate_default(mp);
> > +     RTE_TEST_ASSERT(ret > 0, "Failed to populate mempool: %s",
> > +                     rte_strerror(rte_errno));
> > +     RTE_TEST_ASSERT(mp->flags & MEMPOOL_F_NON_IO,
> > +                     "NON_IO flag is not set when NO_IOVA_CONTIG is
> set");
> > +     rte_mempool_free(mp);
> > +     return 0;
> > +}
> 
> One comment that also applies to the previous patch. Using
> RTE_TEST_ASSERT_*() is convenient to test a condition, display an error
> message and return on error in one operation. But here it can cause a
> leak on test failure.
> 
> I don't know what is the best approach to solve the issue. Having
> equivalent test macros that do "goto fail" instead of "return -1" would
> help here. I mean something like:
>   RTE_TEST_ASSERT_GOTO_*(cond, label, fmt, ...)
> 
> What do you think?

This can work with existing macros:

	#define TEST_TRACE_FAILURE(...) goto fail

Because of "trace" in the name it looks a bit like a hijacking.
Probably the macro should be named TEST_HANDLE_FAILURE
to suggest broader usages than just tracing,
but for now it looks the most neat way.