From: Markus Armbruster <armbru@redhat.com>
To: "Daniel P. Berrange" <berrange@redhat.com>
Cc: Stefan Hajnoczi <stefanha@gmail.com>,
Thomas Huth <thuth@redhat.com>,
qemu-devel@nongnu.org,
"Dr . David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 2/2] doc: Introduce coding style for errors
Date: Tue, 24 Nov 2015 08:30:51 +0100 [thread overview]
Message-ID: <871tbfc0r8.fsf@blackfin.pond.sub.org> (raw)
In-Reply-To: <20151123203654.GY18085@redhat.com> (Daniel P. Berrange's message of "Mon, 23 Nov 2015 20:36:54 +0000")
"Daniel P. Berrange" <berrange@redhat.com> writes:
> On Mon, Nov 23, 2015 at 09:05:30PM +0100, Lluís Vilanova wrote:
>> Daniel P Berrange writes:
>> [...]
>> > I don't think this "Errors in user inputs" vs "Other errors" distinction
>> > really makes sense. Whether an error raised in a piece of code is related
>> > to user input or not is almost impossible to determine in practice. So as
>> > a rule to follow it is not practical.
>>
>> > AFAIK, include/qemu/error-report.h is the historical failed experiment
>> > in structured error reporting, while include/qapi/error.h is the new
>> > preferred error reporting system that everything should be using.
>>
>> > On this basis, I'd simply say that include/qemu/error-report.h is
>> > legacy code that should no longer be used, and that new code should
>> > use include/qapi/error.h exclusively and existing code converted
>> > where practical.
>>
>> Mmmm, I've just reviewed both headers and you sound partially right.
>>
>> AFAIU, "qemu/error-report.h" contains the additional logic to manage "input
>> locations", not present anywhere else. Also, you state that only the reporting
>> functions in "qemu/error.h" should be used.
>>
>> Since "qemu/error.h" internally uses 'error_report()' (from
>> "qemu/error-report.h"), it includes the input location information
>> (if any). So,
>> I will simply refer to "qemu/error.h" for the general reporting
>> functions, plus
>> the location management functions in "qemu/error-report.h".
>
> I don't think the location management functions need to be pointed
> out as broadly speaking, no patches ever need to use them. It should
> be sufficient to just describe the new error reporting functions
> in no new code will ever want to use them in general. Really we
> only need document the qapi/error.h functions and tell people not
> to use anything else
People *will* need to use error-report.h to report errors to stderr or
HMP monitor in code wherever such reporting is appropriate.
Good examples include handling command line options, HMP command
handlers.
A not so good example would be device emulation code; there we should
arguably use a logging interface instead, which we don't have.
Counter-examples are a QMP command handler, and pretty much any other
code that's expected to pass on its errors for somebody else to handle.
Locations are mostly automatic, but there are cases where you can
improve error message quality by setting the right location. Underused.
A few examples are visible in grep loc_pop().
next prev parent reply other threads:[~2015-11-24 7:31 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-23 18:41 [Qemu-devel] [RFC][PATCH v2 0/2] utils: Improve and document error reporting Lluís Vilanova
2015-11-23 18:41 ` [Qemu-devel] [PATCH v2 1/2] utils: Add warning messages Lluís Vilanova
2015-11-26 10:41 ` Markus Armbruster
2015-12-29 19:31 ` Lluís Vilanova
2015-11-23 18:41 ` [Qemu-devel] [PATCH v2 2/2] doc: Introduce coding style for errors Lluís Vilanova
2015-11-23 18:51 ` Daniel P. Berrange
2015-11-23 20:05 ` Lluís Vilanova
2015-11-23 20:36 ` Daniel P. Berrange
2015-11-24 7:30 ` Markus Armbruster [this message]
2015-11-24 7:20 ` Markus Armbruster
2015-11-24 15:04 ` Lluís Vilanova
2015-11-24 15:59 ` Markus Armbruster
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=871tbfc0r8.fsf@blackfin.pond.sub.org \
--to=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=dgilbert@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@gmail.com \
--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 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.