On Tue, Dec 3, 2013 at 8:02 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Krzesimir Nowak <krzesimir@xxxxxxxxxxxx> writes:
>> +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;
>> +}
[...]
>> @@ -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.
I think this simply follows "$input = validate_sth($input) or return undef;"
pattern used in this area of gitweb code (perhaps mis-used).
Stricly speaking pure refactoring (no functional change, e.g. no assign
to $input) would be "check_ref_format($input) or return undef;", or even
"return check_ref_format($input);" if we keep check_ref_format() passthru
on valid refname.
--
Jakub Narebski
--
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