threads / patch / 61137

patch, 2 partst-prio-queue: shorten array index message

Subject: [PATCH 1/2] t-prio-queue: shorten array index message

## tl;dr

3 messages between Mar 16, 2024 and Mar 18, 2024. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

René Scharfe· Mar 16, 2024, 20:45 UTC · lore

If we get an unexpected result, the prio-queue unit test reports it like this:

 # check "result[j++] == show(get)" failed at t/unit-tests/t-prio-queue.c:43
 #    left: 5
 #   right: 1
 # failed at result[] index 0

That last line repeats "failed" and "result" from the first line. Shorten it to resemble a similar one in t-ctype and also remove the incrementation from the first line to avoid possible distractions from the message of which comparison went wrong where:

 # check "result[j] == show(get)" failed at t/unit-tests/t-prio-queue.c:43
 #    left: 5
 #   right: 1
 #       j: 0

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 t/unit-tests/t-prio-queue.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Show changes to t/unit-tests/t-prio-queue.c +6 −5

diff --git a/t/unit-tests/t-prio-queue.c b/t/unit-tests/t-prio-queue.c
index d78b002f9e..616d0fc86f 100644
--- a/t/unit-tests/t-prio-queue.c
+++ b/t/unit-tests/t-prio-queue.c
@@ -31,16 +31,18 @@ static void test_prio_queue(int *input, int *result, size_t input_size)
 			get = prio_queue_get(&pq);
 			if (!check(peek == get))
 				return;
-			if(!check_int(result[j++], ==, show(get)))
-				test_msg("failed at result[] index %d", j-1);
+			if (!check_int(result[j], ==, show(get)))
+				test_msg("      j: %d", j);
+			j++;
 			break;
 		case DUMP:
 			while ((peek = prio_queue_peek(&pq))) {
 				get = prio_queue_get(&pq);
 				if (!check(peek == get))
 					return;
-				if(!check_int(result[j++], ==, show(get)))
-					test_msg("failed at result[] index %d", j-1);
+				if (!check_int(result[j], ==, show(get)))
+					test_msg("      j: %d", j);
+				j++;
 			}
 			break;
 		case STACK:
--
2.44.0

René Scharfe· Mar 16, 2024, 21:09 UTC · re: René Scharfe · lore

[PATCH 2/2] t-prio-queue: check result array bounds

Avoid reading past the end of the "result" array, which could otherwise happen if the prio-queue were to yield more items than were put into it due to an implementation bug, or if the array has not enough entries due to a test bug.

Also check at the end whether all "result" entries were consumed, which would not be the case if the prio-queue forgot some entries or the test definition contained too many.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 t/unit-tests/t-prio-queue.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Show changes to t/unit-tests/t-prio-queue.c +11 −4

diff --git a/t/unit-tests/t-prio-queue.c b/t/unit-tests/t-prio-queue.c
index 616d0fc86f..5358346361 100644
--- a/t/unit-tests/t-prio-queue.c
+++ b/t/unit-tests/t-prio-queue.c
@@ -19,11 +19,13 @@ static int show(int *v)
 	return v ? *v : MISSING;
 }

-static void test_prio_queue(int *input, int *result, size_t input_size)
+static void test_prio_queue(int *input, size_t input_size,
+			    int *result, size_t result_size)
 {
 	struct prio_queue pq = { intcmp };
+	int j = 0;

-	for (int i = 0, j = 0; i < input_size; i++) {
+	for (int i = 0; i < input_size; i++) {
 		void *peek, *get;
 		switch(input[i]) {
 		case GET:
@@ -31,6 +33,8 @@ static void test_prio_queue(int *input, int *result, size_t input_size)
 			get = prio_queue_get(&pq);
 			if (!check(peek == get))
 				return;
+			if (!check_uint(j, <, result_size))
+				break;
 			if (!check_int(result[j], ==, show(get)))
 				test_msg("      j: %d", j);
 			j++;
@@ -40,6 +44,8 @@ static void test_prio_queue(int *input, int *result, size_t input_size)
 				get = prio_queue_get(&pq);
 				if (!check(peek == get))
 					return;
+				if (!check_uint(j, <, result_size))
+					break;
 				if (!check_int(result[j], ==, show(get)))
 					test_msg("      j: %d", j);
 				j++;
@@ -56,6 +62,7 @@ static void test_prio_queue(int *input, int *result, size_t input_size)
 			break;
 		}
 	}
+	check_uint(j, ==, result_size);
 	clear_prio_queue(&pq);
 }

@@ -79,7 +86,8 @@ static void test_prio_queue(int *input, int *result, size_t input_size)
 {								\
 	int input[] = {INPUT};					\
 	int result[] = {RESULT};				\
-	test_prio_queue(input, result, ARRAY_SIZE(input));	\
+	test_prio_queue(input, ARRAY_SIZE(input),		\
+			result, ARRAY_SIZE(result));		\
 }

 TEST_INPUT(BASIC_INPUT, BASIC_RESULT, basic)
--
2.44.0

Junio C Hamano· Mar 18, 2024, 16:25 UTC · re: René Scharfe · lore

Re: [PATCH 1/2] t-prio-queue: shorten array index message

René Scharfe <l.s.r@web.de> writes:

Show 17 quoted lines

> If we get an unexpected result, the prio-queue unit test reports it like
> this:
>
>  # check "result[j++] == show(get)" failed at t/unit-tests/t-prio-queue.c:43
>  #    left: 5
>  #   right: 1
>  # failed at result[] index 0
>
> That last line repeats "failed" and "result" from the first line.
> Shorten it to resemble a similar one in t-ctype and also remove the
> incrementation from the first line to avoid possible distractions from
> the message of which comparison went wrong where:
>
>  # check "result[j] == show(get)" failed at t/unit-tests/t-prio-queue.c:43
>  #    left: 5
>  #   right: 1
>  #       j: 0

Very nice touch, especially the removal of ++ from the message ;-).

Show 34 quoted lines

> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  t/unit-tests/t-prio-queue.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/t/unit-tests/t-prio-queue.c b/t/unit-tests/t-prio-queue.c
> index d78b002f9e..616d0fc86f 100644
> --- a/t/unit-tests/t-prio-queue.c
> +++ b/t/unit-tests/t-prio-queue.c
> @@ -31,16 +31,18 @@ static void test_prio_queue(int *input, int *result, size_t input_size)
>  			get = prio_queue_get(&pq);
>  			if (!check(peek == get))
>  				return;
> -			if(!check_int(result[j++], ==, show(get)))
> -				test_msg("failed at result[] index %d", j-1);
> +			if (!check_int(result[j], ==, show(get)))
> +				test_msg("      j: %d", j);
> +			j++;
>  			break;
>  		case DUMP:
>  			while ((peek = prio_queue_peek(&pq))) {
>  				get = prio_queue_get(&pq);
>  				if (!check(peek == get))
>  					return;
> -				if(!check_int(result[j++], ==, show(get)))
> -					test_msg("failed at result[] index %d", j-1);
> +				if (!check_int(result[j], ==, show(get)))
> +					test_msg("      j: %d", j);
> +				j++;
>  			}
>  			break;
>  		case STACK:
> --
> 2.44.0