Re: [PATCH 18/29] grep: catch a missing enum in switch statement

On Thu, May 11, 2017 at 10:40 PM, Stefan Beller [off-list ref] wrote:

On Thu, May 11, 2017 at 1:08 PM, Brandon Williams [off-list ref] wrote:

quoted

On 05/11, Ævar Arnfjörð Bjarmason wrote:

quoted

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 <redacted>
---
 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, I was on the fence about it, will remove it (and add the
mention of -Wswitch-default to the commit message).