On 04/20, Ævar Arnfjörð Bjarmason wrote:
> Change two case statements added in commit 0281e487fd ("grep:
> optionally recurse into submodules", 2016-12-16) so that they die if
> new GREP_PATTERN_* enum fields are added without updating them.
>
> These case statements currently check for an exhaustive list of
> fields, but if a new field is added it's easy to introduce a bug here
> where the code will start subtly doing the wrong thing, e.g. if a new
> pattern type is added we'll fall through to
> GREP_PATTERN_TYPE_UNSPECIFIED, i.e. the "basic" POSIX regular
> expressions.
>
> This should arguably be done for the switch(opt->binary)
> case-statement as well, but isn't trivial to add since that code isn't
> currently working with an exhaustive list.
I was under the impression that the code wouldn't compile if there is a
missing enum field in the switch statement. Does it instead silently
fall through? I would choose not compiling over a die statement that may
not be caught during the development of a new series.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
> builtin/grep.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 3ffb5b4e81..be3dbd6957 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -495,6 +495,8 @@ static void compile_submodule_options(const struct grep_opt *opt,
> break;
> case GREP_PATTERN_TYPE_UNSPECIFIED:
> break;
> + default:
> + die("BUG: Added a new grep pattern type without updating switch statement");
> }
>
> for (pattern = opt->pattern_list; pattern != NULL;
> @@ -515,6 +517,8 @@ static void compile_submodule_options(const struct grep_opt *opt,
> case GREP_PATTERN_BODY:
> case GREP_PATTERN_HEAD:
> break;
> + default:
> + die("BUG: Added a new grep token type without updating case statement");
> }
> }
>
> --
> 2.11.0
>
--
Brandon Williams