On Sat, 9 May 2026 10:47:53 +0200 Morten Brørup <[email protected]> wrote:
> > From: Stephen Hemminger [mailto:[email protected]] > > 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 <[email protected]> > > 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);
