[PATCH v2][next] hook: check for NULL pointer before deref

[PATCH v2][next] hook: check for NULL pointer before deref

[Adrian Ratiu]

Fix a compiler warning (-Werror=analyzer-deref-before-check) due to
dereferencing the options pointer before NULL checking it.

In practice run_hooks_opt() is never called with a NULL opt struct,
so this just fixes the code to not trigger the warning anymore.

The NULL check is kept as-is because some future patches might end up
calling run_hooks_opt with a NULL opt struct, which is clearly a bug.

While at it, also fix the BUG message function name.

Reported-by: correctmost <cmlists@sent.com>
Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
This is based on next, because it fixes a warning introduced in next.

Changes in v2:
* Applied fix to two additional dereferences (Patrick)

Succesful CI run: https://github.com/10ne1/git/actions/runs/20859773595
---
 hook.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hook.c b/hook.c
index 35211e5ed7..6b8ddfe7b6 100644
--- a/hook.c
+++ b/hook.c
@@ -148,28 +148,29 @@ int run_hooks_opt(struct repository *r, const char *hook_name,
 	};
 	const char *const hook_path = find_hook(r, hook_name);
 	int ret = 0;
-	const struct run_process_parallel_opts opts = {
+	struct run_process_parallel_opts opts = {
 		.tr2_category = "hook",
 		.tr2_label = hook_name,
 
 		.processes = 1,
-		.ungroup = options->ungroup,
 
 		.get_next_task = pick_next_hook,
 		.start_failure = notify_start_failure,
-		.feed_pipe = options->feed_pipe,
-		.consume_output = options->consume_output,
 		.task_finished = notify_hook_finished,
 
 		.data = &cb_data,
 	};
 
 	if (!options)
-		BUG("a struct run_hooks_opt must be provided to run_hooks");
+		BUG("a struct run_hooks_opt must be provided to run_hooks_opt");
 
 	if (options->path_to_stdin && options->feed_pipe)
 		BUG("options path_to_stdin and feed_pipe are mutually exclusive");
 
+	opts.ungroup = options->ungroup;
+	opts.feed_pipe = options->feed_pipe;
+	opts.consume_output = options->consume_output;
+
 	if (options->invoked_hook)
 		*options->invoked_hook = 0;
 
-- 
2.51.2

Re: [PATCH v2][next] hook: check for NULL pointer before deref

[Junio C Hamano]

Adrian Ratiu <adrian.ratiu@collabora.com> writes:

> Fix a compiler warning (-Werror=analyzer-deref-before-check) due to
> dereferencing the options pointer before NULL checking it.
>
> In practice run_hooks_opt() is never called with a NULL opt struct,
> so this just fixes the code to not trigger the warning anymore.
>
> The NULL check is kept as-is because some future patches might end up
> calling run_hooks_opt with a NULL opt struct, which is clearly a bug.
>
> While at it, also fix the BUG message function name.
>
> Reported-by: correctmost <cmlists@sent.com>
> Suggested-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
> This is based on next, because it fixes a warning introduced in next.

Actually the problematic topic has been in 'master' for a few days
already, isn't it?  I'd apply directly on top of ar/run-comand-hook
topic.

Thanks.

Re: [PATCH v2][next] hook: check for NULL pointer before deref

[Adrian Ratiu]

On Fri, 09 Jan 2026, Junio C Hamano <gitster@pobox.com> wrote:
> Adrian Ratiu <adrian.ratiu@collabora.com> writes:
>
>> Fix a compiler warning (-Werror=analyzer-deref-before-check) due to
>> dereferencing the options pointer before NULL checking it.
>>
>> In practice run_hooks_opt() is never called with a NULL opt struct,
>> so this just fixes the code to not trigger the warning anymore.
>>
>> The NULL check is kept as-is because some future patches might end up
>> calling run_hooks_opt with a NULL opt struct, which is clearly a bug.
>>
>> While at it, also fix the BUG message function name.
>>
>> Reported-by: correctmost <cmlists@sent.com>
>> Suggested-by: Patrick Steinhardt <ps@pks.im>
>> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
>> ---
>> This is based on next, because it fixes a warning introduced in next.
>
> Actually the problematic topic has been in 'master' for a few days
> already, isn't it?  I'd apply directly on top of ar/run-comand-hook
> topic.

Yes, that is correct, the commits landed in master so this patch applies
cleanly there as well (just double checked to confirm). Thanks!