From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Sun, 10 Oct 2010 20:42:44 GMT
Subject: Re: [PATCH v3 08/14] daemon: use run-command api for async serving
Message-ID: <AANLkTinZgXF5pxq_HG1veQwO9_=Pmm5Vk2PUHd6=h7yQ@mail.gmail.com>
In-Reply-To: <4CB21A5D.4020408@sunshineco.com>

On Sun, Oct 10, 2010 at 9:56 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On 10/10/2010 9:20 AM, Erik Faye-Lund wrote:
>>
>> fork() is only available on POSIX, so to support git-daemon
>> on Windows we have to use something else.
>>
>> Instead we invent the flag --serve, which is a stripped down
>> version of --inetd-mode. We use start_command() to call
>> git-daemon with this flag appended to serve clients.
>>
>> Signed-off-by: Erik Faye-Lund<kusmabite@gmail.com>
>> ---
>> @@ -654,14 +641,14 @@ static void remove_child(pid_t pid)
>>   */
>>  static void kill_some_child(void)
>>  {
>> -       const struct child *blanket, *next;
>> +       struct child *blanket, *next;
>
> It is not immediately obvious why 'const' was dropped.
>

It's a left-over hunk from a previous version. Thanks for pointing it out!

>> @@ -671,18 +658,26 @@ static void check_dead_children(void)
>>        int status;
>>        pid_t pid;
>>
>> -       while ((pid = waitpid(-1,&status, WNOHANG))>  0) {
>> -               const char *dead = "";
>> -               remove_child(pid);
>> -               if (!WIFEXITED(status) || (WEXITSTATUS(status)>  0))
>> -                       dead = " (with error)";
>> -               loginfo("[%"PRIuMAX"] Disconnected%s", (uintmax_t)pid,
>> dead);
>> -       }
>> +       struct child **cradle, *blanket;
>> +       for (cradle =&firstborn; (blanket = *cradle);)
>> +               if ((pid = waitpid(blanket->cld.pid,&status, WNOHANG))>
>>  1) {
>> +                       const char *dead = "";
>> +                       if (status)
>> +                               dead = " (with error)";
>> +                       loginfo("[%"PRIuMAX"] Disconnected%s",
>> (uintmax_t)pid, dead);
>> +
>> +                       /* remove the child */
>> +                       *cradle = blanket->next;
>> +                       live_children--;
>> +                       free(blanket);
>> +               } else
>> +                       cradle =&blanket->next;
>>  }
>>
>> +char **cld_argv;
>>  static void handle(int incoming, struct sockaddr *addr, int addrlen)
>>  {
>
> Should 'cld_argv' be declared static?
>

Yes it should, thanks!