hook: replace hook_list_clear() -> string_list_clear_func()
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> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Adrian Ratiu committed
Mar 25, 2026 at 21:54 UTC
a8b1ba86d494ea8825292c91c243e5d84fd7ee2c
3 files changed
+37
-25
builtin/hook.c
+1
-1
@@ -78,7 +78,7 @@ static int list(int argc, const char **argv, const char *prefix,
78
}
79
80
cleanup:
81
- hook_list_clear(head, NULL);
81
+ string_list_clear_func(head, hook_free);
82
free(head);
83
return ret;
84
}
hook.c
+22
-18
@@ -52,8 +52,10 @@ const char *find_hook(struct repository *r, const char *name)
52
return path.buf;
53
}
54
55
-static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
55
+void hook_free(void *p, const char *str UNUSED)
56
{
57
+ struct hook *h = p;
58
+
59
if (!h)
60
return;
61
@@ -64,22 +66,12 @@ static void hook_clear(struct hook *h, hook_data_free_fn cb_data_free)
66
free((void *)h->u.configured.command);
67
}
68
67
- if (cb_data_free)
68
- cb_data_free(h->feed_pipe_cb_data);
69
+ if (h->data_free && h->feed_pipe_cb_data)
70
+ h->data_free(h->feed_pipe_cb_data);
71
72
free(h);
73
}
74
73
-void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free)
74
-{
75
- struct string_list_item *item;
76
-
77
- for_each_string_list_item(item, hooks)
78
- hook_clear(item->util, cb_data_free);
79
-
80
- string_list_clear(hooks, 0);
81
-}
82
-
75
/* Helper to detect and add default "traditional" hooks from the hookdir. */
76
static void list_hooks_add_default(struct repository *r, const char *hookname,
77
struct string_list *hook_list,
@@ -100,9 +92,15 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
92
if (options && options->dir)
93
hook_path = absolute_path(hook_path);
94
103
- /* Setup per-hook internal state cb data */
104
- if (options && options->feed_pipe_cb_data_alloc)
95
+ /*
96
+ * Setup per-hook internal state callback data.
97
+ * When provided, the alloc/free callbacks are always provided
98
+ * together, so use them to alloc/free the internal hook state.
99
+ */
100
+ if (options && options->feed_pipe_cb_data_alloc) {
101
h->feed_pipe_cb_data = options->feed_pipe_cb_data_alloc(options->feed_pipe_ctx);
102
+ h->data_free = options->feed_pipe_cb_data_free;
103
+ }
104
105
h->kind = HOOK_TRADITIONAL;
106
h->u.traditional.path = xstrdup(hook_path);
@@ -316,10 +314,16 @@ static void list_hooks_add_configured(struct repository *r,
314
315
CALLOC_ARRAY(hook, 1);
316
319
- if (options && options->feed_pipe_cb_data_alloc)
317
+ /*
318
+ * When provided, the alloc/free callbacks are always provided
319
+ * together, so use them to alloc/free the internal hook state.
320
+ */
321
+ if (options && options->feed_pipe_cb_data_alloc) {
322
hook->feed_pipe_cb_data =
323
options->feed_pipe_cb_data_alloc(
324
options->feed_pipe_ctx);
325
+ hook->data_free = options->feed_pipe_cb_data_free;
326
+ }
327
328
hook->kind = HOOK_CONFIGURED;
329
hook->u.configured.friendly_name = xstrdup(friendly_name);
@@ -362,7 +366,7 @@ int hook_exists(struct repository *r, const char *name)
366
{
367
struct string_list *hooks = list_hooks(r, name, NULL);
368
int exists = hooks->nr > 0;
365
- hook_list_clear(hooks, NULL);
369
+ string_list_clear_func(hooks, hook_free);
370
free(hooks);
371
return exists;
372
}
@@ -516,7 +520,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name,
520
run_processes_parallel(&opts);
521
ret = cb_data.rc;
522
cleanup:
519
- hook_list_clear(cb_data.hook_command_list, options->feed_pipe_cb_data_free);
523
+ string_list_clear_func(cb_data.hook_command_list, hook_free);
524
free(cb_data.hook_command_list);
525
run_hooks_opt_clear(options);
526
return ret;
hook.h
+14
-6
@@ -7,6 +7,9 @@
7
8
struct repository;
9
10
+typedef void (*hook_data_free_fn)(void *data);
11
+typedef void *(*hook_data_alloc_fn)(void *init_ctx);
12
+
13
/**
14
* Represents a hook command to be run.
15
* Hooks can be:
@@ -41,10 +44,15 @@ struct hook {
44
* Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it.
45
*/
46
void *feed_pipe_cb_data;
44
-};
47
46
-typedef void (*hook_data_free_fn)(void *data);
47
-typedef void *(*hook_data_alloc_fn)(void *init_ctx);
48
+ /**
49
+ * Callback to free `feed_pipe_cb_data`.
50
+ *
51
+ * It is called automatically and points to the `feed_pipe_cb_data_free`
52
+ * provided via the `run_hook_opt` parameter.
53
+ */
54
+ hook_data_free_fn data_free;
55
+};
56
57
struct run_hooks_opt {
58
/* Environment vars to be set for each hook */
@@ -185,10 +193,10 @@ struct string_list *list_hooks(struct repository *r, const char *hookname,
193
struct run_hooks_opt *options);
194
195
/**
188
- * Frees the memory allocated for the hook list, including the `struct hook`
189
- * items and their internal state.
196
+ * Frees a struct hook stored as the util pointer of a string_list_item.
197
+ * Suitable for use as a string_list_clear_func_t callback.
198
*/
191
-void hook_list_clear(struct string_list *hooks, hook_data_free_fn cb_data_free);
199
+void hook_free(void *p, const char *str);
200
201
/**
202
* Frees the hook configuration cache stored in `struct repository`.