Re: [PATCH v8 19/37] am: convert applypatch hooks to use config

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 12, 2021, 10:23 UTC
Message-ID: <xmqqy2es7jro.fsf@gitster.g>
In-Reply-To: <20210311021037.3001235-20-emilyshaffer@google.com>

Emily Shaffer <emilyshaffer@google.com> writes:

Show 22 quoted lines

> @@ -1558,8 +1563,10 @@ static void do_commit(const struct am_state *state)
>  	struct commit_list *parents = NULL;
>  	const char *reflog_msg, *author, *committer = NULL;
>  	struct strbuf sb = STRBUF_INIT;
> +	struct run_hooks_opt hook_opt;
> +	run_hooks_opt_init_async(&hook_opt);
>  
> -	if (run_hook_le(NULL, "pre-applypatch", NULL))
> +	if (run_hooks("pre-applypatch", &hook_opt))
>  		exit(1);
>  
>  	if (write_cache_as_tree(&tree, 0, NULL))
> @@ -1611,8 +1618,9 @@ static void do_commit(const struct am_state *state)
>  		fclose(fp);
>  	}
>  
> -	run_hook_le(NULL, "post-applypatch", NULL);
> +	run_hooks("post-applypatch", &hook_opt);
>  
> +	run_hooks_opt_clear(&hook_opt);
>  	strbuf_release(&sb);
>  }

This one does opt_init(), run_hooks(), and another run_hooks() and then opt_clear(). If run_hooks() is a read-only operation on the hook_opt, then that would be alright, but it just smells iffy that it is not done as two separate opt_init(), run_hooks(), opt_clear() sequences for two separate run_hooks() invocations. The same worry about future safety I meantioned elsewhere in the series also applies.

Thanks.