On Fri, Apr 15, 2016 at 2:13 AM, Jeff King <peff@xxxxxxxx> wrote:
> 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
True, I'm surprised that went unnoticed, will fix it, thanks :)
--
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html