From: Theo Buehler <tb@theobuehler.org>
Subject: Re: [PATCH] libressl: Make lhash load test verify exact number of deleted entries
To: Kenjiro Nakayama <nakayamakenjiro@gmail.com>
Cc: tech@openbsd.org
Date: Sat, 3 May 2025 10:48:18 +0200

On Mon, Apr 21, 2025 at 03:41:06PM +0900, Kenjiro Nakayama wrote:
> Track successful deletions using a context struct, and compare the
> remaining item count against the expected value. Replace magic numbers
> with named constants for clarity.

While I'm not against doing this in addition to the current check, this
diff also changes the semantics of the test itself. Previously it checked
that 0 is not chosen substantially differently than all numbers != 0.
This check is lost with the precise count (which basically only verifies
that the test_doall_delete_some() callback is actually called).

> ---
>  src/regress/lib/libcrypto/lhash/lhash_test.c | 39 +++++++++++++++-----
>  1 file changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/src/regress/lib/libcrypto/lhash/lhash_test.c b/src/regress/lib/libcrypto/lhash/lhash_test.c
> index d6dcb33..f71045c 100644
> --- a/src/regress/lib/libcrypto/lhash/lhash_test.c
> +++ b/src/regress/lib/libcrypto/lhash/lhash_test.c
> @@ -27,6 +27,15 @@
>   *  - custom comparison function
>   */
>  
> +#define LOAD_BATCHES     1024
> +#define LOAD_PER_BATCH   1024
> +#define LOAD_COUNT       (LOAD_BATCHES * LOAD_PER_BATCH)
> +
> +struct doall_delete_ctx {
> +	_LHASH *lh;
> +	int *deleted_count;
> +};
> +
>  static void
>  test_doall_count(void *arg1, void *arg2)
>  {
> @@ -226,12 +235,16 @@ static void
>  test_doall_delete_some(void *arg1, void *arg2)
>  {
>  	void *data;
> +	struct doall_delete_ctx *ctx = arg2;
>  
>  	if (arc4random_uniform(32) != 0)
>  		return;
>  
> -	data = lh_delete(arg2, arg1);
> -	free(data);
> +	data = lh_delete(ctx->lh, arg1);
> +	if (data != NULL) {
> +		free(data);
> +		(*ctx->deleted_count)++;
> +	}
>  }
>  
>  static void
> @@ -251,12 +264,13 @@ test_lhash_load(void)
>  	char *data = NULL;
>  	int i, j;
>  	int failed = 1;
> +	long remaining, expected, deleted_count = 0;
>  
>  	if ((lh = lh_new(NULL, NULL)) == NULL)
>  		goto failure;
>  
> -	for (i = 0; i < 1024; i++) {
> -		for (j = 0; j < 1024; j++) {
> +	for (i = 0; i < LOAD_BATCHES; i++) {
> +		for (j = 0; j < LOAD_PER_BATCH; j++) {
>  			if ((data = calloc(1, 128)) == NULL)
>  				goto failure;
>  
> @@ -284,16 +298,23 @@ test_lhash_load(void)
>  			}
>  			data = NULL;
>  		}
> -		lh_doall_arg(lh, test_doall_delete_some, lh);
> +
> +		struct doall_delete_ctx ctx = { lh, &deleted_count };
> +		lh_doall_arg(lh, test_doall_delete_some, &ctx);
>  	}
>  
> -	/* We should have ~31,713 entries. */
> -	if (lh_num_items(lh) < 31000 || lh_num_items(lh) > 33000) {
> -		fprintf(stderr, "FAIL: unexpected number of entries (%ld)\n",
> -		    lh_num_items(lh));
> +	remaining = lh_num_items(lh);
> +	expected = LOAD_COUNT - deleted_count;
> +
> +	if (remaining != expected) {
> +		fprintf(stderr, "FAIL: item count mismatch: remaining = %ld, expected = %ld\n",
> +		    remaining, expected);
>  		goto failure;
>  	}
>  
> +	fprintf(stdout, "PASS: inserted = %d, deleted = %ld, remaining = %ld\n",
> +	    LOAD_COUNT, deleted_count, remaining);
> +
>  	failed = 0;
>  
>   failure:
> -- 
> 2.39.5 (Apple Git-154)
>