From: John Snow <jsnow@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: "Thomas Huth" <thuth@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@redhat.com>,
"QEMU Developers" <qemu-devel@nongnu.org>,
"Max Reitz" <mreitz@redhat.com>
Subject: Auditing QEMU to replace NULL with &error_abort
Date: Tue, 22 Jun 2021 11:20:44 -0400 [thread overview]
Message-ID: <7132e6a4-8d3a-828e-1c08-b17954445341@redhat.com> (raw)
One of our Bite-Sized tasks on the wiki was to audit QEMU and, where
applicable, replace NULL with &error_abort.
Everywhere else where it is intentional, we ought to add a comment or
some other indication explaining why it's the right thing to do in that
case.
That task was ported to GitLab here:
https://gitlab.com/qemu-project/qemu/-/issues/414
mreitz and thuth have chimed in with excellent clarifications. Phil
suggests that we should replace the intentional cases of NULL with
&error_ignore, to possibly log squelched errors in debugging mode. This
sounds like a great idea to me:
- It allows us to remove NULL entirely, which as mreitz states "is
fishy", but sometimes valid.
- It annotates callsites where we have decided the ignore is intentional
and correct.
- It gives us a review opportunity to require good comments at those
callsites.
- It gives us a good way to measure progress of the audit by making the
removal of NULL a concrete goal. (Can we use coccinelle to find all
instances of the literal NULL being passed to a variable named errp?)
From a brief chat on IRC, Markus is "reluctant to deviate from GError
even more". It sounds like there isn't consensus here. We should
probably reach consensus on this point before trying to pass the task
off to a neophyte, though -- so I'm raising this discussion on the list
and CC'ing Markus to see if we can define the task better or not.
--js
(Personally, I've got no horse in the race beyond moving these tasks off
the wiki and onto the tracker. Since I moved the issue, though, I might
as well make sure the filing is accurate.)
next reply other threads:[~2021-06-22 15:22 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-22 15:20 John Snow [this message]
2021-06-23 12:16 ` Auditing QEMU to replace NULL with &error_abort Markus Armbruster
2021-06-23 15:01 ` John Snow
2021-06-23 15:08 ` Daniel P. Berrangé
2021-06-23 15:31 ` Marc-André Lureau
2021-06-23 15:39 ` Daniel P. Berrangé
2021-06-23 17:24 ` 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=7132e6a4-8d3a-828e-1c08-b17954445341@redhat.com \
--to=jsnow@redhat.com \
--cc=armbru@redhat.com \
--cc=mreitz@redhat.com \
--cc=philmd@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=thuth@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).