Junio C Hamano [off-list ref] writes:
quoted
+ 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.
On Thu, Aug 28, 2014 at 02:36:22PM -0700, Junio C Hamano wrote:
Junio C Hamano [off-list ref] writes:
quoted
quoted
+ 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);
Hmm. But that is because "git tag" always makes one type of tag: one in
which the "tag" field is the same as the refname in which we store it.
So the name must be a valid refname there to meet the ref storage
requirement, and therefore the tag name must, too.
But is that something we necessarily need or want to enforce? Is it OK
for me to have refs/tags/foo pointing to a tag object that is not
related to "foo" (either semantically or syntactically)?
I dunno. I cannot think of a reason you would want to do such a thing,
but this seems like outlawing it because git does not generate it, not
because it is necessarily a problematic thing to be doing.
-Peff