Re: [dpdk-dev] [PATCH v3 3/5] test/ring: move common function to header file

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 23 Oct 2020 23:54:22 +0000
Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:

> <snip>
> 
> >   
> > > Move test_ring_inc_ptr to header file so that it can be used by
> > > functions in other files.
> > >
> > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > Reviewed-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> > > ---
> > >  app/test/test_ring.c | 11 -----------  app/test/test_ring.h | 11
> > > +++++++++++
> > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/app/test/test_ring.c b/app/test/test_ring.c index
> > > a62cb263b..329d538a9 100644
> > > --- a/app/test/test_ring.c
> > > +++ b/app/test/test_ring.c
> > > @@ -243,17 +243,6 @@ test_ring_deq_impl(struct rte_ring *r, void **obj,  
> > int esize, unsigned int n,  
> > >  			NULL);
> > >  }
> > >
> > > -static void**
> > > -test_ring_inc_ptr(void **obj, int esize, unsigned int n) -{
> > > -	/* Legacy queue APIs? */
> > > -	if ((esize) == -1)
> > > -		return ((void **)obj) + n;
> > > -	else
> > > -		return (void **)(((uint32_t *)obj) +
> > > -					(n * esize / sizeof(uint32_t)));
> > > -}
> > > -
> > >  static void
> > >  test_ring_mem_init(void *obj, unsigned int count, int esize)  { diff
> > > --git a/app/test/test_ring.h b/app/test/test_ring.h index
> > > d4b15af7c..16697ee02 100644
> > > --- a/app/test/test_ring.h
> > > +++ b/app/test/test_ring.h
> > > @@ -42,6 +42,17 @@ test_ring_create(const char *name, int esize,  
> > unsigned int count,  
> > >  						(socket_id), (flags));
> > >  }
> > >
> > > +static inline void**
> > > +test_ring_inc_ptr(void **obj, int esize, unsigned int n) {
> > > +	/* Legacy queue APIs? */
> > > +	if ((esize) == -1)
> > > +		return ((void **)obj) + n;
> > > +	else
> > > +		return (void **)(((uint32_t *)obj) +
> > > +					(n * esize / sizeof(uint32_t))); }  
> > 
> > In all these pointer arithemetics, why do you need 'void **'?
> > Why just not 'void*', or even uintptr_t?  
> I will change it as follows:
> 
> static inline void*
> test_ring_inc_ptr(void *obj, int esize, unsigned int n)
> {
>         int sz;
> 
>         sz = esize;
>         /* Legacy queue APIs? */
>         if ((esize) == -1)

Extra (paren) doesn't help readability either