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

From: Junio C Hamano <[email-masked]>
Date: Wed, 03 Jan 2024 11:18:50 -0800

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

>> -			cp = getenv("GIT_FLUSH");
>> -			if (cp)
>> -				skip_stdout_flush = (atoi(cp) == 0);
>> -			else if ((fstat(fileno(stdout), &st) == 0) &&
>> -				 S_ISREG(st.st_mode))
>> -				skip_stdout_flush = 1;
>> -			else
>> -				skip_stdout_flush = 0;
>> +			skip_stdout_flush = git_env_bool("GIT_FLUSH", -1);
>> +			if (skip_stdout_flush < 0) {
>> +				struct stat st;
>> +				if (fstat(fileno(f), &st))
>> +					skip_stdout_flush = 0;
>> +				else
>> +					skip_stdout_flush = S_ISREG(st.st_mode);
>> +			}
>>  		}
>>  		if (skip_stdout_flush && !ferror(f))
>>  			return;
>> --- >8 ---
>
> Thanks for a nice reading - I can not imagine a better version.

Yup, the flow of the logic feels very natural with this version by
making it clear that the case that the default "-1" is returned to
us is where we need to figure out an appropriate value ourselves.
An added bonus is that the scope "struct stat" has is limited to the
absolute minimum.  I like it, too.

Thanks.