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

[Jeff King <peff@peff.net>]

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