[PATCH] run-command: avoid undefined behavior in exists_in_PATH

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Jan 7, 2020, 01:36 UTC
Message-ID: <20200107013640.1821227-1-sandals@crustytoothpaste.net>

In this function, we free the pointer we get from locate_in_PATH and then check whether it's NULL. However, this is undefined behavior if the pointer is non-NULL, since the C standard no longer permits us to use a valid pointer after freeing it.

The only case in which the C standard would permit this to be defined behavior is if r were NULL, since it states that in such a case "no action occurs" as a result of calling free.

It's easy to suggest that this is not likely to be a problem, but we know that GCC does aggressively exploit the fact that undefined behavior can never occur to optimize and rewrite code, even when that's contrary to the expectations of the programmer. It is, in fact, very common for it to omit NULL pointer checks, just as we have here.

Since it's easy to fix, let's do so, and avoid a potential headache in the future.

Noticed-by: Miriam R. <mirucam@gmail.com>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 run-command.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index 9942f120a9..f5e1149f9b 100644
--- a/run-command.c
+++ b/run-command.c
@@ -213,8 +213,9 @@ static char *locate_in_PATH(const char *file)
 static int exists_in_PATH(const char *file)
 {
 	char *r = locate_in_PATH(file);
+	int found = r != NULL;
 	free(r);
-	return r != NULL;
+	return found;
 }
 
 int sane_execvp(const char *file, char * const argv[])