user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* [PATCH 3/3] reduce calls to close unless error checks are needed
  @ 2016-02-28 11:28  7% ` Eric Wong
  0 siblings, 0 replies; 1+ results
From: Eric Wong @ 2016-02-28 11:28 UTC (permalink / raw)
  To: meta

We can rely on timely auto-destruction based on reference
counting; reducing the chance of redundant close(2) calls
which may hit the wront FD.

We do care about certain close calls (e.g. writing to a buffered
IO handle) if we require error-checking for write-integrity.  In
other cases, let things go out-of-scope so it can be freed
automatically after use.
---
 lib/PublicInbox/Config.pm      |  1 -
 lib/PublicInbox/Feed.pm        |  2 --
 lib/PublicInbox/Git.pm         | 10 ++--------
 lib/PublicInbox/ProcessPipe.pm |  5 ++---
 lib/PublicInbox/SearchIdx.pm   |  2 --
 lib/PublicInbox/Spawn.pm       |  1 -
 6 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index 844f666..b511638 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -101,7 +101,6 @@ sub try_cat {
 	if (open(my $fh, '<', $path)) {
 		local $/;
 		$rv = <$fh>;
-		close $fh;
 	}
 	$rv;
 }
diff --git a/lib/PublicInbox/Feed.pm b/lib/PublicInbox/Feed.pm
index a5828a8..54cbf23 100644
--- a/lib/PublicInbox/Feed.pm
+++ b/lib/PublicInbox/Feed.pm
@@ -255,7 +255,6 @@ sub each_recent_blob {
 		}
 	}
 
-	close $log; # we may EPIPE here
 	# for pagination
 	($first_commit, $last_commit);
 }
@@ -269,7 +268,6 @@ sub get_feedopts {
 	my %rv;
 	if (open my $fh, '<', "$ctx->{git_dir}/description") {
 		chomp($rv{description} = <$fh>);
-		close $fh;
 	} else {
 		$rv{description} = '($GIT_DIR/description missing)';
 	}
diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 57d17d3..0f92dd9 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -29,8 +29,6 @@ sub _bidi_pipe {
 	my @cmd = ('git', "--git-dir=$self->{git_dir}", qw(cat-file), $batch);
 	my $redir = { 0 => fileno($out_r), 1 => fileno($in_w) };
 	$self->{$pid} = spawn(\@cmd, undef, $redir);
-	close $out_r or fail($self, "close failed: $!");
-	close $in_w or fail($self, "close failed: $!");
 	$out_w->autoflush(1);
 	$self->{$out} = $out_w;
 	$self->{$in} = $in_r;
@@ -100,13 +98,9 @@ sub check {
 
 sub _destroy {
 	my ($self, $in, $out, $pid) = @_;
-	my $p = $self->{$pid} or return;
-	$self->{$pid} = undef;
+	my $p = delete $self->{$pid} or return;
 	foreach my $f ($in, $out) {
-		my $fh = $self->{$f};
-		defined $fh or next;
-		close $fh;
-		$self->{$f} = undef;
+		delete $self->{$f};
 	}
 	waitpid $p, 0;
 }
diff --git a/lib/PublicInbox/ProcessPipe.pm b/lib/PublicInbox/ProcessPipe.pm
index eade524..e088c10 100644
--- a/lib/PublicInbox/ProcessPipe.pm
+++ b/lib/PublicInbox/ProcessPipe.pm
@@ -15,13 +15,12 @@ sub READ { sysread($_[0]->{fh}, $_[1], $_[2], $_[3] || 0) }
 
 sub READLINE { readline($_[0]->{fh}) }
 
-sub CLOSE { close($_[0]->{fh}) }
+sub CLOSE { delete($_[0]->{fh}) }
 
 sub FILENO { fileno($_[0]->{fh}) }
 
 sub DESTROY {
-	my $fh = delete($_[0]->{fh});
-	close $fh if $fh;
+	delete($_[0]->{fh});
 	waitpid($_[0]->{pid}, 0);
 }
 
diff --git a/lib/PublicInbox/SearchIdx.pm b/lib/PublicInbox/SearchIdx.pm
index 1d0d926..415decd 100644
--- a/lib/PublicInbox/SearchIdx.pm
+++ b/lib/PublicInbox/SearchIdx.pm
@@ -348,7 +348,6 @@ sub rlog {
 			$latest = $1;
 		}
 	}
-	close $log;
 	$latest;
 }
 
@@ -446,7 +445,6 @@ sub _read_git_config_perm {
 	my @cmd = qw(config core.sharedRepository);
 	my $fh = PublicInbox::Git->new($self->{git_dir})->popen(@cmd);
 	my $perm = <$fh>;
-	close $fh;
 	chomp $perm if defined $perm;
 	$perm;
 }
diff --git a/lib/PublicInbox/Spawn.pm b/lib/PublicInbox/Spawn.pm
index 51ad269..02c5446 100644
--- a/lib/PublicInbox/Spawn.pm
+++ b/lib/PublicInbox/Spawn.pm
@@ -171,7 +171,6 @@ sub popen_rd {
 	$r->blocking($blocking) if defined $blocking;
 	$opts->{1} = fileno($w);
 	my $pid = spawn($cmd, $env, $opts);
-	close $w;
 	return ($r, $pid) if wantarray;
 	my $ret = gensym;
 	tie *$ret, 'PublicInbox::ProcessPipe', $pid, $r;
-- 
EW


^ permalink raw reply related	[relevance 7%]

Results 1-1 of 1 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2016-02-28 11:28     [PATCH 0/3] another round of minor updates Eric Wong
2016-02-28 11:28  7% ` [PATCH 3/3] reduce calls to close unless error checks are needed Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

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).