From: Ilari Liusvaara <ilari.liusvaara@elisanet•fi>
To: "Shawn O. Pearce" <spearce@spearce•org>
Cc: git@vger•kernel.org
Subject: Re: [RFC PATCH v3 5/8] Support taking over transports
Date: Mon, 7 Dec 2009 23:19:08 +0200 [thread overview]
Message-ID: <20091207211907.GA3916@Knoppix> (raw)
In-Reply-To: <20091207174947.GF17173@spearce.org>
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
next prev parent reply other threads:[~2009-12-07 21:19 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-12-06 16:28 [RFC PATCH v3 0/8] Remote helpers smart transport extensions Ilari Liusvaara
2009-12-06 16:28 ` [RFC PATCH v3 1/8] Add remote helper debug mode Ilari Liusvaara
2009-12-06 16:28 ` [RFC PATCH v3 2/8] Support mandatory capabilities Ilari Liusvaara
2009-12-06 16:28 ` [RFC PATCH v3 3/8] Pass unknown protocols to external protocol handlers Ilari Liusvaara
2009-12-06 16:28 ` [RFC PATCH v3 4/8] Refactor git transport options parsing Ilari Liusvaara
2009-12-06 16:28 ` [RFC PATCH v3 5/8] Support taking over transports Ilari Liusvaara
2009-12-07 17:49 ` Shawn O. Pearce
2009-12-07 21:19 ` Ilari Liusvaara [this message]
2009-12-06 16:28 ` [RFC PATCH v3 6/8] Support remote helpers implementing smart transports Ilari Liusvaara
2009-12-07 18:11 ` Shawn O. Pearce
2009-12-07 20:35 ` Ilari Liusvaara
2009-12-06 16:28 ` [RFC PATCH v3 7/8] Support remote archive from external protocol helpers Ilari Liusvaara
2009-12-07 18:12 ` Shawn O. Pearce
2009-12-07 20:37 ` Ilari Liusvaara
2009-12-06 16:28 ` [RFC PATCH v3 8/8] Remove special casing of http, https and ftp Ilari Liusvaara
2009-12-07 7:36 ` [RFC PATCH v3 0/8] Remote helpers smart transport extensions Junio C Hamano
2009-12-07 12:06 ` Nanako Shiraishi
2009-12-07 12:57 ` Erik Faye-Lund
2009-12-07 15:44 ` Nicolas Pitre
2009-12-07 20:07 ` Junio C Hamano
2009-12-07 22:25 ` Nanako Shiraishi
2009-12-08 5:57 ` Jeff King
2009-12-08 6:29 ` Jeff King
2009-12-07 16:33 ` Ilari Liusvaara
2009-12-07 20:05 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20091207211907.GA3916@Knoppix \
--to=ilari.liusvaara@elisanet$(echo .)fi \
--cc=git@vger$(echo .)kernel.org \
--cc=spearce@spearce$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox