Re: [RFC/PATCH] tag: disallow '-' as tag name

From: Junio C Hamano <gitster@pobox.com>
Date: May 9, 2011, 23:08 UTC
Message-ID: <7v62pjo4km.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4DC87113.4030204@elegosoft.com>

Michael Schubert <mschub@elegosoft.com> writes:

> Add strbuf_check_tag_ref() as helper to check a refname for a tag.
>
> Signed-off-by: Michael Schubert <mschub@elegosoft.com>
> ---

That was quick ;-).

Show 15 quoted lines

>  builtin/tag.c |   30 ++++++++++++++++++++++--------
>  1 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/tag.c b/builtin/tag.c
> index b66b34a..f087a7f 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -352,11 +352,26 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
>  	return 0;
>  }
>  
> +static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)
> +{
> +	if (name[0] == '-')
> +		return CHECK_REF_FORMAT_ERROR;

So contrary to what the title claims, it forbids a tag that begins with '-', e.g. '-foo', not just a single dash. That is fine by me (we do the same in strbuf-check-branch-ref) but it needs to be explained better.

> +	strbuf_reset(sb);
> +	strbuf_add(sb, "refs/tags/", 10);
> +	strbuf_add(sb, name, strlen(name));

strbuf_addf(sb, "refs/tags/%s", name)?

> +	if (sb->len > PATH_MAX)
> +		die(_("tag name too long: %.*s..."), 50, name);

I think that should be

	if (PATH_MAX <= sb->len)

but I do not see the point of checking against PATH_MAX if you are already using a strbuf...