Re: [PATCH v2 1/7] run-command: add preliminary support for multiple hooks

From: Johannes Sixt <j6t@kdbg.org>
To: "brian m. carlson" <sandals@crustytoothpaste.net>
Cc: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	git@vger.kernel.org, "Jeff King" <peff@peff.net>,
	"Duy Nguyen" <pclouds@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Jonathan Nieder" <jrnieder@gmail.com>
Subject: Re: [PATCH v2 1/7] run-command: add preliminary support for multiple hooks
Date: Thu, 16 May 2019 21:11:10 +0200	[thread overview]
Message-ID: <91532c2d-7536-6e15-728b-bd18256b2ef3@kdbg.org> (raw)
In-Reply-To: <20190515224425.GK7458@genre.crustytoothpaste.net>

Am 16.05.19 um 00:44 schrieb brian m. carlson:
> On Tue, May 14, 2019 at 05:12:39PM +0200, Johannes Schindelin wrote:
>> On Tue, 14 May 2019, brian m. carlson wrote:
>>> +/*
>>> + * Return 1 if a hook exists at path (which may be modified) using access(2)
>>> + * with check (which should be F_OK or X_OK), 0 otherwise. If strip is true,
>>> + * additionally consider the same filename but with STRIP_EXTENSION added.
>>> + * If check is X_OK, warn if the hook exists but is not executable.
>>> + */
>>> +static int has_hook(struct strbuf *path, int strip, int check)
>>> +{
>>> +	if (access(path->buf, check) < 0) {
>>> +		int err = errno;
>>> +
>>> +		if (strip) {
>>> +#ifdef STRIP_EXTENSION
>>> +			strbuf_addstr(path, STRIP_EXTENSION);
>>> +			if (access(path->buf, check) >= 0)
>>> +				return 1;
>>> +			if (errno == EACCES)
>>> +				err = errno;
>>> +#endif
>>> +		}
>>
>> How about simply guarding the entire `if()`? It is a bit unusual to guard
>> *only* the inside block ;-)
> 
> I can make that change.

But then we'll have an unused argument in some build configurations.

-- Hannes