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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Æ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)




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux