All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: qemu-devel@nongnu.org,
	Alex Williamson <alex.williamson@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 3/4] trace: forbid use of %m in trace event format strings
Date: Tue, 22 Jan 2019 17:23:33 +0000	[thread overview]
Message-ID: <20190122172333.GA13143@redhat.com> (raw)
In-Reply-To: <c06dd0a6-166f-a522-1f54-736519076432@redhat.com>

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

  reply	other threads:[~2019-01-22 18:41 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-18 17:30 [Qemu-devel] [PATCH v2 0/4] trace: make systemtap easier to use for simple logging Daniel P. Berrangé
2019-01-18 17:31 ` [Qemu-devel] [PATCH v2 1/4] display: ensure qxl log_buf is a nul terminated string Daniel P. Berrangé
2019-01-18 17:40   ` Eric Blake
2019-01-18 17:42     ` Daniel P. Berrangé
2019-01-21  7:14   ` Gerd Hoffmann
2019-01-21 10:45   ` Stefan Hajnoczi
2019-01-22 14:17     ` Daniel P. Berrangé
2019-01-18 17:31 ` [Qemu-devel] [PATCH v2 2/4] trace: enforce that every trace-events file has a final newline Daniel P. Berrangé
2019-01-18 17:42   ` Eric Blake
2019-01-22 14:43     ` Daniel P. Berrangé
2019-01-21 10:47   ` Stefan Hajnoczi
2019-01-18 17:31 ` [Qemu-devel] [PATCH v2 3/4] trace: forbid use of %m in trace event format strings Daniel P. Berrangé
2019-01-18 17:50   ` Eric Blake
2019-01-18 22:50     ` Alex Williamson
2019-01-22 14:32     ` Daniel P. Berrangé
2019-01-22 17:19       ` Eric Blake
2019-01-22 17:23         ` Daniel P. Berrangé [this message]
2019-01-22 18:10           ` Eric Blake
2019-10-18  9:31             ` Thomas Huth
2019-10-18  9:34               ` Daniel P. Berrangé
2019-01-18 17:31 ` [Qemu-devel] [PATCH v2 4/4] trace: add ability to do simple printf logging via systemtap Daniel P. Berrangé
2019-01-18 18:14   ` Eric Blake
2019-01-21 10:53     ` Stefan Hajnoczi
2019-01-22 14:34       ` Daniel P. Berrangé
2019-01-22 14:39     ` Daniel P. Berrangé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190122172333.GA13143@redhat.com \
    --to=berrange@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=eblake@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.