On Mon, Dec 07, 2009 at 09:49:47AM -0800, Shawn O. Pearce wrote:
>
> > + if (duped < 0)
> > + die_errno("Can't dup helper output fd");
> > + data->out = xfdopen(duped, "r");
> > + setvbuf(data->out, NULL, _IONBF, 0);
>
> I wonder if this is really a good idea. Most helpers actually
> use a lot of text based IO to communicate. Disabling buffering
> for those helpers to avoid overreading the advertisement from a
> connect is a problem.
I dropped the buffering change.
> Maybe we could leave buffering on, but use a handshake protocol
> with the helper during connect:
>
> (1) > "connect git-upload-pack\n"
> (2) < "\n"
> (3) > "begin\n"
>
> During 2 we are still buffered, but the only content on the pipe
> should be the single blank line, so we pull that in and the FILE*
> buffer should be empty.
Doesn't work. Stream buffering can only be changed before first
I/O.
I figured out a solution. Dup the file descriptor second time and
make another FILE* for it. Then send the connect and read the
response over the new stream. This avoids overreading and allows
other I/O to be buffered. After connect attempt, the new streaam
can be closed.
This way most of the I/O can be buffered, only reading responses
to connect commands can't, and that is at most 1 byte if fallbacks
aren't required. And the thing fits nicely inside _process_connect().
Oh, and no protocol change needed.
-Ilari
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html