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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 3, 2024, 19:18 UTC
Message-ID: <xmqq5y0a9qed.fsf@gitster.g>
In-Reply-To: <20240103184203.GA4334@tb-raspi4>

Torsten Bögershausen <tboegi@web.de> writes:

Show 22 quoted lines

>> -			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.