Re: `message' not outputting the newline "atomically"

[Paul Eggert <eggert@cs.ucla.edu>]

On 6/24/19 1:17 PM, Lars Ingebrigtsen wrote:
>> I think the fflush() here can be dropped, stderr is not buffered.
> Probably, but Emacs is being built on a large number of systems.  Is
> stderr unbuffered on all of them?

It's safe to drop the fflush, since stderr isn't fully-buffered on any 
system and we should remove cargo-cult code that gets in the way of 
maintenance. But there are more-important problems with the patch.

* The patch also needs a FIXME comment saying it fixes only "message" 
output, not the other uses of stderr in Emacs.

* The patched code has undefined behavior if the string length is 
INT_MAX, and messes up in other ways if the string length exceeds 
INT_MAX. This bug is unlikely but should be fixed.

* The second and third arguments to fwrite should be interchanged. This 
makes a difference on some non-POSIX platforms, and we might as well do 
it right here.

* More important, the patched code shouldn't call xmalloc. Having to 
allocate a buffer as part of an error diagnostic is a recipe for 
trouble. (Suppose the diagnostic is related to being low on memory?) 
Instead, the patched code should just use a fixed-size buffer that is 
guaranteed to exist and be big enough. This is a basic design principle 
for error diagnostics (I vaguely recall Dijkstra did this back in the 
1960s).

* The buffer should contain only PIPE_BUF bytes, since writes larger 
than that can be split by the operating system anyway so any excess size 
is wasted. On POSIX platforms you can use fpathconf to calculate 
PIPE_BUF for stderr. I don't know how to calculate it on MS-Windows 
platforms, but maybe it doesn't matter and you can just pretend it's 
1024 or whatever.

* Larger strings can be be output PIPE_BUF bytes at a time. You'll need 
a loop for this of course, and the loop should do the right thing if one 
of the earlier fwrites fail.

* Better yet, fix the code so that it doesn't need to copy the string 
into an extra buffer at all. Admittedly this will be more work, but it's 
what the code really should be doing anyway.


When I originally looked into this problem, I drafted a patch along the 
lines you're suggesting. But it's a lot of effort to get it right, and 
it's effort that we should be spending elsewhere, not here. This is why 
the three-line patch that I already gave is way better than what you're 
proposing.