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 :|
next prev parent 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).