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

From: Jeff King <peff@peff.net>
Date: Jan 7, 2020, 11:01 UTC
Message-ID: <20200107110145.GA1073219@coredump.intra.peff.net>
In-Reply-To: <20200107013640.1821227-1-sandals@crustytoothpaste.net>

On Tue, Jan 07, 2020 at 01:36:40AM +0000, brian m. carlson wrote:

Show 14 quoted lines

> 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.

OK, I agree it makes sense to be on the safe side here (and the patch is obviously the right fix).

> Noticed-by: Miriam R. <mirucam@gmail.com>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>

I think Miriam actually posted the same patch in her initial email:

  https://lore.kernel.org/git/CAN7CjDDBA0ZoCG9aaQf5rg3gxqny=EjR6v6jE1mnxvUJQSF_0Q@mail.gmail.com/

I don't know how we want to handle authorship.

-Peff