Re: [PATCH v2] midx: use buffered I/O to talk to pack-objects

From: Jeff King <peff@peff.net>
Date: Aug 13, 2020, 09:11 UTC
Message-ID: <20200813091103.GD3092220@coredump.intra.peff.net>
In-Reply-To: <xmqqd03v61xd.fsf@gitster.c.googlers.com>

On Wed, Aug 12, 2020 at 01:31:58PM -0700, Junio C Hamano wrote:

Show 18 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > René Scharfe <l.s.r@web.de> writes:
> >
> >>  		nth_midxed_object_oid(&oid, m, i);
> >> -		xwrite(cmd.in, oid_to_hex(&oid), the_hash_algo->hexsz);
> >> -		xwrite(cmd.in, "\n", 1);
> >> +		fprintf(cmd_in, "%s\n", oid_to_hex(&oid));
> >
> > I do think it is silly to send an object name and terminating LF in
> > two different system calls per object.
> >
> > The original uses xwrite() so that it does not have to worry about
> > having to restart interrupted system calls and such.
> 
> Oops.  There is not much in "and such".  xwrite() only restarts but
> the caller must deal with truncated write, which the original does
> not do.

Yeah, so I think the new code should be an improvement there, because it will eventually retry the write when the buffer fills, or when we eventually flush on close (my "everything written by the time we hit fflush()" in the previous mail really should have been "...by the time we hit fclose()"). Maybe another reason to use stdio. :)

The original also doesn't handle "real" errors at all (stuff besides EINTR, etc). Nor does the rewrite. I guess it's unlikely to see a write error here that doesn't also involve pack-objects returning a non-zero exit code.

-Peff