nntp: avoid modifying $_[0] in RE conversions

I've been getting occasional t/nntp.t warnings about
uninitialized variables which I'm not able to reproduce
reliably.  My current theory is that modifying $_[0] may get
wonky when it's happening across several layers of the call
stack, so stop doing it since it's unlikely to yield any
real world speedups and only made the code more difficult
to understand, here.

diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index 457e0fb7c736bed77dc3c02db33c82c43a24fb5a..eab6d301dc80648cd7ef7f3c79a0c6d0bd07151b 100644 (file)
--- a/lib/PublicInbox/NNTP.pm
+++ b/lib/PublicInbox/NNTP.pm
@@ -121,8 +121,8 @@ sub names2ibx ($;$) {
 
 sub _list_groups (@) {
        my ($cb, $self, $wildmat) = @_;
-       wildmat2re($wildmat);
-       my @names = grep /$wildmat/, @{$self->{nntpd}->{groupnames}};
+       my $re = wildmat2re($wildmat);
+       my @names = grep /$re/, @{$self->{nntpd}->{groupnames}};
        $self->long_response($cb, names2ibx($self, \@names));
 }
 
@@ -274,11 +274,11 @@ sub cmd_newgroups ($$$;$$) {
        $self->long_response(\&newgroups_i, $ts, names2ibx($self));
 }
 
-sub wildmat2re (;$) {
-       return $_[0] = qr/.*/ if (!defined $_[0] || $_[0] eq '*');
+sub wildmat2re ($) {
+       my $tmp = $_[0] // '*';
+       return qr/.*/ if $tmp eq '*';
        my %keep;
        my $salt = rand;
-       my $tmp = $_[0];
 
        $tmp =~ s#(?<!\\)\[(.+)(?<!\\)\]#
                my $orig = $1;
@@ -295,14 +295,14 @@ sub wildmat2re (;$) {
                        defined $orig ? $orig : $1;
                        #ge;
        }
-       $_[0] = qr/\A$tmp\z/;
+       qr/\A$tmp\z/;
 }
 
-sub ngpat2re (;$) {
-       return $_[0] = qr/\A\z/ unless defined $_[0];
+sub ngpat2re ($) {
+       my $tmp = $_[0] // return qr/\A\z/;
        my %map = ('*' => '.*', ',' => '|');
-       $_[0] =~ s!(.)!$map{$1} || "\Q$1"!ge;
-       $_[0] = qr/\A(?:$_[0])\z/;
+       $tmp =~ s!(.)!$map{$1} || "\Q$1\E"!ge;
+       qr/\A(?:$tmp)\z/;
 }
 
 sub newnews_i {
@@ -332,8 +332,8 @@ sub cmd_newnews ($$$$;$$) {
        return r501 if $@;
        $self->msg_more("230 list of new articles by message-id follows\r\n");
        my ($keep, $skip) = split(/!/, $newsgroups, 2);
-       ngpat2re($keep);
-       ngpat2re($skip);
+       $keep = ngpat2re $keep;
+       $skip = ngpat2re $skip;
        my @names = grep(/$keep/, @{$self->{nntpd}->{groupnames}});
        @names = grep(!/$skip/, @names);
        return \".\r\n" unless scalar(@names);

diff --git a/t/nntp.t b/t/nntp.t
index f1801cfe0f2322b5865a7af028fb5d5e2c95c9a5..01c55d2e56388eb32e6b2488a58c0fb7e4c8f473 100644 (file)
--- a/t/nntp.t
+++ b/t/nntp.t
@@ -12,15 +12,15 @@ use POSIX qw(strftime);
        my $wm_prepare = sub {
                my ($wm) = @_;
                my $orig = qq{'$wm'};
-               PublicInbox::NNTP::wildmat2re($_[0]);
-               my $new = explain($_[0]);
-               ($orig, $new);
+               my $re = PublicInbox::NNTP::wildmat2re($wm);
+               my $new = explain($re);
+               ($orig, $new, $re);
        };
 
        my $wildmat_like = sub {
                my ($str, $wm) = @_;
-               my ($orig, $new) = $wm_prepare->($wm);
-               like($str, $wm, "$orig matches '$str' using $new");
+               my ($orig, $new, $re) = $wm_prepare->($wm);
+               like($str, $re, "$orig matches '$str' using $new");
        };
 
        my $wildmat_unlike = sub {
@@ -30,8 +30,8 @@ use POSIX qw(strftime);
                        my $re = qr/$wm/;
                        like($str, $re, "normal re with $wm matches, but ...");
                }
-               my ($orig, $new) = $wm_prepare->($wm);
-               unlike($str, $wm, "$orig does not match '$str' using $new");
+               my ($orig, $new, $re) = $wm_prepare->($wm);
+               unlike($str, $re, "$orig does not match '$str' using $new");
        };
 
        $wildmat_like->('[foo]', '[\[foo\]]');
@@ -47,8 +47,8 @@ use POSIX qw(strftime);
        my $ngpat_like = sub {
                my ($str, $pat) = @_;
                my $orig = $pat;
-               PublicInbox::NNTP::ngpat2re($pat);
-               like($str, $pat, "'$orig' matches '$str' using $pat");
+               my $re = PublicInbox::NNTP::ngpat2re($pat);
+               like($str, $re, "'$orig' matches '$str' using $re");
        };
 
        $ngpat_like->('any', '*');