Re: [PATCH v1 6/7] test_hexdump: test all possible group sizes for overflow

[Rasmus Villemoes <linux@rasmusvillemoes.dk>]

On Wed, Nov 11 2015, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Currently only one combination is tested for overflow, i.e. rowsize = 16,
> groupsize = 1, len = 1.  Do various test to go all possible branches.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  lib/test_hexdump.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
> index 9b95b67..fa2f4af 100644
> --- a/lib/test_hexdump.c
> +++ b/lib/test_hexdump.c
> @@ -126,12 +126,13 @@ static void __init test_hexdump_set(int rowsize, bool ascii)
>  	test_hexdump(len, rowsize, 1, ascii);
>  }
>  
> -static void __init test_hexdump_overflow(size_t buflen, bool ascii)
> +static void __init test_hexdump_overflow(size_t buflen, size_t len,
> +					 int rowsize, int groupsize,
> +					 bool ascii)
>  {
>  	char test[TEST_HEXDUMP_BUF_SIZE];
>  	char buf[TEST_HEXDUMP_BUF_SIZE];
> -	size_t len = 1;
> -	int rs = 16, gs = 1;
> +	int rs = rowsize, gs = groupsize;
>  	int ae, he, e, r;
>  	bool a;
>  
> @@ -170,6 +171,18 @@ static void __init test_hexdump_overflow(size_t buflen, bool ascii)
>  	}
>  }
>  
> +static void __init test_hexdump_overflow_set(size_t buflen, bool ascii)
> +{
> +	unsigned int i = 0;
> +
> +	do {
> +		int gs = 1 << i;
> +		size_t len = get_random_int() % 16 + gs;
> +
> +		test_hexdump_overflow(buflen, rounddown(len, gs), 16, gs, ascii);
> +	} while (i++ < 3);
> +}


aren't you missing a

  test_hexdump_overflow(buflen, rounddown(len, gs), 32, gs, ascii);

here to also exercise the rowsize==32 code?


> +
>  static int __init test_hexdump_init(void)
>  {
>  	unsigned int i;
> @@ -186,10 +199,10 @@ static int __init test_hexdump_init(void)
>  		test_hexdump_set(rowsize, true);
>  
>  	for (i = 0; i <= TEST_HEXDUMP_BUF_SIZE; i++)
> -		test_hexdump_overflow(i, false);
> +		test_hexdump_overflow_set(i, false);
>  
>  	for (i = 0; i <= TEST_HEXDUMP_BUF_SIZE; i++)
> -		test_hexdump_overflow(i, true);
> +		test_hexdump_overflow_set(i, true);

It seems neater to do one loop:

for (i = 0; i <= TEST_HEXDUMP_BUF_SIZE; i++) {
  test_hexdump_overflow_set(i, false);
  test_hexdump_overflow_set(i, true);
}

Rasmus