Re: [dpdk-dev] [PATCH 2/3] mbuf: add a non fatal sanity check helper

[Olivier Matz <olivier.matz@6wind.com>]

On Mon, Sep 10, 2018 at 11:33:44AM +0300, Andrew Rybchenko wrote:
> On 09/10/2018 11:24 AM, David Marchand wrote:
> > On Mon, Sep 10, 2018 at 10:12 AM, Andrew Rybchenko
> > <arybchenko@solarflare.com> wrote:
> > > +/**
> > > + * Sanity checks on a mbuf.
> > > + *
> > > + * Almost like rte_mbuf_sanity_check(), but this function gives the reason
> > > + * if corruption is detected rather than panic.
> > > + *
> > > + * @param m
> > > + *   The mbuf to be checked.
> > > + * @param is_header
> > > + *   True if the mbuf is a packet header, false if it is a sub-segment
> > > + *   of a packet (in this case, some fields like nb_segs are not checked)
> > > + * @param reason
> > > + *   A reference to a string pointer where to store the reason why a mbuf
> > > is
> > > + *   considered invalid.
> > > + * @return
> > > + *   - 0 if no issue has been found, reason is left untouched.
> > > + *   - -1 if a problem is detected, reason then points to a string
> > > describing
> > > + *     the reason why the mbuf is deemed invalid.
> > > + */
> > > +__rte_experimental
> > > +int rte_mbuf_check(const struct rte_mbuf *m, int is_header,
> > > +   const char **reason);
> > > +
> > > 
> > > 
> > > May be it would be better to return reason as return value and if it is NULL
> > > everything is OK?
> > This was what I had done with my first attempt.
> > But given the name "rte_mbuf_check", having a int (used as a bool)
> > returned makes sense to me.
> 
> Yes, good point.
> 
> > So no strong opinion here.
> 
> Me too.

I think an int is fine.

Acked-by: Olivier Matz <olivier.matz@6wind.com>