patchfixup! bisect--helper: `bisect_next_check` & bisect_voc shell function in C

>
> 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)
>              ^~~~~~~~
>
> If you "re-roll" (or, as pointed out at the Contributors' Summit, better
> put: if you send another iteration of the patch series), please squash
> this fix in.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

> 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)
>              ^~~~~~~~

> 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.

> 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.

> René Scharfe <l.s.r@web.de> writes:
>
>> Initializing to NULL is still the correct thing to do, of course --
>> together with removing the conditionals (or at least the negations).
>
> So, let's give Pranit a concrete "here is what we want to see
> squashed in", while you guys discuss peculiarity with various
> platforms and their system headers, which admittedly is a more
> interesting tangent ;-)
>
> There are early returns with "goto finish" even before _syn
> variables are first assigned to, so they would need to be
> initialized to NULL.  The other two get their initial values
> right at the beginning, so they are OK.
>
>  builtin/bisect--helper.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 8cd6527bd1..6949e8e5ca 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -280,7 +280,7 @@ static int bisect_next_check(const struct bisect_terms *terms,
>         int missing_good = 1, missing_bad = 1, retval = 0;
>         char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad);
>         char *good_glob = xstrfmt("%s-*", terms->term_good);
> -       char *bad_syn, *good_syn;
> +       char *bad_syn = NULL, *good_syn = NULL;
>
>         if (ref_exists(bad_ref))
>                 missing_bad = 0;
> @@ -341,14 +341,10 @@ static int bisect_next_check(const struct bisect_terms *terms,
>         }
>         goto finish;
>  finish:
> -       if (!bad_ref)
> -               free(bad_ref);
> -       if (!good_glob)
> -               free(good_glob);
> -       if (!bad_syn)
> -               free(bad_syn);
> -       if (!good_syn)
> -               free(good_syn);
> +       free(bad_ref);
> +       free(good_glob);
> +       free(bad_syn);
> +       free(good_syn);
>         return retval;
>  }

> ...
> So, let's give Pranit a concrete "here is what we want to see
> squashed in", while you guys discuss peculiarity with various
> platforms and their system headers, which admittedly is a more
> interesting tangent ;-)
>
> There are early returns with "goto finish" even before _syn
> variables are first assigned to, so they would need to be
> initialized to NULL.  The other two get their initial values
> right at the beginning, so they are OK.
>
>  builtin/bisect--helper.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)