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

René Scharfe [off-list ref] writes:

On 9/12/26 12:09 AM, Junio C Hamano wrote:

quoted

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.

quoted

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.