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 6ff1678347a66fc7b3101dfa06e28000bbd3d46a..033f15a88971a3349aee5d7b6c7dd6f01ebc6e45 100644 (file)
--- 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 f83b64652aafffa70cc0d660a61e43ba8614dd4e..24802d3bfe39c3d51e72bbc4af56025ef696367a 100644 (file)
--- 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 8be0713563a952ce6b3c7be2a99806a1d2ef2168..b670cca5d5eecaa72cd889e7146e7d5caf6449ca 100644 (file)
--- 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 7097653283e7d57e326e6fa6aeb9b9584702283c..e1a22a48b85fac9f72ac30ee70302c5b6d4ff70d 100644 (file)
--- 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 b78e460f7fb953effd6c2753ea5b95223b537068..3cc36e8b4bf046b233b4427d6e06aea0c4f197e3 100644 (file)
--- 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 2981b058c1cce43121d5a223b3f148e399618ac9..047508efe17572e0fa99579c1a360fc47c495b62 100644 (file)
--- 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 31099f09c153dd80f8a89930c28b322a2a3c442e..8362dae3823975a0daa39daf82e852bdfc3b026b 100644 (file)
--- 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 230e604a2200aa6b39c5ea3d860ba048a0e4ec38..90473e642eb2c92ecf4b09ef3cad6359cc85184b 100644 (file)
--- 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 69b658ce0d2cbf0b587517ce6f653e97fe077521..cb0b15400550b706163b5accdbee4e303c794058 100644 (file)
--- 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 6d40e32addc5e19210e745376ba965c13cc6afc4..1f5826990ac0e34a2993d1c4e9b4b36aafd9a579 100644 (file)
--- 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);