On Sat, Jun 10, 2023 at 11:21:20AM +0200, Claudio Jeker wrote:
> On Sat, Jun 10, 2023 at 10:15:53AM +0200, Theo Buehler wrote:
> > On Sat, Jun 10, 2023 at 09:00:54AM +0200, Claudio Jeker wrote:
> > > Instead of building an API for ibufs to handle dynamic strings use
> > > open_memstream(3) which does the same via stdio.
> > >
> > > Now open_memstream() requires a bit more plumbing (one needs to close the
> > > FILE stream and free the buffer) but on the plus side you can use all
> > > stdio functions like fprintf() to fill this string.
> > > While doing this also add error handling and check if the generated string
> > > was created successfully before calling log_info().
> >
> > This is a lot better than the mess that was there before. One thing I'm
> > not entirely sure about is whether fflush() can fail. I think it can't
> > since previous writes should already have led to reallocations, so
> > another ferror() after fflush() would probably be overdoing it.
>
> Indeed, I was under the impression that fflush() would just update the
> pointers but actually the fflush() call is needed to flush out the
> internal stdio buffer into the open_memstream() buffer.
I see, I missed that. Agreed.
> So the fflush()
> should happen before checking for errors. Another option is to use
> setvbuf() with _IONBF to make the FILE stream unbuffered.
I'd rather avoid further complexity.
> So the new pattern is:
> fflush(spif);
> if (ftello(spif) > 0 && !ferror(spif)) {
> log_info(...
Our ftello() calls __sflush() internally, so at least for OpenBSD the
original fflush() call should not be able to fail (and is redundant in
either version). I'm not sure we can rely on that in portable, so I think
your new pattern is the right thing to do.
