Re: [PATCH 1/5] Change git_connect() to return a struct child_process instead of a pid_t.

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 2, 2007, 17:54 UTC
Message-ID: <7v641pmmgs.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <4700B8FC.70704@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

Show 18 quoted lines

> Junio C Hamano schrieb:
> ...
>> As to error indication, it somehow does not feel right to return
>> something called "child _process_" structure when we want to
>> tell the caller that there is no process to wait for in the
>> no-error case, although the fact that we can use .in/.out fd in
>> the structure when we _do_ have child process is attractive.
>
> Did you mean: "even if we don't have a child process"?
>
> How about a typedef if you dislike the name?
>
>> As an alternative, we could keep the "NULL return means there
>> was no need to fork" semantics of git_connect(), and instead add
>> "int *status_ret" parameter for the caller to check.
>
> Seriously? Add an *out* parameter when we can get rid of one and have
> a return value, too?

Ah, I somehow got confused and thought that the caller decides not to do the waitpid business at the end of the connection, but as everybody calls finish_connect() with what it got from git_connect(), as long as the fake "child_process" structure records something to let finish_connect() know that it should not waitpid() on the process, all is well.

It might make sense to teach finish_command() that a magic value of (cmd->pid == 0) means there is no process to wait for and this "child_process" structure is only about the in/out stream to the other side.