[15/17] hook: provide stdin by string_list or callback

Commit Message

Emily Shaffer Dec. 5, 2020, 1:46 a.m. UTC

In cases where a hook requires only a small amount of information via
stdin, it should be simple for users to provide a string_list alone. But
in more complicated cases where the stdin is too large to hold in
memory, let's provide a callback the users can populate line after line
with instead.

Signed-off-by: Emily Shaffer <[e-mail]>
---
 hook.c | 39 +++++++++++++++++++++++++++++++++++++++
 hook.h | 25 +++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

Comments

SZEDER Gábor Dec. 8, 2020, 9:09 p.m. UTC | #1

On Fri, Dec 04, 2020 at 05:46:05PM -0800, Emily Shaffer wrote:
> diff --git a/hook.c b/hook.c
> index f0c052d847..fbb49f241d 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -9,6 +9,8 @@ void free_hook(struct hook *ptr)
>  {
>  	if (ptr) {
>  		strbuf_release(&ptr->command);
> +		if (ptr->feed_pipe_cb_data)

Coccinelle suggests to drop this condition, because free() can handle
a NULL pointer just fine.

> +			free(ptr->feed_pipe_cb_data);
>  		free(ptr);
>  	}
>  }

Emily Shaffer Dec. 8, 2020, 10:11 p.m. UTC | #2

On Tue, Dec 08, 2020 at 10:09:25PM +0100, SZEDER Gábor wrote:
> 
> On Fri, Dec 04, 2020 at 05:46:05PM -0800, Emily Shaffer wrote:
> > diff --git a/hook.c b/hook.c
> > index f0c052d847..fbb49f241d 100644
> > --- a/hook.c
> > +++ b/hook.c
> > @@ -9,6 +9,8 @@ void free_hook(struct hook *ptr)
> >  {
> >  	if (ptr) {
> >  		strbuf_release(&ptr->command);
> > +		if (ptr->feed_pipe_cb_data)
> 
> Coccinelle suggests to drop this condition, because free() can handle
> a NULL pointer just fine.

Done (locally). Thanks (and thanks for checking the coccinelle output
too).

> 
> > +			free(ptr->feed_pipe_cb_data);
> >  		free(ptr);
> >  	}
> >  }

Patch

diff --git a/hook.c b/hook.c
index f0c052d847..fbb49f241d 100644
--- a/hook.c
+++ b/hook.c
@@ -9,6 +9,8 @@  void free_hook(struct hook *ptr)
 {
 	if (ptr) {
 		strbuf_release(&ptr->command);
+		if (ptr->feed_pipe_cb_data)
+			free(ptr->feed_pipe_cb_data);
 		free(ptr);
 	}
 }
@@ -38,6 +40,7 @@  static void append_or_move_hook(struct list_head *head, const char *command)
 		strbuf_init(&to_add->command, 0);
 		strbuf_addstr(&to_add->command, command);
 		to_add->from_hookdir = 0;
+		to_add->feed_pipe_cb_data = NULL;
 	}
 
 	/* re-set the scope so we show where an override was specified */
@@ -253,9 +256,32 @@  void run_hooks_opt_clear(struct run_hooks_opt *o)
 {
 	strvec_clear(&o->env);
 	strvec_clear(&o->args);
+	string_list_clear(&o->str_stdin, 0);
 }
 
 
+static int pipe_from_string_list(struct strbuf *pipe, void *pp_cb, void *pp_task_cb)
+{
+	int *item_idx;
+	struct hook *ctx = pp_task_cb;
+	struct string_list *to_pipe = &((struct hook_cb_data*)pp_cb)->options->str_stdin;
+
+	/* Bootstrap the state manager if necessary. */
+	if (!ctx->feed_pipe_cb_data) {
+		ctx->feed_pipe_cb_data = xmalloc(sizeof(unsigned int));
+		*(int*)ctx->feed_pipe_cb_data = 0;
+	}
+
+	item_idx = ctx->feed_pipe_cb_data;
+
+	if (*item_idx < to_pipe->nr) {
+		strbuf_addf(pipe, "%s\n", to_pipe->items[*item_idx].string);
+		(*item_idx)++;
+		return 0;
+	}
+	return 1;
+}
+
 static int pick_next_hook(struct child_process *cp,
 			  struct strbuf *out,
 			  void *pp_cb,
@@ -277,6 +303,10 @@  static int pick_next_hook(struct child_process *cp,
 	if (hook_cb->options->path_to_stdin) {
 		cp->no_stdin = 0;
 		cp->in = xopen(hook_cb->options->path_to_stdin, O_RDONLY);
+	} else if (hook_cb->options->feed_pipe) {
+		/* ask for start_command() to make a pipe for us */
+		cp->in = -1;
+		cp->no_stdin = 0;
 	} else {
 		cp->no_stdin = 1;
 	}
@@ -350,6 +380,14 @@  int run_hooks(const char *hookname, struct run_hooks_opt *options)
 	if (!options)
 		BUG("a struct run_hooks_opt must be provided to run_hooks");
 
+	if ((options->path_to_stdin && options->str_stdin.nr) ||
+	    (options->path_to_stdin && options->feed_pipe) ||
+	    (options->str_stdin.nr && options->feed_pipe))
+		BUG("choose only one method to populate stdin");
+
+	if (options->str_stdin.nr)
+		options->feed_pipe = &pipe_from_string_list;
+
 	strbuf_addstr(&hookname_str, hookname);
 
 	to_run = hook_list(&hookname_str);
@@ -368,6 +406,7 @@  int run_hooks(const char *hookname, struct run_hooks_opt *options)
 	run_processes_parallel_tr2(options->jobs,
 				   pick_next_hook,
 				   notify_start_failure,
+				   options->feed_pipe,
 				   notify_hook_finished,
 				   &cb_data,
 				   "hook",
diff --git a/hook.h b/hook.h
index 4aae8e2dbb..ace26c637e 100644
--- a/hook.h
+++ b/hook.h
@@ -2,6 +2,7 @@ 
 #include "list.h"
 #include "strbuf.h"
 #include "strvec.h"
+#include "run-command.h"
 
 struct hook
 {
@@ -14,6 +15,12 @@  struct hook
 	/* The literal command to run. */
 	struct strbuf command;
 	int from_hookdir;
+
+	/*
+	 * Use this to keep state for your feed_pipe_fn if you are using
+	 * run_hooks_opt.feed_pipe. Otherwise, do not touch it.
+	 */
+	void *feed_pipe_cb_data;
 };
 
 /*
@@ -57,12 +64,24 @@  struct run_hooks_opt
 
 	/* Path to file which should be piped to stdin for each hook */
 	const char *path_to_stdin;
+	/* Pipe each string to stdin, separated by newlines */
+	struct string_list str_stdin;
+	/*
+	 * Callback and state pointer to ask for more content to pipe to stdin.
+	 * Will be called repeatedly, for each hook. See
+	 * hook.c:pipe_from_stdin() for an example. Keep per-hook state in
+	 * hook.feed_pipe_cb_data (per process). Keep initialization context in
+	 * feed_pipe_ctx (shared by all processes).
+	 */
+	feed_pipe_fn feed_pipe;
+	void *feed_pipe_ctx;
 
 	/* Number of threads to parallelize across */
 	int jobs;
 
 	/* Path to initial working directory for subprocess */
 	const char *dir;
+
 };
 
 /*
@@ -81,6 +100,9 @@  struct hook_cb_data {
 	.path_to_stdin = NULL,			\
 	.jobs = 1,				\
 	.dir = NULL,				\
+	.str_stdin = STRING_LIST_INIT_DUP,	\
+	.feed_pipe = NULL,			\
+	.feed_pipe_ctx = NULL,			\
 	.run_hookdir = configured_hookdir_opt()	\
 }
 
@@ -90,6 +112,9 @@  struct hook_cb_data {
 	.path_to_stdin = NULL,			\
 	.jobs = configured_hook_jobs(),		\
 	.dir = NULL,				\
+	.str_stdin = STRING_LIST_INIT_DUP,	\
+	.feed_pipe = NULL,			\
+	.feed_pipe_ctx = NULL,			\
 	.run_hookdir = configured_hookdir_opt()	\
 }