Thread (1 message) 1 message, 1 author, 2021-10-01

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)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help