On Wed, 23 Sep 2020 18:37:19 +0000
"Dumitrescu, Cristian" <cristian.dumitre...@intel.com> wrote:
> Hi Stephen,
>
> > -----Original Message-----
> > From: Stephen Hemminger <step...@networkplumber.org>
> > Sent: Wednesday, September 23, 2020 7:25 PM
> > To: Dumitrescu, Cristian <cristian.dumitre...@intel.com>
> > Cc: dev@dpdk.org; tho...@monjalon.net; david.march...@redhat.com
> > Subject: Re: [dpdk-dev] [PATCH v5 01/41] pipeline: add new SWX pipeline
> > type
> >
> > On Wed, 23 Sep 2020 19:06:05 +0100
> > Cristian Dumitrescu <cristian.dumitre...@intel.com> wrote:
> >
> > > +/*
> > > + * Pipeline.
> > > + */
> > > +struct rte_swx_pipeline {
> > > + int build_done;
> > > + int numa_node;
> > > +};
> > > +
> >
> > Nit, could build_done be a bool type?
> >
>
> As we discussed this in an earlier version of this patch set:
> Isn't the difference between int and bool mostly cosmetic?
> AFAIK we don't have a hard rule in DPDK about bool vs. int.
> IMO doing this change now it likely not going to add any value.
There is no policy and there probably doesn't need to be.
Original code was written using BSD style, and BSD
predates introduction of <stdbool.h>. Linux developers have been
favoring bool.
Purely a human thing, compilers just treat bool == int
and allow assigning anything. Coverity or sparse might check though.
> > +void
> > +rte_swx_pipeline_free(struct rte_swx_pipeline *p)
> > +{
> > + if (!p)
> > + return;
> > +
> > + free(p);
> > +}
> >
> > The free() function in libc is defined to accept NULL as ok.
> > Please remove the if()
>
> This is just the early function wrapper in patch 1 out of 41, mode code is
> added in this function by later patches that need the if statement. IMO this
> change will not add any value at all here.
Sure, makes sense.
