unofficial mirror of meta@public-inbox.org
 help / color / mirror / Atom feed
* [PATCH 0/2] avoid problematic conditional hash assignments
@ 2021-11-01 19:06 Eric Wong
  2021-11-01 19:06 ` [PATCH 1/2] idx_stack: avoid conditional hash assignment weirdness Eric Wong
  2021-11-01 19:06 ` [PATCH 2/2] treewide: kill problematic "$h->{k} //= do {" assignments Eric Wong
  0 siblings, 2 replies; 3+ messages in thread
From: Eric Wong @ 2021-11-01 19:06 UTC (permalink / raw)
  To: meta

Perhaps these will fix some occasional test failures I'm seeing

Eric Wong (2):
  idx_stack: avoid conditional hash assignment weirdness
  treewide: avoid problematic "$h->{k} //= do {" assignments

 lib/PublicInbox/Config.pm    | 7 +++----
 lib/PublicInbox/Git.pm       | 2 +-
 lib/PublicInbox/IMAP.pm      | 4 ++--
 lib/PublicInbox/IdxStack.pm  | 4 ++--
 lib/PublicInbox/Inbox.pm     | 2 +-
 lib/PublicInbox/LEI.pm       | 9 +++++----
 lib/PublicInbox/SharedKV.pm  | 4 ++--
 lib/PublicInbox/WWW.pm       | 4 ++--
 lib/PublicInbox/WwwStatic.pm | 2 +-
 lib/PublicInbox/WwwStream.pm | 4 ++--
 10 files changed, 21 insertions(+), 21 deletions(-)

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 1/2] idx_stack: avoid conditional hash assignment weirdness
  2021-11-01 19:06 [PATCH 0/2] avoid problematic conditional hash assignments Eric Wong
@ 2021-11-01 19:06 ` Eric Wong
  2021-11-01 19:06 ` [PATCH 2/2] treewide: kill problematic "$h->{k} //= do {" assignments Eric Wong
  1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2021-11-01 19:06 UTC (permalink / raw)
  To: meta

I've been seeing the following error on occasion during "make check-run":
$PWD/t/data-gen/reindex-time-range.v1-master index failed: Modification of a read-only value attempted at $DIR/lib/PublicInbox/SearchIdx.pm line 899, <$r> line 1.

Perhaps this fixes it.  In any case, a construct of:

	 $h->{k} //= do { $h->{x} = ...; $val };

seems wrong and may cause Perl to error out depending on how
hashes are randomized.
---
 lib/PublicInbox/IdxStack.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/PublicInbox/IdxStack.pm b/lib/PublicInbox/IdxStack.pm
index d5123006..54d480bd 100644
--- a/lib/PublicInbox/IdxStack.pm
+++ b/lib/PublicInbox/IdxStack.pm
@@ -20,12 +20,12 @@ sub new {
 sub push_rec {
 	my ($self, $file_char, $at, $ct, $blob_oid, $cmt_oid) = @_;
 	my $rec = pack(PACK_FMT, $file_char, $at, $ct, $blob_oid, $cmt_oid);
-	$self->{unpack_fmt} //= do {
+	$self->{unpack_fmt} // do {
 		my $len = length($cmt_oid);
 		my $fmt = PACK_FMT;
 		$fmt =~ s/H\*/H$len/g;
 		$self->{rec_size} = length($rec);
-		$fmt;
+		$self->{unpack_fmt} = $fmt;
 	};
 	print { $self->{wr} } $rec or die "print: $!";
 	$self->{tot_size} += length($rec);

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/2] treewide: kill problematic "$h->{k} //= do {" assignments
  2021-11-01 19:06 [PATCH 0/2] avoid problematic conditional hash assignments Eric Wong
  2021-11-01 19:06 ` [PATCH 1/2] idx_stack: avoid conditional hash assignment weirdness Eric Wong
@ 2021-11-01 19:06 ` Eric Wong
  1 sibling, 0 replies; 3+ messages in thread
From: Eric Wong @ 2021-11-01 19:06 UTC (permalink / raw)
  To: meta

As stated in the previous change, conditional hash assignments
which trigger other hash assignments seem problematic, at times.
So replace:

	$h->{k} //= do { $h->{x} = ...; $val };

	$h->{k} // do {
		$h->{x} = ...;
		$hk->{k} = $val
	};

"||=" is affected the same way, and some instances of "||=" are
replaced with "//=" or "// do {", now.
---
 lib/PublicInbox/Config.pm    | 7 +++----
 lib/PublicInbox/Git.pm       | 2 +-
 lib/PublicInbox/IMAP.pm      | 4 ++--
 lib/PublicInbox/Inbox.pm     | 2 +-
 lib/PublicInbox/LEI.pm       | 9 +++++----
 lib/PublicInbox/SharedKV.pm  | 4 ++--
 lib/PublicInbox/WWW.pm       | 4 ++--
 lib/PublicInbox/WwwStatic.pm | 2 +-
 lib/PublicInbox/WwwStream.pm | 4 ++--
 9 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index 41117ac5..0f002e5e 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -377,8 +377,8 @@ sub get_1 {
 
 sub repo_objs {
 	my ($self, $ibxish) = @_;
-	my $ibx_code_repos = $ibxish->{coderepo} or return;
-	$ibxish->{-repo_objs} //= do {
+	my $ibx_code_repos = $ibxish->{coderepo} // return;
+	$ibxish->{-repo_objs} // do {
 		my $code_repos = $self->{-code_repos};
 		my @repo_objs;
 		for my $nick (@$ibx_code_repos) {
@@ -395,10 +395,9 @@ sub repo_objs {
 			push @repo_objs, $repo if $repo;
 		}
 		if (scalar @repo_objs) {
-			\@repo_objs;
+			$ibxish ->{-repo_objs} = \@repo_objs;
 		} else {
 			delete $ibxish->{coderepo};
-			undef;
 		}
 	}
 }
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 4078dd5b..309f80db 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -71,7 +71,7 @@ sub new {
 
 sub git_path ($$) {
 	my ($self, $path) = @_;
-	$self->{-git_path}->{$path} ||= do {
+	$self->{-git_path}->{$path} //= do {
 		local $/ = "\n";
 		chomp(my $str = $self->qx(qw(rev-parse --git-path), $path));
 
diff --git a/lib/PublicInbox/IMAP.pm b/lib/PublicInbox/IMAP.pm
index 4a7ff2f4..58a0a9e3 100644
--- a/lib/PublicInbox/IMAP.pm
+++ b/lib/PublicInbox/IMAP.pm
@@ -871,12 +871,12 @@ sub eml_index_offs_i { # PublicInbox::Eml::each_part callback
 # prepares an index for BODY[$SECTION_IDX] fetches
 sub eml_body_idx ($$) {
 	my ($eml, $section_idx) = @_;
-	my $idx = $eml->{imap_all_parts} //= do {
+	my $idx = $eml->{imap_all_parts} // do {
 		my $all = {};
 		$eml->each_part(\&eml_index_offs_i, $all, 0, 1);
 		# top-level of multipart, BODY[0] not allowed (nz-number)
 		delete $all->{0};
-		$all;
+		$eml->{imap_all_parts} = $all;
 	};
 	$idx->{$section_idx};
 }
diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm
index b7b71268..1579d500 100644
--- a/lib/PublicInbox/Inbox.pm
+++ b/lib/PublicInbox/Inbox.pm
@@ -48,7 +48,7 @@ sub _cleanup_later ($) {
 sub _set_limiter ($$$) {
 	my ($self, $pi_cfg, $pfx) = @_;
 	my $lkey = "-${pfx}_limiter";
-	$self->{$lkey} ||= do {
+	$self->{$lkey} //= do {
 		# full key is: publicinbox.$NAME.httpbackendmax
 		my $mkey = $pfx.'max';
 		my $val = $self->{$mkey} or return;
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index 78b49a3b..3e1706a0 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -130,9 +130,10 @@ sub url_folder_cache {
 
 sub ale {
 	my ($self) = @_;
-	$self->{ale} //= do {
+	$self->{ale} // do {
 		require PublicInbox::LeiALE;
-		$self->_lei_cfg(1)->{ale} //= PublicInbox::LeiALE->new($self);
+		my $cfg = $self->_lei_cfg(1);
+		$self->{ale} = $cfg->{ale} //= PublicInbox::LeiALE->new($self);
 	};
 }
 
@@ -1159,10 +1160,10 @@ sub event_step {
 sub event_step_init {
 	my ($self) = @_;
 	my $sock = $self->{sock} or return;
-	$self->{-event_init_done} //= do { # persist til $ops done
+	$self->{-event_init_done} // do { # persist til $ops done
 		$sock->blocking(0);
 		$self->SUPER::new($sock, EPOLLIN);
-		$sock;
+		$self->{-event_init_done} = $sock;
 	};
 }
 
diff --git a/lib/PublicInbox/SharedKV.pm b/lib/PublicInbox/SharedKV.pm
index 27407f83..4297efed 100644
--- a/lib/PublicInbox/SharedKV.pm
+++ b/lib/PublicInbox/SharedKV.pm
@@ -15,7 +15,7 @@ use File::Path qw(rmtree make_path);
 
 sub dbh {
 	my ($self, $lock) = @_;
-	$self->{dbh} //= do {
+	$self->{dbh} // do {
 		my $f = $self->{filename};
 		$lock //= $self->lock_for_scope_fast;
 		my $dbh = DBI->connect("dbi:SQLite:dbname=$f", '', '', {
@@ -36,7 +36,7 @@ CREATE TABLE IF NOT EXISTS kv (
 	UNIQUE (k)
 )
 
-		$dbh;
+		$self->{dbh} = $dbh;
 	}
 }
 
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index b4db0582..a282784a 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -468,7 +468,7 @@ sub serve_mbox_range {
 
 sub news_www {
 	my ($self) = @_;
-	$self->{news_www} ||= do {
+	$self->{news_www} //= do {
 		require PublicInbox::NewsWWW;
 		PublicInbox::NewsWWW->new($self->{pi_cfg});
 	}
@@ -476,7 +476,7 @@ sub news_www {
 
 sub cgit {
 	my ($self) = @_;
-	$self->{cgit} ||= do {
+	$self->{cgit} //= do {
 		my $pi_cfg = $self->{pi_cfg};
 
 		if (defined($pi_cfg->{'publicinbox.cgitrc'})) {
diff --git a/lib/PublicInbox/WwwStatic.pm b/lib/PublicInbox/WwwStatic.pm
index b3476ab8..eeb5e565 100644
--- a/lib/PublicInbox/WwwStatic.pm
+++ b/lib/PublicInbox/WwwStatic.pm
@@ -218,7 +218,7 @@ my %path_re_cache;
 sub path_info_raw ($) {
 	my ($env) = @_;
 	my $sn = $env->{SCRIPT_NAME};
-	my $re = $path_re_cache{$sn} ||= do {
+	my $re = $path_re_cache{$sn} //= do {
 		$sn = '/'.$sn unless index($sn, '/') == 0;
 		$sn =~ s!/\z!!;
 		qr!\A(?:https?://[^/]+)?\Q$sn\E(/[^\?\#]+)!;
diff --git a/lib/PublicInbox/WwwStream.pm b/lib/PublicInbox/WwwStream.pm
index 6d7c447f..aee78170 100644
--- a/lib/PublicInbox/WwwStream.pm
+++ b/lib/PublicInbox/WwwStream.pm
@@ -170,9 +170,9 @@ sub html_oneshot ($$;$) {
 		'Content-Length' => undef ];
 	bless $ctx, __PACKAGE__;
 	$ctx->{gz} = PublicInbox::GzipFilter::gz_or_noop($res_hdr, $ctx->{env});
-	$ctx->{base_url} //= do {
+	$ctx->{base_url} // do {
 		$ctx->zmore(html_top($ctx));
-		base_url($ctx);
+		$ctx->{base_url} = base_url($ctx);
 	};
 	$ctx->zmore($$sref) if $sref;
 	my $bdy = $ctx->zflush(_html_end($ctx));

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-11-01 19:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-01 19:06 [PATCH 0/2] avoid problematic conditional hash assignments Eric Wong
2021-11-01 19:06 ` [PATCH 1/2] idx_stack: avoid conditional hash assignment weirdness Eric Wong
2021-11-01 19:06 ` [PATCH 2/2] treewide: kill problematic "$h->{k} //= do {" assignments Eric Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).