[dpdk-dev] [PATCH] mbuf: remove inconsistent assert statements

[Ananyev, Konstantin]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Thursday, June 09, 2016 4:28 PM
> To: Ananyev, Konstantin
> Cc: dev at dpdk.org; Olivier Matz; Adrien Mazarguil; Zhang, Helin
> Subject: Re: [dpdk-dev] [PATCH] mbuf: remove inconsistent assert statements
> 
> 2016-06-09 13:21, Ananyev, Konstantin:
> > From: Olivier Matz [mailto:olivier.matz at 6wind.com]
> > > Today:
> > >
> > >   /* allowed */
> > >   m = rte_pktmbuf_alloc();
> > >   rte_pktmbuf_free(m);
> > >
> > >   /* not allowed */
> > >   m = rte_mbuf_raw_alloc();
> > >   __rte_mbuf_raw_free(m);
> > >
> > >   /* we should do instead (strange): */
> > >   m = rte_mbuf_raw_alloc();
> > >   rte_pktmbuf_free(m);
> > >
> > > What I suggest to have:
> > >
> > >   /* allowed, no change */
> > >   m = rte_pktmbuf_alloc();
> > >   rte_pktmbuf_free(m);
> > >
> > >   /* allowed, these functions would be symetrical */
> > >   m = rte_mbuf_raw_alloc();
> > >   rte_mbuf_raw_free(m);
> > >
> > >   /* not allowed, m->refcnt is uninitialized */
> > >   m = rte_mbuf_raw_alloc();
> > >   rte_pktmbuf_free(m);
> >
> > Hmm, and what it will buy us (except of symmetry)?
> 
> API consistency is important.
> It is a matter of making our users confident in DPDK.
> 
> I would not like to use a library where I need to check in the doc
> for each function because I don't remember and I'm not confident
> that the function fish() do some fishing or hunting.

As you remember, the whole story started when people used
'internal mbuf function' inside their code and then
figured out that it doesn't work as they expect :)

But as I said, if everyone are that desperate about symmetry, 
we can just create a new public function:

void
rte_mbuf_raw_free(stuct rte_mbuf *m)
{
      if (rte_mbuf_refcnt_update(m, -1) == 0)
                __rte_mbuf_raw_free(m);
}

That would be 'symmetric' to rte_mbuf_raw_alloc(),
and all three combinations above would be allowed.
Konstantin