On Thu, Dec 17, 2020 at 03:55:39PM -0700, Todd C. Miller wrote:
> On Thu, 17 Dec 2020 21:48:42 +0100, Stefan Sperling wrote:
> 
> > 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:
> 
> You could also just iterate over the number of items instead of the
> indexes into the array and move the decrement to immediately before
> the call to got_pathlist_insert() to adjust from item number to
> array index.  For example:
> 
> 	for (i = nitems(path_list_input); i > 0; ) {
> 		i--;
> 		err = got_pathlist_insert(NULL, &paths, path_list_input[i],
> 		    NULL);
> 		if (err) {
> 			test_printf("%s\n", __func__, err->msg);
> 			return 0;
> 		}
> 	}
> 
> Or alternately without the standalone decrememnt:
> 
> 		err = got_pathlist_insert(NULL, &paths, path_list_input[--i],
> 		    NULL);
> 
>  - todd

Thank you!

I have commited this just now:

-----------------------------------------------
commit a4153d5b903b1fe20edad776a6d0ec3c3cbcc0aa (main, origin/main)
from: Stefan Sperling <stsp@stsp.name>
date: Thu Dec 17 23:13:10 2020 UTC
 
 more concise fix for path_list_reverse_input() crash; suggested by millert
 
diff a46ac2cbc24e20b46540307ad7cfa4fca47c566a d9be8199616dc843dfa00d6d376b126a0b26b44f
blob - 231a6477402c69e760b2ab21b75fee238ebbddff
blob + cb17e05405edca193cab8ab4da47e83af9ed79a6
--- regress/path/path_test.c
+++ regress/path/path_test.c
@@ -178,19 +178,14 @@ path_list_reverse_input(void)
 	size_t i;
 
 	TAILQ_INIT(&paths);
-	for (i = nitems(path_list_input) - 1; i > 0; i--) {
-		err = got_pathlist_insert(NULL, &paths, path_list_input[i],
+	for (i = nitems(path_list_input); i > 0;) {
+		err = got_pathlist_insert(NULL, &paths, path_list_input[--i],
 		    NULL);
 		if (err) {
 			test_printf("%s\n", __func__, err->msg);
 			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) {