From: René Scharfe Date: Sat, 12 Sep 2026 07:13:29 GMT Subject: Re: [PATCH] cocci: remove risky "if (!E) free(E)" conversion Message-ID: In-Reply-To: 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. On the other hand: Do we get any value out of this rule? Is it a useful guardrail? LeakSanitizer would find a forgotten free(3) call as well, given enough test coverage. René > > Signed-off-by: Junio C Hamano > --- > tools/coccinelle/free.cocci | 10 ---------- > 1 file changed, 10 deletions(-) > > diff --git a/tools/coccinelle/free.cocci b/tools/coccinelle/free.cocci > index 03799e1908..3dfaae9dd8 100644 > --- a/tools/coccinelle/free.cocci > +++ b/tools/coccinelle/free.cocci > @@ -8,16 +8,6 @@ expression E; > commit_list_free(E); > ) > > -@@ > -expression E; > -@@ > -- if (!E) > -( > - free(E); > -| > - commit_list_free(E); > -) > - > @@ > expression E; > @@