[v3,5/6] test/ring: fix wrong size used in memcmp

  When using memcmp function to check data, the third param should be the
size of all elements, rather than the number of the elements.

Furthermore, do code clean up by moving repeated code inside
'test_ring_mem_cmp' function to validate data and print information of
enqueue/dequeue elements if validation fails.

Fixes: a9fe152363e2 ("test/ring: add custom element size functional tests")
Cc: honnappa.nagarahalli@arm.com
Cc: stable@dpdk.org

Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
---
 app/test/test_ring.c | 59 ++++++++++++++++++++------------------------
 1 file changed, 27 insertions(+), 32 deletions(-)
  

<snip>

> 
> When using memcmp function to check data, the third param should be the
> size of all elements, rather than the number of the elements.
> 
> Furthermore, do code clean up by moving repeated code inside
> 'test_ring_mem_cmp' function to validate data and print information of
> enqueue/dequeue elements if validation fails.
This patch is fixing 2 things. But only the memcmp issue need to be backported to stable. I would prefer to split this into 2 and mark only the memcmp commit to be backported.

Hi David,
	Do you have a different opinion here?

> 
> Fixes: a9fe152363e2 ("test/ring: add custom element size functional tests")
> Cc: honnappa.nagarahalli@arm.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> Reviewed-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> ---
>  app/test/test_ring.c | 59 ++++++++++++++++++++------------------------
>  1 file changed, 27 insertions(+), 32 deletions(-)
> 
> diff --git a/app/test/test_ring.c b/app/test/test_ring.c index
> 811adc523..4a2bd39fc 100644
> --- a/app/test/test_ring.c
> +++ b/app/test/test_ring.c
> @@ -258,6 +258,21 @@ test_ring_mem_init(void *obj, unsigned int count, int
> esize)
>  			((uint32_t *)obj)[i] = i;
>  }
> 
> +static int
> +test_ring_mem_cmp(void *src, void *dst, unsigned int size) {
> +	int ret;
> +
> +	ret = memcmp(src, dst, size);
> +	if (ret) {
> +		rte_hexdump(stdout, "src", src, size);
> +		rte_hexdump(stdout, "dst", dst, size);
> +		printf("data after dequeue is not the same\n");
> +	}
> +
> +	return ret;
> +}
> +
>  static void
>  test_ring_print_test_string(const char *istr, unsigned int api_type, int esize)
> { @@ -383,7 +398,7 @@ test_ring_burst_bulk_tests1(unsigned int test_idx)
>  	struct rte_ring *r;
>  	void **src = NULL, **cur_src = NULL, **dst = NULL, **cur_dst = NULL;
>  	int ret;
> -	unsigned int i, j;
> +	unsigned int i, j, temp_sz;
>  	int rand;
>  	const unsigned int rsz = RING_SIZE - 1;
> 
> @@ -444,7 +459,11 @@ test_ring_burst_bulk_tests1(unsigned int test_idx)
>  			TEST_RING_VERIFY(rte_ring_empty(r));
> 
>  			/* check data */
> -			TEST_RING_VERIFY(memcmp(src, dst, rsz) == 0);
> +			temp_sz = rsz * sizeof(void *);
> +			if (esize[i] != -1)
> +				temp_sz = rsz * esize[i];
> +			TEST_RING_VERIFY(test_ring_mem_cmp(src, dst,
> +						temp_sz) == 0);
>  		}
> 
>  		/* Free memory before test completed */ @@ -538,12 +557,8
> @@ test_ring_burst_bulk_tests2(unsigned int test_idx)
>  		cur_dst = test_ring_inc_ptr(cur_dst, esize[i], MAX_BULK);
> 
>  		/* check data */
> -		if (memcmp(src, dst, cur_dst - dst)) {
> -			rte_hexdump(stdout, "src", src, cur_src - src);
> -			rte_hexdump(stdout, "dst", dst, cur_dst - dst);
> -			printf("data after dequeue is not the same\n");
> +		if (test_ring_mem_cmp(src, dst, RTE_PTR_DIFF(cur_dst, dst)))
>  			goto fail;
> -		}
> 
>  		/* Free memory before test completed */
>  		rte_ring_free(r);
> @@ -614,12 +629,8 @@ test_ring_burst_bulk_tests3(unsigned int test_idx)
>  		}
> 
>  		/* check data */
> -		if (memcmp(src, dst, cur_dst - dst)) {
> -			rte_hexdump(stdout, "src", src, cur_src - src);
> -			rte_hexdump(stdout, "dst", dst, cur_dst - dst);
> -			printf("data after dequeue is not the same\n");
> +		if (test_ring_mem_cmp(src, dst, RTE_PTR_DIFF(cur_dst, dst)))
>  			goto fail;
> -		}
> 
>  		/* Free memory before test completed */
>  		rte_ring_free(r);
> @@ -747,12 +758,8 @@ test_ring_burst_bulk_tests4(unsigned int test_idx)
>  			goto fail;
> 
>  		/* check data */
> -		if (memcmp(src, dst, cur_dst - dst)) {
> -			rte_hexdump(stdout, "src", src, cur_src - src);
> -			rte_hexdump(stdout, "dst", dst, cur_dst - dst);
> -			printf("data after dequeue is not the same\n");
> +		if (test_ring_mem_cmp(src, dst, RTE_PTR_DIFF(cur_dst, dst)))
>  			goto fail;
> -		}
> 
>  		/* Free memory before test completed */
>  		rte_ring_free(r);
> @@ -857,12 +864,8 @@ test_ring_basic_ex(void)
>  		}
> 
>  		/* check data */
> -		if (memcmp(src, dst, cur_src - src)) {
> -			rte_hexdump(stdout, "src", src, cur_src - src);
> -			rte_hexdump(stdout, "dst", dst, cur_dst - dst);
> -			printf("data after dequeue is not the same\n");
> +		if (test_ring_mem_cmp(src, dst, RTE_PTR_DIFF(cur_src, src)))
>  			goto fail_test;
> -		}
> 
>  		/* Following tests use the configured flags to decide
>  		 * SP/SC or MP/MC.
> @@ -909,12 +912,8 @@ test_ring_basic_ex(void)
>  		cur_dst = test_ring_inc_ptr(cur_dst, esize[i], 2);
> 
>  		/* check data */
> -		if (memcmp(src, dst, cur_dst - dst)) {
> -			rte_hexdump(stdout, "src", src, cur_src - src);
> -			rte_hexdump(stdout, "dst", dst, cur_dst - dst);
> -			printf("data after dequeue is not the same\n");
> +		if (test_ring_mem_cmp(src, dst, RTE_PTR_DIFF(cur_dst, dst)))
>  			goto fail_test;
> -		}
> 
>  		rte_ring_free(rp);
>  		rte_free(src);
> @@ -1047,12 +1046,8 @@ test_ring_with_exact_size(void)
>  		}
> 
>  		/* check data */
> -		if (memcmp(src, dst, cur_dst - dst)) {
> -			rte_hexdump(stdout, "src", src, cur_src - src);
> -			rte_hexdump(stdout, "dst", dst, cur_dst - dst);
> -			printf("data after dequeue is not the same\n");
> +		if (test_ring_mem_cmp(src, dst, RTE_PTR_DIFF(cur_dst, dst)))
>  			goto test_fail;
> -		}
> 
>  		rte_free(src_orig);
>  		rte_free(dst_orig);
> --
> 2.17.1
  

On Mon, Sep 14, 2020 at 6:38 AM Honnappa Nagarahalli
<Honnappa.Nagarahalli@arm.com> wrote:
>
> <snip>
>
> >
> > When using memcmp function to check data, the third param should be the
> > size of all elements, rather than the number of the elements.
> >
> > Furthermore, do code clean up by moving repeated code inside
> > 'test_ring_mem_cmp' function to validate data and print information of
> > enqueue/dequeue elements if validation fails.
> This patch is fixing 2 things. But only the memcmp issue need to be backported to stable. I would prefer to split this into 2 and mark only the memcmp commit to be backported.

+1