Re: [PATCHv9 4/4] pathspec: allow querying for attributes

From: Junio C Hamano <gitster@pobox.com>
Date: May 19, 2016, 23:32 UTC
Message-ID: <xmqqbn41hbeq.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160519232323.12775-5-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> +attr;;
> +After `attr:` comes a space separated list of "attribute
> +...
> ++

The text looks OK, but does it format well?

> +		attr_len = strcspn(attr, "=");

Scanning for '=' here retains the same bug from the previous iteration where you take !VAR=VAL and silently ignore =VAL part without diagnosing the error, doesn't it?

Perhaps strlen(attr) here, and...

Show 22 quoted lines

> +		switch (*attr) {
> +		case '!':
> +			am->match_mode = MATCH_UNSPECIFIED;
> +			attr++;
> +			attr_len--;
> +			break;
> +		case '-':
> +			am->match_mode = MATCH_UNSET;
> +			attr++;
> +			attr_len--;
> +			break;
> +		default:
> +			if (attr[attr_len] != '=')
> +				am->match_mode = MATCH_SET;
> +			else {
> +				am->match_mode = MATCH_VALUE;
> +				am->value = xstrdup(&attr[attr_len + 1]);
> +				if (strchr(am->value, '\\'))
> +					die(_("attr spec values must not contain backslashes"));
> +			}
> +			break;
> +		}

... doing strcspn() only in default: part would be a quick fix.