> [...] > > +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.