Re: [PATCH 2/2] hooks: fix a TOCTOU in "did we run a hook?" heuristic

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 19, 2022, 00:11 UTC
Message-ID: <xmqq4k4vyata.fsf@gitster.g>
In-Reply-To: <patch-2.2-d01d088073b-20220218T203834Z-avarab@gmail.com>

> -int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...);
> +int run_commit_hook(int editor_is_used, const char *index_file,
> +		    int *invoked_hook, const char *name, ...);
>

Even though my gut feeling tells me that turning the "yes/no" integer into an enum that includes "there was no such hook", "I tried to run it, but it failed to run" [*], "I ran it and it was happy". would be a more viable approach for the longer term, I guess this extra and ad-hoc parameter would be sufficient as a shorter term improvement.

    Side note: optionally "failed to run" may be split into "failed
    to even start (e.g. ENOEXEC)" and "started successfully but
    exited with non-zero status".  There may or may not be callers
    that wants to see them as distinct cases right now, but an
    interface based on returned enum value would be easier to extend
    than having to add a pointer to return variable every time we
    need to know more details.

Thanks.