Re: [PATCH] fixup! bisect--helper: `bisect_next_check` & bisect_voc shell function in C

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 1670 bytes --]

Hi René,

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

> Am 10.02.2017 um 15:20 schrieb Johannes Schindelin:
> > 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.

Ciao,
Dscho