From: Eric Wong Date: Mon, 17 Nov 2025 21:53:49 +0000 (+0000) Subject: treewide: use closedir after readdir for error checking X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c856f9c5dd4562ac6f28fbcf1f4dc19b5cedcf98;p=thirdparty%2Fpublic-inbox.git treewide: use closedir after readdir for error checking Hopefully... Neither Perl's readdir nor readdir(3posix) documents EIO as a possible error, nor does Linux getdents(3). So relying on readdir to detect errors via autodie(3perl) is not supported nor possible, it seems. Perl's closedir is supported by autodie and uses closedir(3posix), which doesn't document EIO, either. However, closedir(3posix) will use close(3posix). Thus in theory, EIO can still be detected if a previous readdir failed. --- diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm index 6ff167834..033f15a88 100644 --- a/lib/PublicInbox/Config.pm +++ b/lib/PublicInbox/Config.pm @@ -17,6 +17,7 @@ use PublicInbox::Git qw(git_exe); use PublicInbox::Spawn qw(popen_rd run_qx); our $LD_PRELOAD = $ENV{LD_PRELOAD}; # only valid at startup our $DEDUPE; # set to {} to dedupe or clear cache +use autodie qw(closedir); sub _array ($) { ref($_[0]) eq 'ARRAY' ? $_[0] : [ $_[0] ] } @@ -276,6 +277,7 @@ sub scan_path_coderepo { my $dir = "$path/$dn"; scan_path_coderepo($self, $base, $dir) if -d $dir; } + closedir $dh; } sub scan_tree_coderepo ($$) { diff --git a/lib/PublicInbox/ExtSearchIdx.pm b/lib/PublicInbox/ExtSearchIdx.pm index f83b64652..24802d3bf 100644 --- a/lib/PublicInbox/ExtSearchIdx.pm +++ b/lib/PublicInbox/ExtSearchIdx.pm @@ -21,7 +21,7 @@ package PublicInbox::ExtSearchIdx; use strict; use v5.10.1; use parent qw(PublicInbox::ExtSearch PublicInbox::Umask PublicInbox::Lock); -use autodie qw(mkdir); +use autodie qw(closedir mkdir); use Carp qw(croak carp); use Scalar::Util qw(blessed); use Sys::Hostname qw(hostname); @@ -1279,6 +1279,7 @@ sub idx_init { # similar to V2Writable unlink($f) if -l $f && !-e $f; } } + closedir $dh; } elsif ($!{ENOENT}) { mkdir $pd; } else { diff --git a/lib/PublicInbox/FakeInotify.pm b/lib/PublicInbox/FakeInotify.pm index 8be071356..b670cca5d 100644 --- a/lib/PublicInbox/FakeInotify.pm +++ b/lib/PublicInbox/FakeInotify.pm @@ -114,7 +114,7 @@ sub read { next; } if (-d _ && $new_st[10] > ($old_ctime - dir_adj($old_ctime))) { - opendir(my $fh, $path) or do { + opendir(my $dh, $path) or do { if ($! == ENOENT || $! == ENOTDIR) { push @$ret, gone($self, $ident, $path); } else { @@ -122,13 +122,14 @@ sub read { } next; }; - @new_st = stat($fh) or die "fstat($path): $!"; + @new_st = stat($dh) or die "fstat($path): $!"; if ("$old_dev $old_ino" ne "@new_st[0,1]") { push @$ret, gone($self, $ident, $path); next; } $w->[2] = $new_st[10]; - on_dir_change($self, $ret, $fh, $path, $dir_delete); + on_dir_change($self, $ret, $dh, $path, $dir_delete); + closedir $dh; } elsif ($new_st[10] > $old_ctime) { # regular files, etc $w->[2] = $new_st[10]; push @$ret, bless(\$path, diff --git a/lib/PublicInbox/Inbox.pm b/lib/PublicInbox/Inbox.pm index 709765328..e1a22a48b 100644 --- a/lib/PublicInbox/Inbox.pm +++ b/lib/PublicInbox/Inbox.pm @@ -5,6 +5,7 @@ package PublicInbox::Inbox; use strict; use v5.10.1; +use autodie qw(closedir); use PublicInbox::Git; use PublicInbox::MID qw(mid2path); use PublicInbox::Eml; @@ -117,6 +118,7 @@ sub max_git_epoch { my $max = max(map { substr($_, 0, -4) + 0; # drop ".git" suffix } grep(/\A[0-9]+\.git\z/, readdir($dh))) // return; + closedir $dh; $cur = $self->{-max_git_epoch} = $max; } } diff --git a/lib/PublicInbox/LeiSavedSearch.pm b/lib/PublicInbox/LeiSavedSearch.pm index b78e460f7..3cc36e8b4 100644 --- a/lib/PublicInbox/LeiSavedSearch.pm +++ b/lib/PublicInbox/LeiSavedSearch.pm @@ -4,7 +4,7 @@ # pretends to be like LeiDedupe and also PublicInbox::Inbox package PublicInbox::LeiSavedSearch; use v5.12; -use autodie qw(opendir); +use autodie qw(closedir opendir); use parent qw(PublicInbox::Lock); use PublicInbox::Git qw(git_exe); use PublicInbox::OverIdx; @@ -61,6 +61,7 @@ sub lss_dir_for ($$;$) { my ($c, $o, @st); opendir(my $dh, $lss_dir); my @d = sort(grep(!/\A\.\.?\z/, readdir($dh))); + closedir $dh; my $re = qr/\A\Q$pfx\E-\./; for my $d (grep(/$re/, @d), grep(!/$re/, @d)) { my $f = "$lss_dir/$d/lei.saved-search"; @@ -89,6 +90,7 @@ sub list { my $p = "$lss_dir/$d/lei.saved-search"; say $fh "\tpath = ", cquote_val($p) if -f $p; } + closedir $dh; $fh->flush or die "$fh->flush: $!"; my $cfg = $lei->cfg_dump($fh->filename); my $out = $cfg ? $cfg->get_all('lei.q.output') : []; diff --git a/lib/PublicInbox/MdirReader.pm b/lib/PublicInbox/MdirReader.pm index 2981b058c..047508efe 100644 --- a/lib/PublicInbox/MdirReader.pm +++ b/lib/PublicInbox/MdirReader.pm @@ -7,6 +7,7 @@ package PublicInbox::MdirReader; use strict; use v5.10.1; +use autodie qw(closedir); use PublicInbox::InboxWritable qw(eml_from_path); use PublicInbox::SHA qw(sha256_hex); @@ -47,6 +48,7 @@ sub maildir_each_file { next if index($fl, 'T') >= 0; # no Trashed messages $cb->($pfx.$bn, $fl, @arg); } + closedir $dh; } } @@ -71,6 +73,7 @@ sub maildir_each_eml { my $eml = eml_from_path($f) or next; $cb->($f, [], $eml, @arg); } + closedir $dh; } $pfx = $dir . 'cur/'; opendir my $dh, $pfx or return; @@ -83,6 +86,7 @@ sub maildir_each_eml { my @kw = sort(map { $c2kw{$_} // () } split(//, $fl)); $cb->($f, \@kw, $eml, @arg); } + closedir $dh; } sub new { bless {}, __PACKAGE__ } diff --git a/lib/PublicInbox/WWW.pm b/lib/PublicInbox/WWW.pm index 31099f09c..8362dae38 100644 --- a/lib/PublicInbox/WWW.pm +++ b/lib/PublicInbox/WWW.pm @@ -13,7 +13,7 @@ package PublicInbox::WWW; use strict; use v5.10.1; -use autodie qw(chdir opendir); +use autodie qw(chdir closedir opendir); use PublicInbox::Config; use PublicInbox::Git; use PublicInbox::Hval; @@ -704,6 +704,7 @@ sub stylesheets_prepare ($$) { }; chdir $dh; my @css = grep /\.css\z/i, readdir $dh; + closedir $dh; for my $fn (@css) { my ($key) = ($fn =~ m!([^/]+?)(?:\.css)?\z!i); next if $css_map->{$key}; diff --git a/lib/PublicInbox/Watch.pm b/lib/PublicInbox/Watch.pm index 230e604a2..90473e642 100644 --- a/lib/PublicInbox/Watch.pm +++ b/lib/PublicInbox/Watch.pm @@ -597,6 +597,7 @@ sub fs_scan_step { unshift @{$self->{scan_q}}, [ $dir, $dh ]; last; } else { + closedir $dh; warn "# done scanning $dir\n"; } } diff --git a/lib/PublicInbox/WwwStatic.pm b/lib/PublicInbox/WwwStatic.pm index 69b658ce0..cb0b15400 100644 --- a/lib/PublicInbox/WwwStatic.pm +++ b/lib/PublicInbox/WwwStatic.pm @@ -11,7 +11,7 @@ package PublicInbox::WwwStatic; use strict; use v5.10.1; use parent qw(Exporter Plack::Component); -use autodie qw(sysseek); +use autodie qw(closedir sysseek); use Fcntl qw(SEEK_SET O_RDONLY O_NONBLOCK); use HTTP::Date qw(time2str); use HTTP::Status qw(status_message); @@ -276,7 +276,7 @@ sub dir_response ($$$) { return r(500); }; my @entries = grep(!/\A\./, readdir($dh)); - $dh = undef; + closedir $dh; my (%dirs, %other, %want_gz); my $path_info = $env->{PATH_INFO}; push @entries, '..' if $path_info ne '/'; diff --git a/lib/PublicInbox/Xapcmd.pm b/lib/PublicInbox/Xapcmd.pm index 6d40e32ad..1f5826990 100644 --- a/lib/PublicInbox/Xapcmd.pm +++ b/lib/PublicInbox/Xapcmd.pm @@ -276,9 +276,9 @@ sub prepare_run { if ($opt->{cow}) { # make existing $DIR/{xap,ei}* CoW my $dfd = dup(fileno($dh)) // die "dup: $!"; open my $fh, '<&='.$dfd; - closedir $dh; PublicInbox::Syscall::yesdatacow_fh($fh); } + closedir $dh; die "No Xapian shards found in $old\n" unless @old_shards; @old_shards = sort { $a <=> $b } @old_shards; my ($src, $max_shard);