Re: [Qemu-devel] [PATCH v2 3/4] trace: forbid use of %m in trace event format strings

[Daniel P . Berrangé]

On Tue, Jan 22, 2019 at 11:19:42AM -0600, Eric Blake wrote:
> On 1/22/19 8:32 AM, Daniel P. Berrangé wrote:
> 
> >>> +++ b/hw/vfio/pci.c
> >>> @@ -2581,7 +2581,7 @@ static void vfio_populate_device(VFIOPCIDevice 
> >>> *vdev, Error **errp)
> >>>      ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_IRQ_INFO, &irq_info);
> >>>      if (ret) {
> >>>          /* This can fail for an old kernel or legacy PCI dev */
> >>> -        trace_vfio_populate_device_get_irq_info_failure();
> >>> +        trace_vfio_populate_device_get_irq_info_failure(errno);
> >>
> >> trace_vfio_populate_device_get_irq_info_failure(strerror(errno))
> > 
> > The caveat is that 'strerror' is not required to be thread safe,
> > however, given that this is Linux only code I guess we can assume
> > the glibc impl which fortunately is thread safe.
> 
> If we are going to worry about thread-safety of strerror(), we have a
> LOT of code to scrub (we are using it rather liberally throughout the
> code base).

Yes, indeed we do and it was something that always worried me a little
(as well as a few other non-reentrant APIs we used previously).

> > 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.

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 ?

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 :|