Re: [PATCH v6 1/3] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

[Pranit Bauva <pranit.bauva@gmail.com>]

Hey Eric,

On Mon, May 16, 2016 at 12:37 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Thu, May 12, 2016 at 1:32 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>> `--next-all` is meant to be used as a subcommand to support multiple
>> "operation mode" though the current implementation does not contain any
>> other subcommand along side with `--next-all` but further commits will
>> include some more subcommands.
>>
>> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
>> ---
>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
>> @@ -23,9 +23,14 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>> -       if (!next_all)
>> +       if (!cmdmode)
>>                 usage_with_options(git_bisect_helper_usage, options);
>>
>> -       /* next-all */
>> -       return bisect_next_all(prefix, no_checkout);
>> +       switch (cmdmode) {
>> +       case NEXT_ALL:
>> +               return bisect_next_all(prefix, no_checkout);
>> +       default:
>> +               die("BUG: unknown subcommand '%d'", cmdmode);
>> +       }
>> +       return 0;
>
> What happens if you remove this useless 'return 0'? Does the (or some)
> compiler incorrectly complain about it falling off the end of the
> function without returning a value?

I tried removing it. It works fine with gcc and clang. You can see the
build on travis-CI[1]. I am not sure of other compilers and also don't
know a way to test it either. You could use my branch on github[2] if
you want to test it on other compilers. I think its better to keep the
return 0 if we aren't sure whether it would work on every compiler.

[1]: https://travis-ci.org/pranitbauva1997/git/builds/131622175
[2]: https://github.com/pranitbauva1997/git/tree/return-try

Regards,
Pranit Bauva