Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
I think that many points you raised in your message are valid, but
there is one thing that is not.
>> +enum {
>> + MODE_UNSPECIFIED = 0,
>> + MODE_NAME_ONLY,
>> + MODE_OID_ONLY
>> +};
>> +
>> +static int cmdmode = MODE_UNSPECIFIED;
>
> Better:
>
> static enum {
> MODE_NAME_ONLY = 1,
> ...
> } cmdmode = MODE_NAME_ONLY;
>
> I.e. no need for the MODE_UNSPECIFIED just to skip past "0".
If the original wanted to make the default to be "unspecified", your
suggestion changes the semantics.
"enum" is not necessarily an "int", and because the pointer of
"cmdmode" is given to OPT_CMDMODE(), which expects a pointer to
"int", your suggestion breaks the code there, too.
I wonder if cmdmode cannot be a on-stack variable in cmd_ls_tree()
that is passed as the context pointer to show_tree() via
read_tree(), though. The enum definition still need to be visible
throughout the file, but such a structure would let us lose a
"global" variable.
Thanks.
