Sorry for the long delay in the reply, but I'm a little low on time these days (and I've already spent some time trying to figure out what I was thinking - I wrote these patches a while ago). On Thu, Nov 26, 2009 at 11:03 PM, Johannes Sixt <j6t@xxxxxxxx> wrote: > On Donnerstag, 26. November 2009, Erik Faye-Lund wrote: >> @@ -372,37 +372,35 @@ static int run_service_command(const char **argv) >> cld.argv = argv; >> cld.git_cmd = 1; >> cld.err = -1; >> + cld.in = cld.out = fd; > > You shouldn't do that. In fact, the next patch 9 has a hunk that correctly > calls dup() once. > OK, as long as it works as expected, sure. But perhaps this needs a little change (see discussion later) >> - close(0); >> - close(1); > > Here, stdin and stdout were closed and start_command() used both. But these > two new calls > >> + exit(execute(0, addr)); >> ... >> + return execute(0, peer); > > are the only places where a value is assigned to fd. Now it is always only > stdin. Where does the old code initialize stdout? Shouldn't this place need a > change, too? The "dup2(incoming, 0)"-call in handle() is AFAICT what makes it work to use the forked process' stdin as both stdin and stdout for the service-process pipe (since fd 0 now becomes a pipe that is both readable and writable). This isn't exactly a pretty mechanism, and I guess I should rework it. At the very least, I should remove the "dup2(incoming, 1)"-call, but I'm open to other suggestions. Perhaps I can change this patch to do the entire socket-passing (which is currently in the next patch)? -- Erik "kusma" Faye-Lund -- 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