* [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