Re: [PATCH] cocci: remove risky "if (!E) free(E)" conversion
flat view
From: René Scharfe <hidden>
Date: 2026-09-12 07:13:31
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é
quoted hunk ↗ jump to hunk
Signed-off-by: Junio C Hamano <redacted> --- 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; @@