Re: [dpdk-dev] [PATCH v2 10/10] enforce __rte_experimental at the start of symbol declarations

[David Marchand <david.marchand@redhat.com>]

On Mon, Jul 1, 2019 at 2:05 PM Aaron Conole <aconole@redhat.com> wrote:

> David Marchand <david.marchand@redhat.com> writes:
>
> > On Sat, Jun 29, 2019 at 6:14 PM Thomas Monjalon <thomas@monjalon.net>
> wrote:
> >
> >  29/06/2019 13:58, David Marchand:
> >  > Special mention for rte_mbuf_data_addr_default():
> >  >
> >  > There is either a bug or a (not yet understood) issue with gcc.
> >  > gcc won't drop this inline when unused and
> rte_mbuf_data_addr_default()
> >  > calls rte_mbuf_buf_addr() which itself is experimental.
> >  > This results in a build warning when not accepting experimental apis
> >  > from sources just including rte_mbuf.h.
> >  >
> >  > For this specific case, we hide the call to rte_mbuf_buf_addr() under
> >  > the ALLOW_EXPERIMENTAL_API flag.
> >  [...]
> >  > -static inline char * __rte_experimental
> >  > -rte_mbuf_data_addr_default(struct rte_mbuf *mb)
> >  > +__rte_experimental
> >  > +static inline char *
> >  > +rte_mbuf_data_addr_default(struct rte_mbuf *mb __rte_unused)
> >  >  {
> >  > +     /* gcc complains about calling this experimental function even
> >  > +      * when not using it. Hide it with ALLOW_EXPERIMENTAL_API.
> >  > +      */
> >  > +#ifdef ALLOW_EXPERIMENTAL_API
> >  >       return rte_mbuf_buf_addr(mb, mb->pool) + RTE_PKTMBUF_HEADROOM;
> >  > +#else
> >  > +     return NULL;
> >  > +#endif
> >  >  }
> >
> >  Doxygen is confused by having __rte_unused at the end:
> >
> >  lib/librte_mbuf/rte_mbuf.h:876: warning:
> >          argument 'mb' of command @param is not found in the argument
> list of
> >          rte_mbuf_data_addr_default(struct rte_mbuf *mb __rte_unused)
> >  lib/librte_mbuf/rte_mbuf.h:889: warning:
> >          The following parameters of
> >          rte_mbuf_data_addr_default(struct rte_mbuf *mb __rte_unused)
> >          are not documented:
> >                  parameter '__rte_unused'
> >
> >  I move __rte_unused at the beginning while merging.
> >
> > Indeed.
> > Thanks for catching and fixing.
> >
> > Consequently, could we have the documentation (html only?) generated in
> the CI if it is not too time
> > consuming.
> > WDYT Aaron, Michael?
>
> Just as a check that it will generate?  I think it's a good idea.
>


Just knowing it generates fine is enough.
If it fails, then we go and see what is wrong.
Saving each builds would be a loss of space because (hopefully) it usually
builds fine.


-- 
David Marchand