Re: [PATCH 18/29] grep: catch a missing enum in switch statement
- From
Stefan Beller <sbeller@google.com>
- Date
- May 11, 2017, 20:40 UTC
- Message-ID
- <CAGZ79kb_ueU8hArtA3yvYywvawnuD0RGQBtm1=OEuQi2L0rjmA@mail.gmail.com>
- In-Reply-To
- <20170511200804.GF83655@google.com>
On Thu, May 11, 2017 at 1:08 PM, Brandon Williams <bmwill@google.com> wrote:
Show 35 quoted lines
> On 05/11, Ævar Arnfjörð Bjarmason wrote:
>> Add a die(...) to a default case for the switch statement selecting
>> between grep pattern types under --recurse-submodules.
>>
>> Normally this would be caught by -Wswitch, but the grep_pattern_type
>> type is converted to int by going through parse_options(). Changing
>> the argument type passed to compile_submodule_options() won't work,
>> the value will just get coerced.
>>
>> Thus catching this at runtime is the least worst option. This won't
>> ever trigger in practice, but if a new pattern type were to be added
>> this catches an otherwise silent bug during development.
>>
>> See commit 0281e487fd ("grep: optionally recurse into submodules",
>> 2016-12-16) for the initial addition of this code.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>> builtin/grep.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/builtin/grep.c b/builtin/grep.c
>> index 3ffb5b4e81..1c0adb30f3 100644
>> --- a/builtin/grep.c
>> +++ b/builtin/grep.c
>> @@ -495,6 +495,12 @@ static void compile_submodule_options(const struct grep_opt *opt,
>> break;
>> case GREP_PATTERN_TYPE_UNSPECIFIED:
>> break;
>> + default:
>> + /*
>> + * -Wswitch doesn't catch this due to casting &
>> + * -Wswitch-default is too noisy.
>> + */
>> + die("BUG: Added a new grep pattern type without updating switch statement");I am not sure if the comment is of enough value for the used screen real estate value. People interested in the existence of the default would just use blame/log to find out.
Thanks, Stefan