Re: [PATCH v2 02/11] parse-options.[ch]: consistently use "enum parse_opt_flags"
From: Junio C Hamano <hidden>
Date: 2021-10-01 21:45:22
Ævar Arnfjörð Bjarmason [off-list ref] writes:
Use the "enum parse_opt_flags" instead of an "int flags" as arguments to the various functions in parse-options.c.
OK.
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.
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)