Re: [PATCH v2 0/6] Force pipes to flush immediately on NonStop platform

[Junio C Hamano <gitster@pobox.com>]

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

> On Fri, Jan 19, 2018 at 12:33:59PM -0500, randall.s.becker@rogers.com wrote:
>> From: "Randall S. Becker" <rsbecker@nexbridge.com>
>> 
>> * wrapper.c: called setbuf(stream,0) to force pipe flushes not enabled by
>>   default on the NonStop platform.
>> 
>> Signed-off-by: Randall S. Becker <rsbecker@nexbridge.com>
>> ---
>>  wrapper.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/wrapper.c b/wrapper.c
>> index d20356a77..671cbb4b4 100644
>> --- a/wrapper.c
>> +++ b/wrapper.c
>> @@ -403,6 +403,9 @@ FILE *xfdopen(int fd, const char *mode)
>>  	FILE *stream = fdopen(fd, mode);
>>  	if (stream == NULL)
>>  		die_errno("Out of memory? fdopen failed");
>> +#ifdef __TANDEM
>> +	setbuf(stream,0);
>> +#endif
>
> Reading the commit message, I would have expected someting similar to
>
> #ifdef FORCE_PIPE_FLUSHES
> 	setbuf(stream,0);
> #endif
>
> (Because other systems may need the tweak as well, some day)
> Of course you need to change that in the Makefile and config.mak.uname

I actually wouldn't have expected anything like that after reading
the commit message.  

First I thought it was describing only what it does (i.e. "let's use
setbuf() to set the stream unbuffered on TANDEM"), which is a
useless description that only says what it does which we can read
from the diff, but "NonStop by default creates pipe that does not
flush" is a potentially useful information the log message adds.
But it is just "potentially"---we cannot read what exact problem the
change is trying to address.  Making a pipe totally unbuffered is a
heavy hammer that may not be an appropriate solution---it could be
that we are missing calls to fflush() where we need and have been
lucky because most of the systems we deal with do line-buffered by
default, or something silly/implausible like that, and if that is
the case, a more proper fix may be to add these missing fflush() to
right places.

IOW, I do not see it explained clearly why this change is needed on
any single platform---so "that issue may be shared by others, too"
is a bit premature thing for me to listen to and understand, as
"that issue" is quite unclear to me.