git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 7, 2020, 16:58 UTC
Message-ID
<xmqqzhez5hlq.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<20200107110145.GA1073219@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 28 quoted lines
> 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.
Previous: Jeff KingNext: brian m. carlson
Message 8 of 11 in “run-command: avoid undefined behavior in exists_in_PATH”
  1. run-command: avoid undefined behavior in exists_in_PATHbrian m. carlson, Jan 7, 2020
  2. Jonathan NiederJan 7, 2020
  3. brian m. carlsonJan 7, 2020
  4. Bryan TurnerJan 7, 2020
  5. Bryan TurnerJan 7, 2020
  6. Jeff KingJan 7, 2020
  7. Jeff KingJan 7, 2020
  8. Junio C HamanoJan 7, 2020
  9. brian m. carlsonJan 8, 2020
  10. Miriam R.Jan 8, 2020
  11. Christian CouderJan 8, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.