From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH 07/30] use read_all in more places to improve safety
Date: Tue, 17 Oct 2023 23:37:52 +0000 [thread overview]
Message-ID: <20231017233815.1637932-8-e@80x24.org> (raw)
In-Reply-To: <20231017233815.1637932-1-e@80x24.org>
`readline' ops may not detect errors on partial reads.
This saves us some code to reduce cognitive overhead for
readers. We'll also support reusing a destination buffers so it
can work more nicely with existing code.
---
lib/PublicInbox/Gcf2.pm | 7 +++----
lib/PublicInbox/Git.pm | 9 +++++----
lib/PublicInbox/InboxWritable.pm | 6 +++---
lib/PublicInbox/LeiALE.pm | 11 +++--------
lib/PublicInbox/LeiBlob.pm | 6 +++---
lib/PublicInbox/LeiConfig.pm | 2 +-
lib/PublicInbox/LeiMailSync.pm | 5 ++---
lib/PublicInbox/LeiSucks.pm | 5 +++--
lib/PublicInbox/MultiGit.pm | 3 ++-
lib/PublicInbox/ViewVCS.pm | 12 +++++-------
lib/PublicInbox/WWW.pm | 4 ++--
lib/PublicInbox/XapHelper.pm | 3 ++-
lib/PublicInbox/XapHelperCxx.pm | 6 +++---
script/public-inbox-edit | 4 ++--
script/public-inbox-init | 3 ++-
15 files changed, 41 insertions(+), 45 deletions(-)
diff --git a/lib/PublicInbox/Gcf2.pm b/lib/PublicInbox/Gcf2.pm
index 37262e28..4f163cde 100644
--- a/lib/PublicInbox/Gcf2.pm
+++ b/lib/PublicInbox/Gcf2.pm
@@ -9,7 +9,7 @@ use PublicInbox::Spawn qw(which popen_rd); # may set PERL_INLINE_DIRECTORY
use Fcntl qw(SEEK_SET);
use Time::HiRes qw(clock_gettime CLOCK_MONOTONIC);
use IO::Handle; # autoflush
-use PublicInbox::Git;
+use PublicInbox::Git qw(read_all);
use PublicInbox::Lock;
BEGIN {
@@ -43,12 +43,11 @@ BEGIN {
# build them.
my $f = "$dir/gcf2_libgit2.h";
open my $src, '<', $f;
- local $/;
- $c_src = <$src> // die "read $f: $!";
+ $c_src = read_all($src);
}
unless ($c_src) {
seek($err, 0, SEEK_SET);
- $err = do { local $/; <$err> };
+ $err = read_all($err);
die "E: libgit2 not installed: $err\n";
}
# append pkg-config results to the source to ensure Inline::C
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 35bd10ef..a460d155 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -550,11 +550,12 @@ sub modified ($;$) {
# read_all/try_cat can probably be moved somewhere else...
-sub read_all ($;$) {
- my ($fh, $len) = @_;
- my $r = read($fh, my $buf, $len //= -s $fh);
+sub read_all ($;$$) {
+ my ($fh, $len, $bref) = @_;
+ $bref //= \(my $buf);
+ my $r = read($fh, $$bref, $len //= -s $fh);
croak("$fh read ($r != $len)") if $len != $r;
- $buf;
+ $$bref;
}
sub try_cat {
diff --git a/lib/PublicInbox/InboxWritable.pm b/lib/PublicInbox/InboxWritable.pm
index 65952aa2..6af72e71 100644
--- a/lib/PublicInbox/InboxWritable.pm
+++ b/lib/PublicInbox/InboxWritable.pm
@@ -7,6 +7,7 @@ use strict;
use v5.10.1;
use parent qw(PublicInbox::Inbox PublicInbox::Umask Exporter);
use PublicInbox::Import;
+use PublicInbox::Git qw(read_all);
use PublicInbox::Filter::Base qw(REJECT);
use Errno qw(ENOENT);
our @EXPORT_OK = qw(eml_from_path);
@@ -114,9 +115,8 @@ sub filter {
sub eml_from_path ($) {
my ($path) = @_;
if (sysopen(my $fh, $path, O_RDONLY|O_NONBLOCK)) {
- return unless -f $fh; # no FIFOs or directories
- my $str = do { local $/; <$fh> } or return;
- PublicInbox::Eml->new(\$str);
+ return unless -f $fh && -s _; # no FIFOs or directories
+ PublicInbox::Eml->new(\(my $str = read_all($fh, -s _)));
} else { # ENOENT is common with Maildir
warn "failed to open $path: $!\n" if $! != ENOENT;
undef;
diff --git a/lib/PublicInbox/LeiALE.pm b/lib/PublicInbox/LeiALE.pm
index cc9a2095..b198af1c 100644
--- a/lib/PublicInbox/LeiALE.pm
+++ b/lib/PublicInbox/LeiALE.pm
@@ -9,7 +9,7 @@ package PublicInbox::LeiALE;
use strict;
use v5.10.1;
use parent qw(PublicInbox::LeiSearch PublicInbox::Lock);
-use PublicInbox::Git;
+use PublicInbox::Git qw(read_all);
use PublicInbox::Import;
use PublicInbox::LeiXSearch;
use Fcntl qw(SEEK_SET);
@@ -54,11 +54,7 @@ sub refresh_externals {
$self->git->cleanup;
my $lk = $self->lock_for_scope;
my $cur_lxs = ref($lxs)->new;
- my $orig = do {
- local $/;
- readline($self->{lockfh}) //
- die "readline($self->{lock_path}): $!";
- };
+ my $orig = read_all($self->{lockfh});
my $new = '';
my $old = '';
my $gone = 0;
@@ -91,8 +87,7 @@ sub refresh_externals {
$new = $old = '';
my $f = $self->git->{git_dir}.'/objects/info/alternates';
if (open my $fh, '<', $f) {
- local $/;
- $old = <$fh> // die "readline($f): $!";
+ read_all($fh, -s $fh, \$old);
}
for my $x (@ibxish) {
$new .= $lei->canonpath_harder($x->git->{git_dir})."/objects\n";
diff --git a/lib/PublicInbox/LeiBlob.pm b/lib/PublicInbox/LeiBlob.pm
index d069d4a8..40f64bd9 100644
--- a/lib/PublicInbox/LeiBlob.pm
+++ b/lib/PublicInbox/LeiBlob.pm
@@ -10,6 +10,7 @@ use parent qw(PublicInbox::IPC);
use PublicInbox::Spawn qw(run_wait popen_rd which);
use PublicInbox::DS;
use PublicInbox::Eml;
+use PublicInbox::Git qw(read_all);
sub get_git_dir ($$) {
my ($lei, $d) = @_;
@@ -137,9 +138,8 @@ sub lei_blob {
extract_attach($lei, $blob, $bref) :
$lei->out($$bref);
if ($opt->{mail}) {
- my $eh = $rdr->{2};
- seek($eh, 0, 0);
- return $lei->child_error($cerr, do { local $/; <$eh> });
+ seek($rdr->{2}, 0, 0);
+ return $lei->child_error($cerr, read_all($rdr->{2}));
} # else: fall through to solver below
}
diff --git a/lib/PublicInbox/LeiConfig.pm b/lib/PublicInbox/LeiConfig.pm
index b3495487..c47708d8 100644
--- a/lib/PublicInbox/LeiConfig.pm
+++ b/lib/PublicInbox/LeiConfig.pm
@@ -28,7 +28,7 @@ sub cfg_edit_done { # PktOp lei->do_env cb
$lei->cfg_dump($self->{-f});
} or do {
seek($fh, 0, SEEK_SET);
- return cfg_do_edit($self, do { local $/; <$fh> });
+ return cfg_do_edit($self, read_all($fh));
};
$self->cfg_verify($cfg) if $self->can('cfg_verify');
}
diff --git a/lib/PublicInbox/LeiMailSync.pm b/lib/PublicInbox/LeiMailSync.pm
index 415459d5..74ef1362 100644
--- a/lib/PublicInbox/LeiMailSync.pm
+++ b/lib/PublicInbox/LeiMailSync.pm
@@ -10,7 +10,7 @@ use PublicInbox::Compat qw(uniqstr);
use DBI qw(:sql_types); # SQL_BLOB
use PublicInbox::ContentHash qw(git_sha);
use Carp ();
-use PublicInbox::Git qw(%HEXLEN2SHA);
+use PublicInbox::Git qw(%HEXLEN2SHA read_all);
sub dbh_new {
my ($self) = @_;
@@ -456,8 +456,7 @@ WHERE b.oidbin = ?
open my $fh, '<', $f or next;
# some (buggy) Maildir writers are non-atomic:
next unless -s $fh;
- local $/;
- my $raw = <$fh>;
+ my $raw = read_all($fh, -s _);
if ($vrfy) {
my $sha = $HEXLEN2SHA{length($oidhex)};
my $got = git_sha($sha, \$raw)->hexdigest;
diff --git a/lib/PublicInbox/LeiSucks.pm b/lib/PublicInbox/LeiSucks.pm
index 35d0a8de..82aea8d4 100644
--- a/lib/PublicInbox/LeiSucks.pm
+++ b/lib/PublicInbox/LeiSucks.pm
@@ -12,6 +12,7 @@ use Config;
use POSIX ();
use PublicInbox::Config;
use PublicInbox::IPC;
+use PublicInbox::Git qw(read_all);
sub lei_sucks {
my ($lei, @argv) = @_;
@@ -58,8 +59,8 @@ sub lei_sucks {
for my $m (grep(m{^PublicInbox/}, sort keys %INC)) {
my $f = $INC{$m} // next; # lazy require failed (missing dep)
open my $fh, '<', $f or do { warn "open($f): $!"; next };
- my $hex = sha1_hex('blob '.(-s $fh)."\0".
- (do { local $/; <$fh> } // die("read: $!")));
+ my $size = -s $fh;
+ my $hex = sha1_hex("blob $size\0".read_all($fh, $size));
push @out, ' '.$hex.' '.$m."\n";
}
push @out, <<'EOM';
diff --git a/lib/PublicInbox/MultiGit.pm b/lib/PublicInbox/MultiGit.pm
index 74a9e1df..35bd0251 100644
--- a/lib/PublicInbox/MultiGit.pm
+++ b/lib/PublicInbox/MultiGit.pm
@@ -9,6 +9,7 @@ use PublicInbox::Spawn qw(run_die popen_rd);
use PublicInbox::Import;
use File::Temp 0.19;
use List::Util qw(max);
+use PublicInbox::Git qw(read_all);
sub new {
my ($cls, $topdir, $all, $epfx) = @_;
@@ -31,7 +32,7 @@ sub read_alternates {
qr!\A\Q../../$self->{epfx}\E/([0-9]+)\.git/objects\z! :
undef;
$$moderef = (stat($fh))[2] & 07777;
- for my $rel (split(/^/m, do { local $/; <$fh> })) {
+ for my $rel (split(/^/m, read_all($fh, -s _))) {
chomp(my $dir = $rel);
my $score;
if (defined($is_edir) && $dir =~ $is_edir) {
diff --git a/lib/PublicInbox/ViewVCS.pm b/lib/PublicInbox/ViewVCS.pm
index 8a90c2d6..86c46e69 100644
--- a/lib/PublicInbox/ViewVCS.pm
+++ b/lib/PublicInbox/ViewVCS.pm
@@ -17,6 +17,7 @@ use strict;
use v5.10.1;
use File::Temp 0.19 (); # newdir
use PublicInbox::SolverGit;
+use PublicInbox::Git qw(read_all);
use PublicInbox::GitAsyncCat;
use PublicInbox::WwwStream qw(html_oneshot);
use PublicInbox::Linkify;
@@ -61,12 +62,9 @@ sub dbg_log ($) {
warn "seek(log): $!";
return '<pre>debug log seek error</pre>';
}
- $log = do { local $/; <$log> } // do {
- if (!eof($log)) {
- warn "readline(log): $!";
- return '<pre>debug log read error</pre>';
- }
- '';
+ $log = eval { read_all($log) } // do {
+ warn "read(log): $@";
+ return '<pre>debug log read error</pre>';
};
return '' if $log eq '';
$ctx->{-linkify} //= PublicInbox::Linkify->new;
@@ -251,7 +249,7 @@ EOM
if (-s $fh > $MAX_SIZE) {
print $zfh "---\n patch is too large to show\n";
} else { # prepare flush_diff:
- read($fh, $x, -s _);
+ read_all($fh, -s _, \$x);
utf8_maybe($x);
$ctx->{-apfx} = $ctx->{-spfx} = $upfx;
$x =~ s/\r?\n/\n/gs;
diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm
index 9919f975..86fd7e22 100644
--- a/lib/PublicInbox/WWW.pm
+++ b/lib/PublicInbox/WWW.pm
@@ -587,9 +587,9 @@ sub stylesheets_prepare ($$) {
next;
};
my $ctime = 0;
- my $local = do { local $/; <$fh> };
+ my $local = read_all($fh, -s $fh);
if ($local =~ /\S/) {
- $ctime = sprintf('%x',(stat($fh))[10]);
+ $ctime = sprintf('%x',(stat(_))[10]);
$local = $mini->($local);
}
diff --git a/lib/PublicInbox/XapHelper.pm b/lib/PublicInbox/XapHelper.pm
index ae907766..ca993ca8 100644
--- a/lib/PublicInbox/XapHelper.pm
+++ b/lib/PublicInbox/XapHelper.pm
@@ -10,6 +10,7 @@ $GLP->configure(qw(require_order bundling no_ignore_case no_auto_abbrev));
use PublicInbox::Search qw(xap_terms);
use PublicInbox::CodeSearch;
use PublicInbox::IPC;
+use PublicInbox::Git qw(read_all);
use Socket qw(SOL_SOCKET SO_TYPE SOCK_SEQPACKET AF_UNIX);
use PublicInbox::DS qw(awaitpid);
use POSIX qw(:signal_h);
@@ -123,7 +124,7 @@ sub cmd_dump_roots {
$req->{A} or return warn('dump_roots requires -A PREFIX');
open my $fh, '<', $root2id_file or die "open($root2id_file): $!";
my $root2id; # record format: $OIDHEX "\0" uint32_t
- my @x = split(/\0/, do { local $/; <$fh> } // die "readline: $!");
+ my @x = split(/\0/, read_all($fh));
while (@x) {
my $oidhex = shift @x;
$root2id->{$oidhex} = shift @x;
diff --git a/lib/PublicInbox/XapHelperCxx.pm b/lib/PublicInbox/XapHelperCxx.pm
index dbb0a915..83015379 100644
--- a/lib/PublicInbox/XapHelperCxx.pm
+++ b/lib/PublicInbox/XapHelperCxx.pm
@@ -8,6 +8,7 @@
package PublicInbox::XapHelperCxx;
use v5.12;
use PublicInbox::Spawn qw(popen_rd which);
+use PublicInbox::Git qw(read_all);
use PublicInbox::Search;
use Fcntl qw(SEEK_SET);
use Config;
@@ -34,7 +35,7 @@ sub xap_cfg (@) {
chomp(my $ret = do { local $/; <$rd> });
return $ret if close($rd);
seek($err, 0, SEEK_SET) or die "seek: $!";
- $err = do { local $/; <$err> };
+ $err = read_all($err);
die <<EOM;
@$cmd failed: Xapian development files missing? (\$?=$?)
$err
@@ -70,8 +71,7 @@ sub build () {
for (@srcs) {
say $fh qq(# line 1 "$_");
open my $rfh, '<', $_;
- local $/;
- print $fh readline($rfh);
+ print $fh read_all($rfh);
}
print $fh PublicInbox::Search::generate_cxx();
print $fh PublicInbox::CodeSearch::generate_cxx();
diff --git a/script/public-inbox-edit b/script/public-inbox-edit
index 1fb6f32b..77028817 100755
--- a/script/public-inbox-edit
+++ b/script/public-inbox-edit
@@ -15,6 +15,7 @@ PublicInbox::Admin::check_require('-index');
use PublicInbox::Eml;
use PublicInbox::InboxWritable qw(eml_from_path);
use PublicInbox::Import;
+use PublicInbox::Git qw(read_all);
my $help = <<'EOF';
usage: public-inbox-edit -m MESSAGE-ID [--all] [INBOX_DIRS]
@@ -184,8 +185,7 @@ retry_edit:
# rename/relink $edit_fn
open my $new_fh, '<', $edit_fn or
die "can't read edited file ($edit_fn): $!\n";
- defined(my $new_raw = do { local $/; <$new_fh> }) or die
- "read $edit_fn: $!\n";
+ my $new_raw = read_all($new_fh);
if (!$opt->{raw}) {
PublicInbox::Eml::strip_from($new_raw);
diff --git a/script/public-inbox-init b/script/public-inbox-init
index b3a16cfb..33bee310 100755
--- a/script/public-inbox-init
+++ b/script/public-inbox-init
@@ -125,13 +125,14 @@ my $auto_unlink = PublicInbox::OnDestroy->new($$, sub { unlink $lockfile });
my $perm = 0644 & ~umask;
my %seen;
if (-e $pi_config) {
+ require PublicInbox::Git;
open(my $oh, '<', $pi_config) or die "unable to read $pi_config: $!\n";
my @st = stat($oh);
$perm = $st[2];
defined $perm or die "(f)stat failed on $pi_config: $!\n";
chmod($perm & 07777, $fh) or
die "(f)chmod failed on future $pi_config: $!\n";
- defined(my $old = do { local $/; <$oh> }) or die "read $pi_config: $!\n";
+ my $old = PublicInbox::Git::read_all($oh);
print $fh $old or die "failed to write: $!\n";
close $oh or die "failed to close $pi_config: $!\n";
next prev parent reply other threads:[~2023-10-17 23:38 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-17 23:37 [PATCH 00/30] autodie-ification and code simplifications Eric Wong
2023-10-17 23:37 ` [PATCH 01/30] lei_mirror: start converting to autodie Eric Wong
2023-10-17 23:37 ` [PATCH 02/30] lei_mirror: autodie most `close' calls Eric Wong
2023-10-17 23:37 ` [PATCH 03/30] lei_mirror: use autodie for most `open' calls Eric Wong
2023-10-17 23:37 ` [PATCH 04/30] git: introduce read_all function Eric Wong
2023-10-17 23:37 ` [PATCH 05/30] import: use read_all to detect short reads Eric Wong
2023-10-17 23:37 ` [PATCH 06/30] lei_mirror: use read_all Eric Wong
2023-10-17 23:37 ` Eric Wong [this message]
2023-10-17 23:37 ` [PATCH 08/30] xap_helper*: use autodie in more places Eric Wong
2023-10-17 23:37 ` [PATCH 09/30] xap_helper: die more easily in both implementations Eric Wong
2023-10-17 23:37 ` [PATCH 10/30] xap_helper: simplify SIGTERM exit checks Eric Wong
2023-10-17 23:37 ` [PATCH 11/30] xap_helper: autodie for getsockopt Eric Wong
2023-10-17 23:37 ` [PATCH 12/30] xap_client: autodie for pipe and socketpair Eric Wong
2023-10-17 23:37 ` [PATCH 13/30] xt/git-http-backend: remove Net::HTTP usage Eric Wong
2023-10-17 23:37 ` [PATCH 14/30] ds: introduce and use do_fork helper Eric Wong
2023-10-17 23:38 ` [PATCH 15/30] ds: get rid of SetLoopTimeout Eric Wong
2023-10-17 23:38 ` [PATCH 16/30] cindex: drop some unused functions Eric Wong
2023-10-17 23:38 ` [PATCH 17/30] syscall: common $F_SETPIPE_SZ definition Eric Wong
2023-10-17 23:38 ` [PATCH 18/30] t/lei-up: additional diagnostics for match failures Eric Wong
2023-10-17 23:38 ` [PATCH 19/30] test_common: use autodie and read_all where possible Eric Wong
2023-10-17 23:38 ` [PATCH 20/30] test_common: only hide TCP port in messages Eric Wong
2023-10-17 23:38 ` [PATCH 21/30] test_common: use $cwdfh for every run_script command Eric Wong
2023-10-17 23:38 ` [PATCH 22/30] init: drop extraneous `+' Eric Wong
2023-10-17 23:38 ` [PATCH 23/30] init: use autodie to reduce distractions Eric Wong
2023-10-17 23:38 ` [PATCH 24/30] xt/mem-imapd-tls: remove unused/broken epoll imports Eric Wong
2023-10-17 23:38 ` [PATCH 25/30] xt/mem-imapd-tls: reduce FDs for lsof use Eric Wong
2023-10-17 23:38 ` [PATCH 26/30] lei: use autodie where appropriate Eric Wong
2023-10-17 23:38 ` [PATCH 27/30] lei_auth: update comments and use v5.12 Eric Wong
2023-10-17 23:38 ` [PATCH 28/30] lei_config: drop redundant open check Eric Wong
2023-10-17 23:38 ` [PATCH 29/30] convert: use read_all to simplify error checks Eric Wong
2023-10-17 23:38 ` [PATCH 30/30] idx_stack: use autodie + read_all Eric Wong
2023-10-19 1:14 ` [PATCH 31/30] lei: simplify startq/au_done wakeup notifications Eric Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://public-inbox.org/README
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231017233815.1637932-8-e@80x24.org \
--to=e@80x24.org \
--cc=meta@public-inbox.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).