Re: [PATCH 4/6] fsck: check tag objects' headers

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 28, 2014, 21:36 UTC
Message-ID: <xmqqegw08fft.fsf@gitster.dls.corp.google.com>
In-Reply-To: <xmqqlhq88fyb.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 5 quoted lines

>> +	if (check_refname_format(buffer, REFNAME_ALLOW_ONELEVEL))
>> +		ret = error_func(&tag->object, FSCK_ERROR, "invalid 'tag' name: %s", buffer);
>> +	*eol = '\n';
>
> I actually think this check is harmful.

Let me take this one back; we do a moral equivalent when we create a tag, like this:

	strbuf_addf(sb, "refs/tags/%s", name);
        return check_refname_format(sb->buf, 0);

So validating using check_refname_format() is indeed a very good thing to do.

As you have length and buffer here, I would suggest updating this part of your patch to print into a strbuf

	strbuf_addf(&sb, "refs/tags/%.*s", (eol - buffer), buffer);
       	if (check_refname_format(sb.buf))
        	ret = ...

and keep the constness of the incoming data.