From: Adrian Ratiu <adrian.ratiu@collabora•com>
To: git@vger•kernel.org
Cc: Emily Shaffer <emilyshaffer@google•com>,
Junio C Hamano <gitster@pobox•com>,
Patrick Steinhardt <ps@pks•im>,
"brian m . carlson" <sandals@crustytoothpaste•net>,
Adrian Ratiu <adrian.ratiu@collabora•com>
Subject: [PATCH v3 06/12] hook: replace hook_list_clear() -> string_list_clear_func()
Date: Wed, 25 Mar 2026 21:54:57 +0200 [thread overview]
Message-ID: <20260325195503.1139418-7-adrian.ratiu@collabora.com> (raw)
In-Reply-To: <20260325195503.1139418-1-adrian.ratiu@collabora.com>
Replace the custom function with string_list_clear_func() which
is a more common pattern for clearing a string_list.
To be able to do this, rework hook_clear() into hook_free(), so
it can be passed to string_list_clear_func().
A slight complication is the need to keep a copy of the internal
cb data free() pointer, however I think it's worth it since the
API becomes cleaner, e.g. no more calls with NULL function args
like hook_list_clear(hooks, NULL).
In other words, the callers don't need to keep track of hook
internal state to determine when cleanup is necessary or not
(pass NULL) because each `struct hook` now owns its data_free
callback.
Suggested-by: Patrick Steinhardt <ps@pks•im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora•com>
---
builtin/hook.c | 2 +-
hook.c | 40 ++++++++++++++++++++++------------------
hook.h | 20 ++++++++++++++------
3 files changed, 37 insertions(+), 25 deletions(-)
diff --git a/builtin/hook.c b/builtin/hook.c
index e641614b84..54b737990b 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -78,7 +78,7 @@ static int list(int argc, const char **argv, const char *prefix,
}
cleanup:
- hook_list_clear(head, NULL);
+ string_list_clear_func(head, hook_free);
free(head);
return ret;
}
diff --git a/hook.c b/hook.c
index b0226ed716..021110f216 100644
--- a/hook.c
+++ b/hook.c
@@ -52,8 +52,10 @@ const char *find_hook(struct repository *r, const char *name)
return path.buf;
}
-static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
+void hook_free(void *p, const char *str UNUSED)
{
+ struct hook *h = p;
+
if (!h)
return;
@@ -64,22 +66,12 @@ static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
free((void *)h->u.configured.command);
}
- if (cb_data_free)
- cb_data_free(h->feed_pipe_cb_data);
+ if (h->data_free && h->feed_pipe_cb_data)
+ h->data_free(h->feed_pipe_cb_data);
free(h);
}
-void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free)
-{
- struct string_list_item *item;
-
- for_each_string_list_item(item, hooks)
- hook_clear(item->util, cb_data_free);
-
- string_list_clear(hooks, 0);
-}
-
/* Helper to detect and add default "traditional" hooks from the hookdir. */
static void list_hooks_add_default(struct repository *r, const char *hookname,
struct string_list *hook_list,
@@ -100,9 +92,15 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
if (options && options->dir)
hook_path = absolute_path(hook_path);
- /* Setup per-hook internal state cb data */
- if (options && options->feed_pipe_cb_data_alloc)
+ /*
+ * Setup per-hook internal state callback data.
+ * When provided, the alloc/free callbacks are always provided
+ * together, so use them to alloc/free the internal hook state.
+ */
+ if (options && options->feed_pipe_cb_data_alloc) {
h->feed_pipe_cb_data = options->feed_pipe_cb_data_alloc(options->feed_pipe_ctx);
+ h->data_free = options->feed_pipe_cb_data_free;
+ }
h->kind = HOOK_TRADITIONAL;
h->u.traditional.path = xstrdup(hook_path);
@@ -316,10 +314,16 @@ static void list_hooks_add_configured(struct repository *r,
CALLOC_ARRAY(hook, 1);
- if (options && options->feed_pipe_cb_data_alloc)
+ /*
+ * When provided, the alloc/free callbacks are always provided
+ * together, so use them to alloc/free the internal hook state.
+ */
+ if (options && options->feed_pipe_cb_data_alloc) {
hook->feed_pipe_cb_data =
options->feed_pipe_cb_data_alloc(
options->feed_pipe_ctx);
+ hook->data_free = options->feed_pipe_cb_data_free;
+ }
hook->kind = HOOK_CONFIGURED;
hook->u.configured.friendly_name = xstrdup(friendly_name);
@@ -362,7 +366,7 @@ int hook_exists(struct repository *r, const char *name)
{
struct string_list *hooks = list_hooks(r, name, NULL);
int exists = hooks->nr > 0;
- hook_list_clear(hooks, NULL);
+ string_list_clear_func(hooks, hook_free);
free(hooks);
return exists;
}
@@ -516,7 +520,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name,
run_processes_parallel(&opts);
ret = cb_data.rc;
cleanup:
- hook_list_clear(cb_data.hook_command_list, options->feed_pipe_cb_data_free);
+ string_list_clear_func(cb_data.hook_command_list, hook_free);
free(cb_data.hook_command_list);
run_hooks_opt_clear(options);
return ret;
diff --git a/hook.h b/hook.h
index 965794a5b8..a56ac20ccf 100644
--- a/hook.h
+++ b/hook.h
@@ -7,6 +7,9 @@
struct repository;
+typedef void (*hook_data_free_fn)(void *data);
+typedef void *(*hook_data_alloc_fn)(void *init_ctx);
+
/**
* Represents a hook command to be run.
* Hooks can be:
@@ -41,10 +44,15 @@ struct hook {
* Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it.
*/
void *feed_pipe_cb_data;
-};
-typedef void (*hook_data_free_fn)(void *data);
-typedef void *(*hook_data_alloc_fn)(void *init_ctx);
+ /**
+ * Callback to free `feed_pipe_cb_data`.
+ *
+ * It is called automatically and points to the `feed_pipe_cb_data_free`
+ * provided via the `run_hook_opt` parameter.
+ */
+ hook_data_free_fn data_free;
+};
struct run_hooks_opt {
/* Environment vars to be set for each hook */
@@ -185,10 +193,10 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
struct run_hooks_opt *options);
/**
- * Frees the memory allocated for the hook list, including the `struct hook`
- * items and their internal state.
+ * Frees a struct hook stored as the util pointer of a string_list_item.
+ * Suitable for use as a string_list_clear_func_t callback.
*/
-void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free);
+void hook_free(void *p, const char *str);
/**
* Frees the hook configuration cache stored in `struct repository`.
--
2.52.0.732.gb351b5166d.dirty
next prev parent reply other threads:[~2026-03-25 19:56 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-09 0:54 [PATCH 00/10] config-hook cleanups and two small 'git hook list' features Adrian Ratiu
2026-03-09 0:54 ` [PATCH 01/10] hook: move unsorted_string_list_remove() to string-list.[ch] Adrian Ratiu
2026-03-10 19:56 ` SZEDER Gábor
2026-03-11 11:08 ` Adrian Ratiu
2026-03-09 0:54 ` [PATCH 02/10] hook: fix minor style issues Adrian Ratiu
2026-03-09 2:12 ` Eric Sunshine
2026-03-09 0:54 ` [PATCH 03/10] hook: rename cb_data_free/alloc -> hook_data_free/alloc Adrian Ratiu
2026-03-11 10:24 ` Patrick Steinhardt
2026-03-11 11:09 ` Adrian Ratiu
2026-03-09 0:54 ` [PATCH 04/10] hook: detect & emit two more bugs Adrian Ratiu
2026-03-09 0:54 ` [PATCH 05/10] hook: replace hook_list_clear() -> string_list_clear_func() Adrian Ratiu
2026-03-09 2:18 ` Eric Sunshine
2026-03-10 14:20 ` Adrian Ratiu
2026-03-09 0:54 ` [PATCH 06/10] hook: make consistent use of friendly-name in docs Adrian Ratiu
2026-03-09 0:54 ` [PATCH 07/10] t1800: add test to verify hook execution ordering Adrian Ratiu
2026-03-09 0:54 ` [PATCH 08/10] hook: refactor hook_config_cache from strmap to named struct Adrian Ratiu
2026-03-09 21:59 ` Junio C Hamano
2026-03-10 14:19 ` Adrian Ratiu
2026-03-09 0:54 ` [PATCH 09/10] hook: show config scope in git hook list Adrian Ratiu
2026-03-09 21:59 ` Junio C Hamano
2026-03-10 14:45 ` Adrian Ratiu
2026-03-11 10:24 ` Patrick Steinhardt
2026-03-11 11:47 ` Adrian Ratiu
2026-03-09 0:54 ` [PATCH 10/10] hook: show disabled hooks in "git hook list" Adrian Ratiu
2026-03-11 10:24 ` Patrick Steinhardt
2026-03-11 12:24 ` Adrian Ratiu
2026-03-11 13:53 ` Patrick Steinhardt
2026-03-09 20:14 ` [PATCH 00/10] config-hook cleanups and two small 'git hook list' features Junio C Hamano
2026-03-10 14:37 ` Adrian Ratiu
2026-03-09 20:27 ` Junio C Hamano
2026-03-20 11:52 ` [PATCH v2 " Adrian Ratiu
2026-03-20 11:52 ` [PATCH v2 01/10] hook: move unsorted_string_list_remove() to string-list.[ch] Adrian Ratiu
2026-03-20 11:52 ` [PATCH v2 02/10] hook: fix minor style issues Adrian Ratiu
2026-03-24 8:37 ` Patrick Steinhardt
2026-03-24 19:19 ` Adrian Ratiu
2026-03-20 11:52 ` [PATCH v2 03/10] hook: rename cb_data_free/alloc -> hook_data_free/alloc Adrian Ratiu
2026-03-20 11:52 ` [PATCH v2 04/10] hook: detect & emit two more bugs Adrian Ratiu
2026-03-20 11:52 ` [PATCH v2 05/10] hook: replace hook_list_clear() -> string_list_clear_func() Adrian Ratiu
2026-03-24 8:37 ` Patrick Steinhardt
2026-03-24 22:33 ` Adrian Ratiu
2026-03-25 5:26 ` Patrick Steinhardt
2026-03-20 11:52 ` [PATCH v2 06/10] hook: make consistent use of friendly-name in docs Adrian Ratiu
2026-03-20 11:52 ` [PATCH v2 07/10] t1800: add test to verify hook execution ordering Adrian Ratiu
2026-03-20 11:52 ` [PATCH v2 08/10] hook: introduce hook_config_cache_entry for per-hook data Adrian Ratiu
2026-03-20 11:52 ` [PATCH v2 09/10] hook: show config scope in git hook list Adrian Ratiu
2026-03-24 8:37 ` Patrick Steinhardt
2026-03-25 11:28 ` Adrian Ratiu
2026-03-20 11:52 ` [PATCH v2 10/10] hook: show disabled hooks in "git hook list" Adrian Ratiu
2026-03-24 8:38 ` Patrick Steinhardt
2026-03-24 16:14 ` Junio C Hamano
2026-03-24 19:23 ` Adrian Ratiu
2026-03-23 16:11 ` [PATCH v2 00/10] config-hook cleanups and two small 'git hook list' features Junio C Hamano
2026-03-24 8:38 ` Patrick Steinhardt
2026-03-24 18:56 ` Adrian Ratiu
2026-03-25 19:54 ` [PATCH v3 00/12] config-hook cleanups and three small git-hook features Adrian Ratiu
2026-03-25 19:54 ` [PATCH v3 01/12] hook: move unsorted_string_list_remove() to string-list.[ch] Adrian Ratiu
2026-03-25 19:54 ` [PATCH v3 02/12] builtin/receive-pack: properly init receive_hook strbuf Adrian Ratiu
2026-03-25 19:54 ` [PATCH v3 03/12] hook: fix minor style issues Adrian Ratiu
2026-03-25 19:54 ` [PATCH v3 04/12] hook: rename cb_data_free/alloc -> hook_data_free/alloc Adrian Ratiu
2026-03-25 19:54 ` [PATCH v3 05/12] hook: detect & emit two more bugs Adrian Ratiu
2026-03-25 19:54 ` Adrian Ratiu [this message]
2026-03-25 19:54 ` [PATCH v3 07/12] hook: make consistent use of friendly-name in docs Adrian Ratiu
2026-03-25 19:54 ` [PATCH v3 08/12] t1800: add test to verify hook execution ordering Adrian Ratiu
2026-03-25 19:55 ` [PATCH v3 09/12] hook: introduce hook_config_cache_entry for per-hook data Adrian Ratiu
2026-03-25 19:55 ` [PATCH v3 10/12] hook: show config scope in git hook list Adrian Ratiu
2026-03-25 19:55 ` [PATCH v3 11/12] hook: show disabled hooks in "git hook list" Adrian Ratiu
2026-03-25 19:55 ` [PATCH v3 12/12] hook: reject unknown hook names in git-hook(1) Adrian Ratiu
2026-03-25 21:17 ` [PATCH v3 00/12] config-hook cleanups and three small git-hook features Junio C Hamano
2026-03-26 10:21 ` Adrian Ratiu
2026-03-27 8:04 ` Patrick Steinhardt
2026-03-27 16:11 ` Junio C Hamano
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=20260325195503.1139418-7-adrian.ratiu@collabora.com \
--to=adrian.ratiu@collabora$(echo .)com \
--cc=emilyshaffer@google$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=ps@pks$(echo .)im \
--cc=sandals@crustytoothpaste$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox