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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Tue, Jan 07, 2020 at 01:36:40AM +0000, brian m. carlson wrote:
>
>> 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.

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.