Re: [PATCH 13/20] mbuf: allow NULL array in rte_pktmbuf_free_bulk

[Stephen Hemminger <stephen@networkplumber.org>]

On Sat, 9 May 2026 10:47:53 +0200
Morten Brørup <mb@smartsharesystems.com> wrote:

> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Friday, 8 May 2026 22.34
> > 
> > This allows callers to avoid NULL checks and just call
> > rte_pktmbuf_free_bulk, similar to rte_pktmbuf_free.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>  
> 
> I disagree with this patch.
> 
> The parameter is an array of (pointers to) mbufs.
> We already accept that the array can contain NULL pointers (no mbuf present).
> This is extremely forgiving, considering that other fast path functions don't allow NULL pointers in arrays;
> e.g. rte_eth_tx_burst(), rte_mempool_put_bulk().
> But since it's a "free()" class of function, I don't object to it.
> 
> However, this patch changes the parameter type from "array" to "array or NULL (no array present)".
> And I don't think we should change the parameter type; it should remain "array" only.
> 
> If there are any scenarios where a non-present array (NULL) is passed to the function, the count should be zero too.
> And when the count is zero, the function does not dereference the array, so explicitly checking for NULL is superfluous.
> 
> We have a convention of not checking parameter validity in fast path functions.
> And I consider it invalid parameters passing NULL with a non-zero count.
> 
> You might argue that this is a "free()" class of function, which warrants checking for NULL; but since it already accepts NULL with zero count, it is already covered.
> 
> We could change the function declaration for clarity:
> 
> void rte_pktmbuf_free_bulk(
> 		unsigned int count;
> 		struct rte_mbuf *mbufs[count], unsigned int count);
> 
> Or add a debug assertion at the start of the function:
> RTE_ASSERT(mbufs != NULL || count == 0);

Ok, it was more motivated by common pattern in driver cleanup paths like:

--- a/app/test-compress-perf/comp_perf_test_common.c
+++ b/app/test-compress-perf/comp_perf_test_common.c
@@ -83,11 +83,9 @@ comp_perf_free_memory(struct comp_test_data *test_data,
 {
 	uint32_t i;
 
-	if (mem->decomp_bufs != NULL)
-		rte_pktmbuf_free_bulk(mem->decomp_bufs, mem->total_bufs);
+	rte_pktmbuf_free_bulk(mem->decomp_bufs, mem->total_bufs);
 
-	if (mem->comp_bufs != NULL)
-		rte_pktmbuf_free_bulk(mem->comp_bufs, mem->total_bufs);
+	rte_pktmbuf_free_bulk(mem->comp_bufs, mem->total_bufs);