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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Krzesimir Nowak <krzesimir@xxxxxxxxxxxx> writes:

> This check will be used in more than one place later.
>
> Signed-off-by: Krzesimir Nowak <krzesimir@xxxxxxxxxxxx>
> Reviewed-by: Junio C Hamano <gitster@xxxxxxxxx>
> Reviewed-by: Jakub Narębski <jnareb@xxxxxxxxx>
> Reviewed-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
> ---
>  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.

>  }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]