"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: use size_t for loop indices to avoid signedness warnings
To:
Ed Maste <emaste@freebsd.org>
Cc:
gameoftrees@openbsd.org
Date:
Thu, 17 Dec 2020 21:48:42 +0100

Download raw body.

Thread
On Thu, Dec 17, 2020 at 09:52:58AM -0500, Ed Maste wrote:
> Same change as 16aeacf7088d, for subdirectories other than lib/

Unfortunately, this patch introduced a segfault in one of the tests:

> @@ -175,7 +175,7 @@ path_list_reverse_input(void)
>  	const struct got_error *err = NULL;
>  	struct got_pathlist_head paths;
>  	struct got_pathlist_entry *pe;
> -	int i;
> +	size_t i;
>  
>  	TAILQ_INIT(&paths);
>  	for (i = nitems(path_list_input) - 1; i >= 0; i--) {

Note that the condition i >= 0 is always true for an unsigned type.
The loop no longer stops iterating and eventually attempts to index
the array path_list_input with a very large value after i overflows.

I guess changing i to ssize_t would be one possibility, but I don't
know if that will bring back the warnings you were trying to avoid.
So instead I have removed the assumption that i can become negative:

commit 8891c2aba6a5f6cd43ab51b67115d4fbd0976f73 (main)
from: Stefan Sperling <stsp@stsp.name>
date: Thu Dec 17 20:46:54 2020 UTC
 
 fix crash in path_list_reverse_input() after switch to unsigned loop index
 
diff e8f0f2f60d1eb5651a01b13607ca04014f092753 a46ac2cbc24e20b46540307ad7cfa4fca47c566a
blob - 792c8cdc3d09be7a750bb57780cb89ea60c1e627
blob + 231a6477402c69e760b2ab21b75fee238ebbddff
--- regress/path/path_test.c
+++ regress/path/path_test.c
@@ -178,7 +178,7 @@ path_list_reverse_input(void)
 	size_t i;
 
 	TAILQ_INIT(&paths);
-	for (i = nitems(path_list_input) - 1; i >= 0; i--) {
+	for (i = nitems(path_list_input) - 1; i > 0; i--) {
 		err = got_pathlist_insert(NULL, &paths, path_list_input[i],
 		    NULL);
 		if (err) {
@@ -186,6 +186,11 @@ path_list_reverse_input(void)
 			return 0;
 		}
 	}
+	err = got_pathlist_insert(NULL, &paths, path_list_input[0], NULL);
+	if (err) {
+		test_printf("%s\n", __func__, err->msg);
+		return 0;
+	}
 
 	i = 0;
 	TAILQ_FOREACH(pe, &paths, entry) {