From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <e@80x24.org>
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net
X-Spam-Level: 
X-Spam-ASN:  
X-Spam-Status: No, score=-4.2 required=3.0 tests=ALL_TRUSTED,BAYES_00,
	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF shortcircuit=no
	autolearn=ham autolearn_force=no version=3.4.6
Received: from localhost (dcvr.yhbt.net [127.0.0.1])
	by dcvr.yhbt.net (Postfix) with ESMTP id D5D191F55F
	for <meta@public-inbox.org>; Fri, 29 Sep 2023 10:41:05 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=80x24.org;
	s=selector1; t=1695984065;
	bh=ohJVRnaFXwSSCoI/bwMhRAaAqZP9FzPOPCvfKq2Q5D8=;
	h=From:To:Subject:Date:From;
	b=40wPM+hXO2pl+j3lgxKhRnSjm4HSygHQi46NFj9j0kaEm3O3bJ7oJ3N24hQD8ZtrM
	 XGAzC2MrbLNdb2uJQti1R8nzC1s0VO6gUYLNJO4pfI6RRoQVWn5zXHg5AAXQM1besv
	 q8sVy6Yfp7vI7b93vkbB7wvsFqsYbz7zJ8KNW23A=
From: Eric Wong <e@80x24.org>
To: meta@public-inbox.org
Subject: [PATCH] git: fix unused code path for cat-file stderr reset
Date: Fri, 29 Sep 2023 10:41:05 +0000
Message-ID: <20230929104105.2422700-1-e@80x24.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
List-Id: <meta.public-inbox.org>

We haven't used _bidi_pipe idempotently in a while, so
the stderr was never getting reset on reads.
This isn't fully useful when using async eeeae20893a25956
(imap: use git-cat-file asynchronously, 2020-06-10)

So instead of truncating it on reads, we'll truncate
immediately after reading and rely on O_APPEND to keep
new writes at the end.

Fortunately, this stderrr error checking isn't used
outside of solver (which is synchronous).
---
 lib/PublicInbox/Git.pm | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/lib/PublicInbox/Git.pm b/lib/PublicInbox/Git.pm
index 53887e3d..eb88aa48 100644
--- a/lib/PublicInbox/Git.pm
+++ b/lib/PublicInbox/Git.pm
@@ -136,24 +136,20 @@ sub object_format {
 
 sub last_check_err {
 	my ($self) = @_;
-	my $fh = $self->{err_c} or return;
-	sysseek($fh, 0, 0) or $self->fail("sysseek failed: $!");
-	defined(sysread($fh, my $buf, -s $fh)) or
-			$self->fail("sysread failed: $!");
+	my $fh = $self->{err_c} or return '';
+	sysseek($fh, 0, 0) or $self->fail("sysseek: $!");
+	my $size = -s $fh or return '';
+	sysread($fh, my $buf, $size) // $self->fail("sysread: $!");
+	truncate($fh, 0) or $self->fail("truncate: $!");
 	$buf;
 }
 
 sub _bidi_pipe {
 	my ($self, $batch, $in, $out, $pid, $err) = @_;
-	if ($self->{$pid}) {
-		if (defined $err) { # "err_c"
-			my $fh = $self->{$err};
-			sysseek($fh, 0, 0) or $self->fail("sysseek failed: $!");
-			truncate($fh, 0) or $self->fail("truncate failed: $!");
-		}
+	if (defined $self->{$pid}) {
+		Carp::cluck("BUG: self->{$pid} exists unexpectedly");
 		return;
 	}
-
 	pipe(my ($out_r, $out_w)) or $self->fail("pipe failed: $!");
 	my $rdr = { 0 => $out_r, pgid => 0 };
 	my $gd = $self->{git_dir};
@@ -169,9 +165,8 @@ sub _bidi_pipe {
 			'cat-file', "--$batch");
 	if ($err) {
 		my $id = "git.$self->{git_dir}.$batch.err";
-		my $fh = tmpfile($id) or $self->fail("tmpfile($id): $!");
-		$self->{$err} = $fh;
-		$rdr->{2} = $fh;
+		$self->{$err} = $rdr->{2} = tmpfile($id, undef, 1) or
+						$self->fail("tmpfile($id): $!");
 	}
 	# see lib/PublicInbox/ProcessPipe.pm for why we don't use that here
 	my ($in_r, $p) = popen_rd(\@cmd, undef, $rdr);