[v3,3/5] test/ring: move common function to header file

Message ID 20201023044343.13462-4-honnappa.nagarahalli@arm.com
State Superseded
Headers show
Series
  • lib/ring: add zero copy APIs
Related show

Commit Message

Honnappa Nagarahalli Oct. 23, 2020, 4:43 a.m.
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(-)

-- 
2.17.1

Comments

Ananyev, Konstantin Oct. 23, 2020, 2:22 p.m. | #1
> 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?


> +

>  static __rte_always_inline unsigned int

>  test_ring_enqueue(struct rte_ring *r, void **obj, int esize, unsigned int n,

>  			unsigned int api_type)

> --

> 2.17.1
Honnappa Nagarahalli Oct. 23, 2020, 11:54 p.m. | #2
<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)
                sz = sizeof(void *);

        return (void *)((uint32_t *)obj + (n * sz / sizeof(uint32_t)));
}

> 

> 

> > +

> >  static __rte_always_inline unsigned int  test_ring_enqueue(struct

> > rte_ring *r, void **obj, int esize, unsigned int n,

> >  			unsigned int api_type)

> > --

> > 2.17.1
Stephen Hemminger Oct. 24, 2020, 12:29 a.m. | #3
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
Honnappa Nagarahalli Oct. 24, 2020, 12:31 a.m. | #4
<snip>

> > 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

+1

Patch

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)));
+}
+
 static __rte_always_inline unsigned int
 test_ring_enqueue(struct rte_ring *r, void **obj, int esize, unsigned int n,
 			unsigned int api_type)