Re: [PATCH 1/7] submodule--helper: add valid-label-name

From: Junio C Hamano <gitster@pobox.com>
Date: May 11, 2016, 01:11 UTC
Message-ID: <xmqqr3d9l7pk.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1462928397-1708-2-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 7 quoted lines

> +static int submodule_valid_label_name(const char *label)
> +{
> +	if (!label || !strlen(label))
> +		return 0;
> +
> +	if (!isalnum(*label))
> +		return 0;

I'd limit this one to isalpha() if I were doing this to make the restriction similar to identifiers in traditional programming language.

> +	while (*label) {
> +		if (!(isalnum(*label) ||
> +			*label == '-'))

And throw in '_' to the mix while at it.

Show 6 quoted lines

> +			return 0;
> +		label++;
> +	}
> +
> +	return 1;
> +}

If the convention is "0 is good", then please signal "bad" with a negative value, not just "non-zero".