From: Junio C Hamano Date: Sat, 12 Sep 2026 19:07:47 GMT Subject: Re: [PATCH] cocci: remove risky "if (!E) free(E)" conversion Message-ID: In-Reply-To: René Scharfe writes: > 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.