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

From: Junio C Hamano <[email-masked]>
Date: Tue, 02 Jan 2024 08:29:13 -0800

Torsten Bögershausen <[email-masked]> writes:

>> -	char *cp;
>> +	int 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);
>> +			cp = git_env_bool("GIT_FLUSH", -1);
>> +			if (cp >= 0)
>> +				skip_stdout_flush = (cp == 0);
>>  			else if ((fstat(fileno(stdout), &st) == 0) &&
>>  				 S_ISREG(st.st_mode))
>>  				skip_stdout_flush = 1;
>
> I think that the "cp" variable could be renamed,
> cp is not a "char pointer" any more.

Absolutely.  Very good point.

> However, using the logic from git_env_bool(), it can go away anyway,
> wouldn't it ?

True.

If we are doing clean-ups in this area, we may want to also stop
doing "== 0" comparisons on lines the patch touches while at it.

>   diff --git a/write-or-die.c b/write-or-die.c
>   index 42a2dc73cd..01b042bf12 100644
>   --- a/write-or-die.c
>   +++ b/write-or-die.c
>   @@ -13,21 +13,21 @@
>     * more. So we just ignore that case instead (and hope we get
>     * the right error code on the flush).
>     *
>   + * The flushing can be skipped like this:
>   + * GIT_FLUSH=0 git-rev-list HEAD
>   + *
>     * If the file handle is stdout, and stdout is a file, then skip the
>   - * flush entirely since it's not needed.
>   + * flush as well since it's not needed.
>     */
>    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);
>   +                       if (git_env_bool("GIT_FLUSH", -1) == 0)
>   +                               skip_stdout_flush = 1;
>                           else if ((fstat(fileno(stdout), &st) == 0) &&
>                                    S_ISREG(st.st_mode))
>                                   skip_stdout_flush = 1;