From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 13 Nov 2013 20:01:19 GMT
Subject: Re: [PATCH v2 3/3] for-each-ref: introduce %(color:...) for color
Message-ID: <xmqqbo1odqb4.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1384335406-16332-4-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> +		} else if (!prefixcmp(name, "color")) {
> +			;

How is "%(color:short)" parsed with this code?

This part says, "Yeah, I see something starting with color", and
then later strchr(name, ':') will point formatp to "short".

Luckily, "%(colorgarbage:short)" does not even come this far because
parse_atom() would not have allowed the codeflow to, but comparing
with "color:" here may be a lot more defensive and safe, I think.

And find the color-value here, stuffing v->s inside this "else if",
continue, without letting the formatp part work on refname this
piece of code does not even set.  Just like how we handle "flag"
without falling thru to the formatp code.


> +		} else if (!strcmp(name, "flag")) {
>  			char buf[256], *cp = buf;
>  			if (ref->flag & REF_ISSYMREF)
>  				cp = copy_advance(cp, ",symref");
> @@ -729,6 +732,12 @@ static void populate_value(struct refinfo *ref)
>  				else
>  					v->s = "<>";
>  				continue;
> +			} else if (!prefixcmp(name, "color")) {
> +				char color[COLOR_MAXLEN] = "";
> +
> +				color_parse(formatp, "--format", color);
> +				v->s = xstrdup(color);
> +				continue;
>  			} else
>  				die("unknown %.*s format %s",
>  				    (int)(formatp - name), name, formatp);