From: Erik Faye-Lund <kusmabite@googlemail.com>
Date: Fri, 27 Nov 2009 14:23:02 GMT
Subject: Re: [msysGit] [PATCH/RFC 08/11] daemon: use explicit file descriptor
Message-ID: <40aa078e0911270623m1a06890cmd2d46b3d9e216769@mail.gmail.com>
In-Reply-To: <200911262303.57228.j6t@kdbg.org>

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@kdbg.org> 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