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

Jeff King [off-list ref] writes:

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

quoted

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

quoted

Noticed-by: Miriam R. [off-list ref]
Signed-off-by: brian m. carlson <redacted>

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

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

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

I think the explanation in the log message has as much value as, if
not more than, the actual patch text, in this case.  Noticed-by: may
be striking the right balance.

Thanks, all.