[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Paul Eggert
Subject: Re: `message' not outputting the newline "atomically"
Date: Mon, 24 Jun 2019 14:11:31 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0

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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]