Re: [PATCH v2] write-or-die: make GIT_FLUSH a Boolean environment variable

From: Patrick Steinhardt <ps@pks.im>
Date: Jan 3, 2024, 08:22 UTC
Message-ID: <ZZUZNQqDTx3bnveJ@tanuki>
In-Reply-To: <pull.1628.v2.git.1704268708720.gitgitgadget@gmail.com>

On Wed, Jan 03, 2024 at 07:58:28AM +0000, Chandra Pratap via GitGitGadget wrote: [snip]

Show 19 quoted lines

> diff --git a/write-or-die.c b/write-or-die.c
> index 42a2dc73cd3..a6acabd329f 100644
> --- a/write-or-die.c
> +++ b/write-or-die.c
> @@ -20,15 +20,12 @@ void maybe_flush_or_die(FILE *f, const char *desc)
>  {
>  	static int skip_stdout_flush = -1;
>  	struct stat st;
> -	char *cp;
>  
>  	if (f == stdout) {
>  		if (skip_stdout_flush < 0) {
> -			/* NEEDSWORK: make this a normal Boolean */
> -			cp = getenv("GIT_FLUSH");
> -			if (cp)
> -				skip_stdout_flush = (atoi(cp) == 0);
> -			else if ((fstat(fileno(stdout), &st) == 0) &&
> +			if (!git_env_bool("GIT_FLUSH", -1))
> +				skip_stdout_flush = 1;

It's a bit surprising to pass `-1` as default value to `git_env_bool()` here, as this value would hint that the caller wants to explicitly handle the case where the "GIT_FLUSH" envvar is not set at all. We don't though, and essentially fall back to "GIT_FLUSH=1", so passing `1` as the fallback value would be less confusing.

Anyway, the resulting behaviour is the same regardless of whether we pass `1` or `-1`, so I'm not sure whether this is worth a reroll.

Patrick

Show 9 quoted lines

> +			else if (!fstat(fileno(stdout), &st) &&
>  				 S_ISREG(st.st_mode))
>  				skip_stdout_flush = 1;
>  			else
> 
> base-commit: 1a87c842ece327d03d08096395969aca5e0a6996
> -- 
> gitgitgadget
>