diff mbox series

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

Message ID 20201023044343.13462-4-honnappa.nagarahalli@arm.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series lib/ring: add zero copy APIs |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

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

Comments

Ananyev, Konstantin Oct. 23, 2020, 2:22 p.m. UTC | #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. UTC | #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. UTC | #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. UTC | #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
diff mbox series

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)