From 3988b91d29ea037df1f1790ca18d4457e833d87b Mon Sep 17 00:00:00 2001
From: Eric Wong <e@80x24.org>
Date: Thu, 27 Mar 2025 23:20:45 +0000
Subject: [PATCH] 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.
---
 lib/PublicInbox/NNTP.pm | 24 ++++++++++++------------
 t/nntp.t                | 18 +++++++++---------
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/lib/PublicInbox/NNTP.pm b/lib/PublicInbox/NNTP.pm
index 457e0fb7c..eab6d301d 100644
--- 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 f1801cfe0..01c55d2e5 100644
--- 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', '*');
-- 
2.47.2