Re: [PATCH v2 02/11] parse-options.[ch]: consistently use "enum parse_opt_flags"

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 1, 2021, 21:45 UTC
Message-ID: <xmqq7dew4dxu.fsf@gitster.g>
In-Reply-To: <patch-v2-02.11-99f5251c557-20211001T142631Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Use the "enum parse_opt_flags" instead of an "int flags" as arguments
> to the various functions in parse-options.c.

OK.

Show 7 quoted lines

> In C enums aren't first-class types, and the "enum
> parse_opt_option_flag" uses a enum-as-bitfield pattern. So unlike
> exhaustively enumerated "case" arms we're not going to get validation
> that we used the "right" enum labels.
>
> I.e. this won't catch the sort of bug that was fixed with
> "PARSE_OPT_SHELL_EVAL" in the preceding commit.

Drop the two paragraphs above. You do not sell a patch by saying what benefit it does *not* give us. We do buy a patch by what benefit it does give us, which you describe well below.

Show 15 quoted lines

> But there's still a benefit to doing this when it comes to the wider C
> ecosystem. E.g. the GNU debugger (gdb) will helpfully detect and print
> out meaningful enum labels in this case. Here's the output before and
> after when breaking in "parse_options()" after invoking "git stash
> show":
>
> Before:
>
>     (gdb) p flags
>     $1 = 9
>
> After:
>
>     (gdb) p flags
>     $1 = (PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_UNKNOWN)