Re: [PATCH v4 11/16] ref-filter: introduce refname_atom_parser()

> On Sun, Apr 10, 2016 at 12:15:10AM +0530, Karthik Nayak wrote:
>
>> +static void refname_atom_parser(struct used_atom *atom, const char *arg)
>> +{
>> +     if (!arg)
>> +             atom->u.refname.option = R_NORMAL;
>> +     else if (!strcmp(arg, "short"))
>> +             atom->u.refname.option = R_SHORT;
>> +     else if (skip_prefix(arg, "strip=", &arg)) {
>> +             atom->u.contents.option = R_STRIP;
>> +             if (strtoul_ui(arg, 10, &atom->u.refname.strip) ||
>> +                     atom->u.refname.strip <= 0)
>> +                     die(_("positive value expected refname:strip=%s"), arg);
>
> That R_STRIP line should be setting atom->u.refname.option, right?
>
> The same mistake happens in a later patch, too, when parsing for R_BASE
> and R_DIR is added. And I think the same problem is also present in
> objectname_atom_parser.
>
> The compiler doesn't notice because, hey, it's C, and why bother
> complaining when somebody assigns the value from one enum to another?
> And the tests don't notice because the enum is at the front of each
> union member, so the compiler happens to put it in the same place (I
> think it _might_ even be guaranteed by the standard's type-punning
> rules, but that's really not something we should rely on).
>
> -Peff