Re: fprintf_ln() is slow

[Phillip Wood <phillip.wood123@gmail.com>]

Dear Peff and Dscho

On 27/06/2019 22:10, Jeff King wrote:
> On Thu, Jun 27, 2019 at 02:00:54PM +0200, Johannes Schindelin wrote:
> 
>>> We can use setvbuf() to toggle buffering back and forth, but I'm not
>>> sure if there's a way to query the current buffering scheme for a stdio
>>> stream.
>>
>> It also is not very safe, especially when we want to have this work in a
>> multi-threaded fashion.
> 
> I considered that, too, but I think it is safe. stdio has its own
> locking, so every individual call is atomic. The potentially problematic
> case would be where we switch back from line buffering to no-buffering,
> and somebody else has written some content into our stack-based buffer
> (that is about to go out of scope!). But I'd assume that as part of the
> switch to no-buffering that any stdio implementation would flush out the
> buffer that it's detaching from (while under lock). Nothing else makes
> sense.

The C standard section 7.19.5.6 says that
  The setvbuf function may be used only after the stream pointed to by
  a stream has been associated with an open file and before any other
  operation (other than an unsuccessful call to setvbuf) is performed
  on the stream.

The is a note about the buffer that says
  The buffer has to have a lifetime at least as great as the open
  stream, so the stream should be closed before a buffer that has
  automatic storage duration is deallocated upon block exit.

So changing the buffer in the way that has been proposed is undefined
behavior on two counts I think.

Best Wishes

Phillip


> That said...
> 
>> I'd be much more comfortable with rendering the string into a buffer and
>> then sending that buffer wholesale to stderr.
> 
> It's sufficiently complex that I think I prefer to just use our own
> buffer, too.
> 
> It also makes it more likely for the newline and the message to end up
> in an atomic write(), so if multiple threads _are_ writing to stderr
> they'd be more likely to stay together.
> 
> It does sound like people in the other part of the thread are OK with
> just getting rid of the "_ln" functions altogether.
> 
> -Peff
>