Thread (53 messages) 53 messages, 6 authors, 2017-05-30

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

From: Brandon Williams <hidden>
Date: 2017-05-11 20:08:11

On 05/11, Ævar Arnfjörð Bjarmason wrote:
quoted hunk ↗ jump to hunk
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");
 	}
Thanks for adding this, as I got it wrong while developing this bit of
code.
 
 	for (pattern = opt->pattern_list; pattern != NULL;
-- 
2.11.0
-- 
Brandon Williams
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help