[DTrace-devel] [PATCH] Simplification of the varint test

[Kris Van Hees]

Thank you for the suggestion.  It makes sense - I wasn't going for the neatest
code especially since I wrote this little program during implementation of the
varint code - as a way to verify that the implementation was accurate), but I
do appreciate you spending time cleaning it up.

I am going to add one extra case to the check() function that was missing from
my version: dt_vint_skip().  It was missing because I implemented that function
later on when I found it would be convenient to have when implementing various
string operations.  It will simply verify:

	p = dt_vint_skip(s);
	if (!p || (p - s) != len) {
		printf("Skip wrong for %lu: %d vs %d\n",
		       val, p ? p - s : 0, exp);
		exit(1);
	}


On Sat, Jun 12, 2021 at 01:32:49AM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> Intended to be squashed back into
> "Implement variable length integer support".
> ---
>  test/internals/tst.varint.c | 122 ++++++++++++------------------------
>  1 file changed, 41 insertions(+), 81 deletions(-)
> 
> diff --git a/test/internals/tst.varint.c b/test/internals/tst.varint.c
> index 7011b488..bec6947f 100644
> --- a/test/internals/tst.varint.c
> +++ b/test/internals/tst.varint.c
> @@ -6,29 +6,26 @@
>  #include <dt_varint.h>
>  
>  void
> -check_length(uint64_t val, int len, int exp)
> +check(uint64_t val, int exp)
>  {
> -	if (len != exp) {
> -		printf("Length wrong for %lu: %d vs %d\n", val, len, exp);
> +	char s[VARINT_MAX_BYTES];
> +	int rc, len;
> +	uint64_t dval;
> +
> +	rc = dt_int2vint(val, s);
> +	if (rc != exp) {
> +		printf("Length wrong for %lu: %d vs %d\n", val, rc, exp);
>  		exit(1);
>  	}
> -}
> -
> -void
> -check_size(uint64_t val, int len, int exp)
> -{
> +	len = dt_vint_size(val);
>  	if (len != exp) {
>  		printf("Size wrong for %lu: %d vs %d\n", val, len, exp);
>  		exit(1);
>  	}
> -}
> -
> -void
> -check_value(uint64_t dval, uint64_t eval, int len)
> -{
> -	if (dval != eval) {
> +	dval = dt_vint2int(s);
> +	if (dval != val) {
>  		printf("Value decode error (%d byte prefix): %lx vs %lx\n",
> -		       len, dval, eval);
> +		       rc, dval, val);
>  		exit(1);
>  	}
>  }
> @@ -37,59 +34,37 @@ void
>  check_range(uint64_t lo, uint64_t hi, int len)
>  {
>  	uint64_t	val;
> -	char		s[VARINT_MAX_BYTES];
> -	int		rc;
> -
> -	for (val = lo - 10000; val < lo + 10000; val++) {
> -		rc = dt_int2vint(val, s);
> -		if (val < lo) {
> -			check_length(val, rc, len - 1);
> -			check_size(val, dt_vint_size(val), len - 1);
> -		} else {
> -			check_length(val, rc, len);
> -			check_size(val, dt_vint_size(val), len);
> -		}
>  
> -		check_value(dt_vint2int(s), val, rc);
> -	}
> -
> -	for (val = hi - 10000; val < hi + 10000; val++) {
> -		rc = dt_int2vint(val, s);
> -		if (val <= hi) {
> -			check_length(val, rc, len);
> -			check_size(val, dt_vint_size(val), len);
> -		} else {
> -			check_length(val, rc, len + 1);
> -			check_size(val, dt_vint_size(val), len + 1);
> -		}
> -
> -		check_value(dt_vint2int(s), val, rc);
> -	}
> +	/* taste test!  Here are two styles to choose from: */
> +#if 0
> +	for (val = lo - 10000; val < lo; val++)
> +		check(val, len-1);
> +	for (val = lo; val < lo + 10000; val++)
> +		check(val, len);
> +	for (val = hi - 10000; val <= hi; val++)
> +		check(val, len);
> +	for (val = hi + 1; val < hi + 10000; val++)
> +		check(val, len + 1);
> +#else
> +	for (val = lo - 10000; val < lo + 10000; val++)
> +		check(val, val < lo ? len-1 : len);
> +	for (val = hi - 10000; val < hi + 10000; val++)
> +		check(val, val <= hi ? len : len + 1);
> +#endif
>  }
>  
>  int
>  main(void)
>  {
>  	uint64_t	val;
> -	char		s[VARINT_MAX_BYTES];
> -	int		rc;
>  
>  	/* First range: we go through all 16-bit values. */
> -	for (val = 0; val < 0xffff; val++) {
> -		rc = dt_int2vint(val, s);
> -		if (val <= VARINT_1_MAX) {
> -			check_length(val, rc, 1);
> -			check_size(val, dt_vint_size(val), 1);
> -		} else if (val <= VARINT_2_MAX) {
> -			check_length(val, rc, 2);
> -			check_size(val, dt_vint_size(val), 2);
> -		} else {
> -			check_length(val, rc, 3);
> -			check_size(val, dt_vint_size(val), 3);
> -		}
> -
> -		check_value(dt_vint2int(s), val, rc);
> -	}
> +	for (val = 0; val <= VARINT_1_MAX; val++)
> +		check(val, 1);
> +	for (val = VARINT_1_MAX + 1; val <= VARINT_2_MAX; val++)
> +		check(val, 2);
> +	for (val = VARINT_2_MAX + 1; val < 0xffff; val++)
> +		check(val, 3);
>  
>  	/* For higher ranges, verify the low and high boundary ranges. */
>  	check_range(VARINT_3_MIN, VARINT_3_MAX, 3);
> @@ -100,31 +75,16 @@ main(void)
>  	check_range(VARINT_8_MIN, VARINT_8_MAX, 8);
>  
>  	/* Verify the final range. */
> -	for (val = VARINT_9_MIN - 10000; val < VARINT_9_MIN + 10000; val++) {
> -		rc = dt_int2vint(val, s);
> -		if (val < VARINT_9_MIN) {
> -			check_length(val, rc, 8);
> -			check_size(val, dt_vint_size(val), 8);
> -		} else {
> -			check_length(val, rc, 9);
> -			check_size(val, dt_vint_size(val), 9);
> -		}
> -
> -		check_value(dt_vint2int(s), val, rc);
> -	}
> -
> +	for (val = VARINT_9_MIN - 10000; val < VARINT_9_MIN; val++)
> +		check(val, 8);
> +	for (val = VARINT_9_MIN; val < VARINT_9_MIN + 10000; val++)
> +		check(val, 9);
>  	for (val = VARINT_9_MAX - 10000; val < VARINT_9_MAX; val++) {
> -		rc = dt_int2vint(val, s);
> -		check_length(val, rc, 9);
> -		check_size(val, dt_vint_size(val), 9);
> -
> -		check_value(dt_vint2int(s), val, rc);
> +		check(val, 9);
>  	}
> +	check(VARINT_9_MAX, 9);
>  
> -	rc = dt_int2vint(VARINT_9_MAX, s);
> -	check_length(VARINT_9_MAX, rc, 9);
> -	check_size(VARINT_9_MAX, dt_vint_size(VARINT_9_MAX), 9);
> -	check_value(dt_vint2int(s), VARINT_9_MAX, rc);
> +	return 0;
>  }
>  
>  #include "dt_varint.c"
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel