Re: [PATCH v8 19/37] am: convert applypatch hooks to use config
From: Junio C Hamano <hidden>
Date: 2021-03-12 10:27:09
Emily Shaffer [off-list ref] writes:
quoted hunk ↗ jump to hunk
@@ -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.