Re: [PATCH v2 1/8] hook: add internal state alloc/free callbacks
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 19, 2026, 21:47 UTC
- Message-ID
- <xmqq4inc2yom.fsf@gitster.g>
- In-Reply-To
- <20260218222352.55393-2-adrian.ratiu@collabora.com>
Adrian Ratiu <adrian.ratiu@collabora.com> writes:
Show 7 quoted lines
> + /*
> + * Ensure cb_data copy and free functions are either provided together,
> + * or neither one is provided.
> + */
> + if ((options->feed_pipe_cb_data_alloc && !options->feed_pipe_cb_data_free) ||
> + (!options->feed_pipe_cb_data_alloc && options->feed_pipe_cb_data_free))
> + BUG("feed_pipe_cb_data_alloc and feed_pipe_cb_data_free must be set together");A way to avoid being repetitious may be to say
if (!!options->feed_pipe_cb_data_alloc != !!options->feed_pipe_cb_data_free)
or
if (!!options->feed_pipe_cb_data_alloc ^ !!options->feed_pipe_cb_data_free)
but it (especially the latter) might be a bit too cute for some people's taste.
Show 37 quoted lines
> diff --git a/refs.c b/refs.c
> index d432cfb78f..3c124fab29 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2514,24 +2514,38 @@ static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_
> return 0; /* no more input to feed */
> }
>
> +static void *transaction_feed_cb_data_alloc(void *feed_pipe_ctx UNUSED)
> +{
> + struct transaction_feed_cb_data *data = xmalloc(sizeof(*data));
> + strbuf_init(&data->buf, 0);
> + data->index = 0;
> + return data;
> +}
> +
> +static void transaction_feed_cb_data_free(void *data)
> +{
> + struct transaction_feed_cb_data *d = data;
> + if (!d)
> + return;
> + strbuf_release(&d->buf);
> + free(d);
> +}
> +
> static int run_transaction_hook(struct ref_transaction *transaction,
> const char *state)
> {
> struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
> - struct transaction_feed_cb_data feed_ctx = { 0 };
> int ret = 0;
>
> strvec_push(&opt.args, state);
>
> opt.feed_pipe = transaction_hook_feed_stdin;
> opt.feed_pipe_ctx = transaction;
> - opt.feed_pipe_cb_data = &feed_ctx;So, as the proposed log message promised, our stack lost the callback data, and instead uses an allocated piece of memory pointed by the same opt.feed_pipe_cb_data member as before.
Show 8 quoted lines
> - > - strbuf_init(&feed_ctx.buf, 0); > + opt.feed_pipe_cb_data_alloc = transaction_feed_cb_data_alloc; > + opt.feed_pipe_cb_data_free = transaction_feed_cb_data_free; > > ret = run_hooks_opt(transaction->ref_store->repo, "reference-transaction", &opt); > > - strbuf_release(&feed_ctx.buf);
As the run_hooks_opt() machinery internally manages the feed_pipe_cb_data, starting from the call to the custom alloc function and concliding with the call to the custom free function, this function no longer need to manage resources. Instead, the custom alloc/free functions take care of all the associated resources, like the feed_ctx.buf strbuf, as we have seen earlier.
Nice.
Show 36 quoted lines
> diff --git a/transport.c b/transport.c
> index ecf9e1f21c..1581aa0886 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -1357,21 +1357,36 @@ static int pre_push_hook_feed_stdin(int hook_stdin_fd, void *pp_cb UNUSED, void
> return 0;
> }
>
> +static void *pre_push_hook_data_alloc(void *feed_pipe_ctx)
> +{
> +...
> +}
> +
> +static void pre_push_hook_data_free(void *data)
> +{
> +...
> +}
> +
> static int run_pre_push_hook(struct transport *transport,
> struct ref *remote_refs)
> {
> struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
> - struct feed_pre_push_hook_data data;
> int ret = 0;
>
> strvec_push(&opt.args, transport->remote->name);
> strvec_push(&opt.args, transport->url);
>
> - strbuf_init(&data.buf, 0);
> - data.refs = remote_refs;
> -
> opt.feed_pipe = pre_push_hook_feed_stdin;
> - opt.feed_pipe_cb_data = &data;
> + opt.feed_pipe_ctx = remote_refs;
> + opt.feed_pipe_cb_data_alloc = pre_push_hook_data_alloc;
> + opt.feed_pipe_cb_data_free = pre_push_hook_data_free;The same structure is used here. Nice.