All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Sixt <j6t@kdbg.org>
To: Stefan Beller <sbeller@google.com>, gitster@pobox.com
Cc: git@vger.kernel.org
Subject: Re: [PATCH] run-command: do not pass child process data into callbacks
Date: Tue, 1 Mar 2016 08:13:45 +0100	[thread overview]
Message-ID: <56D54129.4090007@kdbg.org> (raw)
In-Reply-To: <1456783026-3328-1-git-send-email-sbeller@google.com>

Am 29.02.2016 um 22:57 schrieb Stefan Beller:
> The expected way to pass data into the callback is to pass them via
> the customizable callback pointer. The error reporting in
> default_{start_failure, task_finished} is not user friendly enough, that
> we want to encourage using the child data for such purposes.
> 
> Furthermore the struct child data is cleaned by the run-command API,
> before we access them in the callbacks, leading to use-after-free
> situations.

Thanks. The code changes match what I had prototyped. But please squash
in this documentation change:

diff --git a/run-command.h b/run-command.h
index c6a3e42..3d1e59e 100644
--- a/run-command.h
+++ b/run-command.h
@@ -191,9 +191,8 @@ typedef int (*task_finished_fn)(int result,
  * (both stdout and stderr) is routed to stderr in a manner that output
  * from different tasks does not interleave.
  *
- * If start_failure_fn or task_finished_fn are NULL, default handlers
- * will be used. The default handlers will print an error message on
- * error without issuing an emergency stop.
+ * start_failure_fn and task_finished_fn can be NULL to omit any
+ * special handling.
  */
 int run_processes_parallel(int n,
 			   get_next_task_fn,

  parent reply	other threads:[~2016-03-01  7:13 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-29 21:57 [PATCH] run-command: do not pass child process data into callbacks Stefan Beller
2016-02-29 21:58 ` Stefan Beller
2016-02-29 23:53   ` Junio C Hamano
2016-03-01  7:13 ` Johannes Sixt [this message]
2016-03-01 17:43   ` Junio C Hamano
2016-03-01 17:55     ` [PATCHv2] " Stefan Beller

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=56D54129.4090007@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sbeller@google.com \
    /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.