[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/4] trace: forbid use of %m in trace event f
From: |
Daniel P . Berrangé |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/4] trace: forbid use of %m in trace event format strings |
Date: |
Fri, 18 Oct 2019 10:34:43 +0100 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
On Fri, Oct 18, 2019 at 11:31:15AM +0200, Thomas Huth wrote:
> On 22/01/2019 19.10, Eric Blake wrote:
> > On 1/22/19 11:23 AM, Daniel P. Berrangé wrote:
> >
> >>
> >>>> On this point though, does anyone know of any platforms we support[1],
> >>>> or are likely to support in future, where 'strerror' is *not* thread
> >>>> safe ?
> >>>
> >>> I'm not coming up with one, and I think the problem is independent of
> >>> this series (if we DO have a problem, it's a series all its own to
> >>> eradicate the use of strerror() in favor of something safer, either
> >>> picking strerror_l() or dealing with the glibc vs. BSD differences in
> >>> strerror_r()).
> >>
> >> Agree that its not really something for this series - this just
> >> made me think of it again.
> >
> > Shoot - FreeBSD strerror() is not threadsafe:
> > https://github.com/freebsd/freebsd/blob/master/lib/libc/string/strerror.c#L119
> >
> > char *
> > strerror(int num)
> > {
> > static char ebuf[NL_TEXTMAX];
> >
> > if (strerror_r(num, ebuf, sizeof(ebuf)) != 0)
> > errno = EINVAL;
> > return (ebuf);
> > }
> >
> >>
> >> We went through the scrubbing in libvirt to use the sane, but still
> >> tedious to call, variant of strerror_r() many years ago. With luck
> >> though it is a worry that can be confined the dustbin of ancient
> >> UNIX history....unless someone can point to evidence to the contrary ?
> >
> > libvirt has it easy - they let gnulib do all the work of futzing around
> > with getting a working strerror() despite platform bugs and despite
> > glibc's insistence on a non-POSIX signature if _GNU_SOURCE is defined.
> > We'll have to do a bit more legwork.
> >
> > That said, I've added it to:
> > https://wiki.qemu.org/Contribute/BiteSizedTasks#Error_checking
> >
> > if someone wants to do the grunt work.
>
> I think we should change that task to switch to g_strerror() from glib
> instead ... as far as I can see, this is a proper wrapper around
> strerror_r(), so we don't have to deal with the implementation oddities
> of strerror_r() in QEMU.
Yeah, I think using g_strerror() makes sense. We've just adopted that
in libvirt precisely to avoid these platform portability oddities and
the really unpleasant API calling convention of strerror_r().
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|