Re: [PATCH v3 5/9] net/ixgbe/base: fix lock checker errors

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Mar 28, 2025 at 09:20:16AM +0100, David Marchand wrote:
> On Thu, Mar 27, 2025 at 3:53 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > When building on FreeBSD, errors are reported in the base code by the
> > lock checker (-Wthread-safety). For example:
> >
> > ../drivers/net/intel/ixgbe/base/ixgbe_osdep.c:42:1: error: mutex 'lock->mutex' is still held at the end of function [-Werror,-Wthread-safety-analysis]
> >    42 | }
> >       | ^
> >
> > These errors are due to the checker not recognising the lock wrapper
> > functions. We can avoid these errors by converting these functions into
> > macros.
> >
> > Fixes: 30b19d1b5c43 ("net/ixgbe/base: add definitions for E610")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> This is the best solution, given that FreeBSD pthread is instrumented
> with clang thread safety annotations.
> 
> As a sidenote, I don't see much value with the remaining
> malloc/calloc/free wrappers in this osdep.c file.
> I suspect this makes some other annotations non working.
> 
Yes, I would tend to agree. Question is whether it is better to convert
them to macros or just move them to the header file as static inlines. I'd
tend towards the latter, because otherwise we'd need to use "," syntax to
avoid potentially introducing other warnings for unused "hw" variable.

Here are two option examples, WDYT of each?

static inline void *
ixgbe_malloc(struct ixgbe_hw __rte_unused *hw, size_t s) { return malloc(s); }

or

#define ixgbe_malloc(hw, s) ((void)hw, malloc(s))

/Bruce