All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Sixt <j.sixt@viscovery.net>
Cc: Johannes Sixt <johannes.sixt@telecom.at>, git@vger.kernel.org
Subject: Re: [PATCH 1/5] Change git_connect() to return a struct child_process instead of a pid_t.
Date: Tue, 02 Oct 2007 10:54:11 -0700	[thread overview]
Message-ID: <7v641pmmgs.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <4700B8FC.70704@viscovery.net> (Johannes Sixt's message of "Mon, 01 Oct 2007 11:08:12 +0200")

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

> 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.

  reply	other threads:[~2007-10-02 17:54 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-30 20:09 [PATCH 0/5] fork/exec removal series Johannes Sixt
2007-09-30 20:09 ` [PATCH 1/5] Change git_connect() to return a struct child_process instead of a pid_t Johannes Sixt
2007-09-30 20:09   ` [PATCH 2/5] Use start_command() in git_connect() instead of explicit fork/exec Johannes Sixt
2007-09-30 20:09     ` [PATCH 3/5] Use start_command() to run the filter " Johannes Sixt
2007-09-30 20:10       ` [PATCH 4/5] Use run_command() to spawn external diff programs instead of fork/exec Johannes Sixt
2007-09-30 20:10         ` [PATCH 5/5] Use start_comand() in builtin-fetch-pack.c instead of explicit fork/exec Johannes Sixt
2007-09-30 21:10         ` [PATCH 4/5] Use run_command() to spawn external diff programs instead of fork/exec Johannes Schindelin
2007-09-30 21:07       ` [PATCH 3/5] Use start_command() to run the filter instead of explicit fork/exec Johannes Schindelin
2007-09-30 20:43   ` [PATCH 1/5] Change git_connect() to return a struct child_process instead of a pid_t Junio C Hamano
2007-09-30 21:40     ` Johannes Sixt
2007-10-03 20:09       ` [PATCH 0/5, resend] fork/exec removal series Johannes Sixt
2007-10-03 20:09         ` [PATCH 1/5] Change git_connect() to return a struct child_process instead of a pid_t Johannes Sixt
2007-10-03 20:09           ` [PATCH 2/5] Use start_command() in git_connect() instead of explicit fork/exec Johannes Sixt
2007-10-03 20:09             ` [PATCH 3/5] Use start_command() to run the filter " Johannes Sixt
2007-10-03 20:09               ` [PATCH 4/5] Use run_command() to spawn external diff programs instead of fork/exec Johannes Sixt
2007-10-03 20:09                 ` [PATCH 5/5] Use start_comand() in builtin-fetch-pack.c instead of explicit fork/exec Johannes Sixt
2007-10-04  8:55                   ` Junio C Hamano
2007-10-04  9:22                     ` Johannes Sixt
2007-10-04 20:11                       ` Johannes Sixt
2007-10-01  7:23     ` [PATCH 1/5] Change git_connect() to return a struct child_process instead of a pid_t Johannes Sixt
2007-10-01  8:39       ` Junio C Hamano
2007-10-01  9:08         ` Johannes Sixt
2007-10-02 17:54           ` Junio C Hamano [this message]
2007-09-30 21:04   ` Johannes Schindelin
2007-09-30 20:20 ` [PATCH 0/5] fork/exec removal series Junio C Hamano
2007-09-30 21:14 ` Johannes Schindelin
2007-09-30 21:34   ` Johannes Sixt
2007-09-30 21:43     ` Johannes Schindelin
2007-10-01  7:07       ` Johannes Sixt
2007-10-01  9:49         ` David Kastrup

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=7v641pmmgs.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=j.sixt@viscovery.net \
    --cc=johannes.sixt@telecom.at \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.