Re: [PATCH v2 2/4] bitset: fix build for GCC without experimental API

[Thomas Monjalon <thomas@monjalon.net>]

16/10/2024 18:03, Mattias Rönnblom:
> On 2024-10-16 17:42, Morten Brørup wrote:
> >> From: David Marchand [mailto:david.marchand@redhat.com]
> >> Sent: Wednesday, 16 October 2024 17.37
> >>
> >> On Wed, Oct 16, 2024 at 4:14 PM Mattias Rönnblom
> >> <hofors@lysator.liu.se> wrote:
> >>>
> >>> On 2024-10-16 13:38, David Marchand wrote:
> >>>> For a reason similar to the change on bitops header, hide bitset
> >>>> implementation relying on experimental API.
> >>>>
> >>>> Fixes: 99a1197647d8 ("eal: add bitset type")
> >>>>
> >>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
> >>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> >>>> ---
> >>>>    lib/eal/include/rte_bitset.h | 123
> >> +++++++++++++++++++++++++++++++++++
> >>>>    1 file changed, 123 insertions(+)
> >>>>
> >>>> diff --git a/lib/eal/include/rte_bitset.h
> >> b/lib/eal/include/rte_bitset.h
> >>>> index 74c643a72a..8ae8425fc2 100644
> >>>> --- a/lib/eal/include/rte_bitset.h
> >>>> +++ b/lib/eal/include/rte_bitset.h
> >>>> @@ -255,7 +255,13 @@ __rte_experimental
> >>>>    static inline bool
> >>>>    rte_bitset_test(const uint64_t *bitset, size_t bit_num)
> >>>>    {
> >>>> +#ifdef ALLOW_EXPERIMENTAL_API
> >>>>        return __RTE_BITSET_DELEGATE(rte_bit_test, bitset, bit_num);
> >>>> +#else
> >>>> +     RTE_SET_USED(bitset);
> >>>> +     RTE_SET_USED(bit_num);
> >>>> +     return false;
> >>>
> >>> This is no RTE_VERIFY(0) here, because this is just dummy code, that
> >>> will never be run. Is that correct?
> >>
> >> Adding a RTE_VERIFY(false) is an interesting idea.
> >> It is not supposed to be run, indeed.
> > 
> > Great idea.
> > 
> >>
> >> Do you prefer I respin with this?
> > 
> > No need.
> > Instead, create a ticket in Bugzilla so RTE_VERIFY(false) goes in everywhere there is dummy code, not just here.
> > 
> 
> No experimental function is supposed to be invoked, if 
> ALLOW_EXPERIMENTAL_API is not set, right? So RTE_VERIFY(), or its 
> compile-time equivalent, should be in every such function, not just the 
> broken ones.

There is no other experimental dummy branch in the current version.
I think respin is a good option.