On Tue, 2013-12-03 at 20:38 +0100, Jakub Narębski wrote:
> 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).
>
Reading the validate_* subs and check_ref_format sub makes me think that
they should just return either 0 when validation fails or 1 when it
succeeds.
If I add an extra branch ref named "0", then check_ref_format just
returns "0" as it matches the git-check-ref-format style regex. But "0"
is coerced to 0 and validate_and_filter_extra_branches will error out. I
actually reproduced that.
> 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.
>
I fixed that too.
--
Krzesimir Nowak
Software Developer
Endocode AG
krzesimir@xxxxxxxxxxxx
------
Endocode AG, Johannisstraße 20, 10117 Berlin
info@xxxxxxxxxxxx | www.endocode.com
Vorstandsvorsitzender: Mirko Boehm
Vorstände: Dr. Karl Beecher, Chris Kühl, Sebastian Sucker
Aufsichtsratsvorsitzende: Jennifer Beecher
Registergericht: Amtsgericht Charlottenburg - HRB 150748 B
--
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