Am 13.02.2017 um 17:23 schrieb Johannes Schindelin:

Hi René,

On Fri, 10 Feb 2017, René Scharfe wrote:

quoted

Am 10.02.2017 um 15:20 schrieb Johannes Schindelin:

quoted

It is curious that only MacOSX builds trigger an error about this, both
GCC and Clang, but not Linux GCC nor Clang (see
https://travis-ci.org/git/git/jobs/200182819#L1152 for details):

builtin/bisect--helper.c:299:6: error: variable 'good_syn' is used
  uninitialized whenever 'if' condition is true
  [-Werror,-Wsometimes-uninitialized]
        if (missing_good && !missing_bad && current_term &&
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
builtin/bisect--helper.c:350:7: note: uninitialized use occurs here
        if (!good_syn)
             ^~~~~~~~

The only way that good_syn could be used in the if block is by going to the
label finish, which does the following before returning:

	if (!bad_ref)
		free(bad_ref);
	if (!good_glob)
		free(good_glob);
	if (!bad_syn)
		free(bad_syn);
	if (!good_syn)
		free(good_syn);

On Linux that code is elided completely -- freeing NULL is a no-op.  I guess
free(3) has different attributes on OS X and compilers don't dare to optimize
it away there.

So instead of calling free(3) only in the case when we did not allocate memory
(which makes no sense and leaks) we should either call it in the opposite
case, or (preferred) unconditionally, as it can handle the NULL case itself.
Once that's fixed initialization will be required even on Linux.

Exactly, free(NULL) is a no-op. The problem before this fixup was that
good_syn was not initialized to NULL.

Strictly speaking: no.  The value doesn't matter -- the free(3) calls
above will be done with NULL regardless, due to the conditionals.
Setting bad_syn and good_syn to an invalid pointer would have calmed
the compiler just as well, and would have had no ill side effect (i.e.
no invalid free(3) call).

Initializing to NULL is still the correct thing to do, of course --
together with removing the conditionals (or at least the negations).

But back to the topic of why the compilers on OS X didn't optimize out
the free(3) calls with their conditionals.  AFAICS no attributes are
set for the function in stdlib.h of in glibc[1] or Darwin[2].  And I
can't see any relevant option in config.mak.uname (e.g. -no-builtin).
It's not terribly important, but does anyone know what prevents the
elision of "if (!p) free(p);" on OS X?

René


[1] https://sourceware.org/git/?p=glibc.git;a=blob;f=stdlib/stdlib.h;h=292c6a2f053a2a578cd09d75307c26ed191e1c00;hb=b987917e6aa7ffe2fd74f0b6a989438e6edd0727
[2] https://opensource.apple.com/source/Libc/Libc-1158.30.7/include/stdlib.h.auto.html