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

Re: [PATCH] cocci: remove risky "if (!E) free(E)" conversion

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 12, 2026, 19:07 UTC
Message-ID
<xmqqcxui8f0c.fsf@gitster.g>
In-Reply-To
<caa39ca4-b35e-4fff-80fb-af6856cb2098@web.de>
René Scharfe <l.s.r@web.de> writes:
Show 34 quoted lines
> On 9/12/26 12:09 AM, Junio C Hamano wrote:
>> The current cocci patches try to convert
>> 
>> 	if (!E)
>> 		free(E);
>> 
>> into an unconditional call to free(E), with the rationale
>> 
>>     cocci: detect useless free(3) calls
>> 
>>     Add a semantic patch for removing checks that cause free(3) to only be
>>     called with a NULL pointer, as that must be a programming mistake.
>> 
>> which came from ec6cd14c7a (cocci: detect useless free(3) calls,
>> 2017-02-11).
>> 
>> Leaving _something_ in ALL.patch output to draw programmers'
>> attention is a good thing, but this changes a piece of code that is
>> originally a no-op to do something else, which may be even worse.
>
> Good point.  It's likely that the programmer just wanted to release the
> object in question and got the check wrong, but it's also possible that
> the free(3) call is wrong as well, and that could do real damage.
>> We could change it to
>> 
>> 	if (!E)
>> 		BUG("free(E) is certainly not what we meant to write");
>> 
>> to force programmers to think.  But it probably is safer to just
>> rewrite one form of no-op into a simpler form of no-op.
>
> With that last sentence I expected the patch to also remove the free(3)
> or commit_list_free() call, replacing the no-op with nothing, which is
> safe and simple.
You mean
	 if (!E)
	-  free(E);
	+  ; /* no op free(E) */

or something? I guess we could do so, but I feared that a compiler that is smart enough complain and trip -Werror on us when E is too obviously a side-effect free expression such as a reference to a simple variable.

Previous: René ScharfeNext: René Scharfe
Message 4 of 5 in “cocci: remove risky "if (!E) free(E)" conversion”
  1. cocci: remove risky "if (!E) free(E)" conversionJunio C Hamano, Sep 11, 2026
  2. cocci: FREE_AND_NULL(E) is safe to call on NULLJunio C Hamano, Sep 11, 2026
  3. René ScharfeSep 12, 2026
  4. Junio C HamanoSep 12, 2026
  5. René ScharfeSep 13, 2026

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.