Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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)