Hi,
On Thu, 15 Jan 2009, Stephan Beyer wrote:
> Stripping out a libified version seemed better to me than
> copy and paste.
Oh, definitely.
> - ret = start_command(&hook);
> - if (ret) {
> - warning("Could not spawn %s", argv[0]);
> - return ret;
> - }
> - ret = finish_command(&hook);
> - if (ret == -ERR_RUN_COMMAND_WAITPID_SIGNAL)
> - warning("%s exited due to uncaught signal", argv[0]);
What are the side effects of replacing this with "ret =
run_command(&hook);"? This has to be discussed and defended in the commit
message.
> diff --git a/run-command.c b/run-command.c
> index c90cdc5..602fe85 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -342,3 +342,38 @@ int finish_async(struct async *async)
> #endif
> return ret;
> }
> +
> +int run_hook(const char *index_file, const char *name, ...)
> +{
> + struct child_process hook;
> + const char *argv[10], *env[2];
> + char index[PATH_MAX];
> + va_list args;
> + int i;
> +
> + va_start(args, name);
> + argv[0] = git_path("hooks/%s", name);
> + i = 0;
> + do {
> + if (++i >= ARRAY_SIZE(argv))
> + die("run_hook(): too many arguments");
> + argv[i] = va_arg(args, const char *);
> + } while (argv[i]);
> + va_end(args);
> +
> + if (access(argv[0], X_OK) < 0)
> + return 0;
> +
> + memset(&hook, 0, sizeof(hook));
> + hook.argv = argv;
> + hook.no_stdin = 1;
> + hook.stdout_to_stderr = 1;
> + if (index_file) {
> + snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
> + env[0] = index;
> + env[1] = NULL;
> + hook.env = env;
> + }
> +
> + return run_command(&hook);
> +}
Lots of improvements possible (I agree; _after_ this patch):
- deuglify the loop,
- use ALLOC_GROW instead of having a fixed size argv,
- use an strbuf for the index file
- checking executability of argv[0] before filling argv,
and possibly others, too.
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
