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

From: Jeff King <peff@peff.net>
Date: Aug 13, 2020, 09:06 UTC
Message-ID: <20200813090635.GC3092220@coredump.intra.peff.net>
In-Reply-To: <xmqqh7t7623d.fsf@gitster.c.googlers.com>

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

Show 14 quoted lines

> 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.  Do we need to
> do that ourselves now or does the stdio layer take care of it for
> us?

I think we're OK in this instance because we are not expecting writes to happen at any given moment. We might do a partial write() for any given fprintf(), but we're OK as long as by the fflush() at the end everything is written.

For more general cases where we do care about ordering (e.g., if we were interleaving writes and reads with something like cat-file), we'd need to fflush() after each write. And I'd expect fflush() to retry across partial write() or EINTR anyway as a quality-of-implementation thing. Which doesn't make it true, but glibc empirically seems to behave that way, and I think we'd be better off adding a transparent wrapper to fflush() on any system that doesn't.

-Peff