Re: [PATCH 1/3] gitweb: Move check-ref-format code into separate function

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 3, 2013, 19:02 UTC
Message-ID: <xmqqd2ldoj0s.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1386082603-8404-2-git-send-email-krzesimir@endocode.com>

Krzesimir Nowak <krzesimir@endocode.com> writes:

Show 39 quoted lines

> This check will be used in more than one place later.
>
> Signed-off-by: Krzesimir Nowak <krzesimir@endocode.com>
> Reviewed-by: Junio C Hamano <gitster@pobox.com>
> Reviewed-by: Jakub Narębski <jnareb@gmail.com>
> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
>  gitweb/gitweb.perl | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 68c77f6..f7730d7 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1452,6 +1452,16 @@ sub validate_pathname {
>  	return $input;
>  }
>  
> +sub check_ref_format {
> +	my $input = shift || return undef;
> +
> +	# restrictions on ref name according to git-check-ref-format
> +	if ($input =~ m!(/\.|\.\.|[\000-\040\177 ~^:?*\[]|/$)!) {
> +		return undef;
> +	}
> +	return $input;
> +}
> +
>  sub validate_refname {
>  	my $input = shift || return undef;
>  
> @@ -1462,10 +1472,9 @@ sub validate_refname {
>  	# it must be correct pathname
>  	$input = validate_pathname($input)
>  		or return undef;
> -	# restrictions on ref name according to git-check-ref-format
> -	if ($input =~ m!(/\.|\.\.|[\000-\040\177 ~^:?*\[]|/$)!) {
> -		return undef;
> -	}

So far, so good.

> +	# check git-check-ref-format restrictions
> +	$input = check_ref_format($input)
> +		or return undef;
>  	return $input;

Hmmm. Why do you need "<LF><INDENT>or return under" here? It would not hurt too much per-se (strictly speaking, if the $input were a string "0", this will return undef instead of "0", which should be an OK name as far as the regexp is concerned), but it seems to be making the logic unnecessarily complex for no real gain.

>  }