[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 05/23] error: Improve documentation around er
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 05/23] error: Improve documentation around error_append_hint() |
Date: |
Thu, 17 Dec 2015 09:59:17 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 12/17/2015 09:49 AM, Markus Armbruster wrote:
> While there, tighten its assertion.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> include/qapi/error.h | 19 +++++++++++++++++--
> util/error.c | 2 +-
> 2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/include/qapi/error.h b/include/qapi/error.h
> index b2362a5..048d947 100644
> --- a/include/qapi/error.h
> +++ b/include/qapi/error.h
> @@ -18,6 +18,15 @@
> * Create an error:
> * error_setg(&err, "situation normal, all fouled up");
> *
> + * Create an error and add additional explanation:
> + * error_setg(&err, "invalid quark");
> + * error_append_hint(&err, "Valid quarks are up, down, strange, "
> + * "charm, top, bottom\n");
Do we want to allow/encourage a trailing dot in the hint? I'm fine if
we don't - but once we document its usage, we should probably then be
consistent with what we document; qemu-option.c used a trailing dot,
qdev-monitor.c did not.
> + *
> + * Do *not* contract this to
> + * error_setg(&err, "invalid quark\n"
> + * "Valid quarks are up, down, strange, charm, top, bottom");
> + *
> * Report an error to stderr:
> * error_report_err(err);
> * This frees the error object.
> @@ -26,6 +35,7 @@
> * const char *msg = error_get_pretty(err);
> * do with msg what needs to be done...
> * error_free(err);
> + * Note that this loses hints added with error_append_hint().
> *
> * Handle an error without reporting it (just for completeness):
> * error_free(err);
> @@ -128,6 +138,7 @@ ErrorClass error_get_class(const Error *err);
> * If @errp is anything else, address@hidden must be NULL.
> * The new error's class is ERROR_CLASS_GENERIC_ERROR, and its
> * human-readable error message is made from printf-style @fmt, ...
> + * The resulting message should not contain newlines.
Should we also discourage trailing punctuation?
Should we also mention no need for leading 'error: ' prefix?
> */
> #define error_setg(errp, fmt, ...) \
> error_setg_internal((errp), __FILE__, __LINE__, __func__, \
> @@ -184,7 +195,11 @@ void error_propagate(Error **dst_errp, Error *local_err);
>
> /**
> * Append a printf-style human-readable explanation to an existing error.
> - * May be called multiple times, and safe if @errp is NULL.
> + * @errp may be NULL, but neither &error_fatal nor &error_abort.
As a native speaker, 'not' sounds better than 'neither' in that
sentence. But I think both choices are grammatically correct.
> + * Trivially the case if you call it only after error_setg() or
> + * error_propagate().
> + * May be called multiple times. The resulting hint should end with a
> + * newline.
> */
> void error_append_hint(Error **errp, const char *fmt, ...)
> GCC_FMT_ATTR(2, 3);
> @@ -218,7 +233,7 @@ void error_free_or_abort(Error **errp);
> /*
> * Convenience function to error_report() and free @err.
> */
> -void error_report_err(Error *);
> +void error_report_err(Error *err);
>
> /*
> * Just like error_setg(), except you get to specify the error class.
> diff --git a/util/error.c b/util/error.c
> index 9b27c45..ebfb74b 100644
> --- a/util/error.c
> +++ b/util/error.c
> @@ -132,7 +132,7 @@ void error_append_hint(Error **errp, const char *fmt, ...)
> return;
> }
> err = *errp;
> - assert(err && errp != &error_abort);
> + assert(err && errp != &error_abort && errp != &error_fatal);
Otherwise looks reasonable.
>
> if (!err->hint) {
> err->hint = g_string_new(NULL);
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2 00/23] Error reporting cleanups and fixes, Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 03/23] error: Use error_report_err() instead of monitor_printf(), Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 02/23] error: Use error_report_err() where appropriate (again), Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 16/23] error: Strip trailing '\n' from error string arguments (again), Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 06/23] block: Clean up "Could not create temporary overlay" error message, Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 05/23] error: Improve documentation around error_append_hint(), Markus Armbruster, 2015/12/17
- Re: [Qemu-devel] [PATCH v2 05/23] error: Improve documentation around error_append_hint(),
Eric Blake <=
- Re: [Qemu-devel] [PATCH v2 05/23] error: Improve documentation around error_append_hint(), Markus Armbruster, 2015/12/17
- Re: [Qemu-devel] [PATCH v2 05/23] error: Improve documentation around error_append_hint(), Eric Blake, 2015/12/17
- Re: [Qemu-devel] [PATCH v2 05/23] error: Improve documentation around error_append_hint(), Markus Armbruster, 2015/12/17
- Re: [Qemu-devel] [PATCH v2 05/23] error: Improve documentation around error_append_hint(), Eric Blake, 2015/12/17
- Re: [Qemu-devel] [PATCH v2 05/23] error: Improve documentation around error_append_hint(), Markus Armbruster, 2015/12/18
[Qemu-devel] [PATCH v2 07/23] qemu-nbd: Clean up "Failed to load snapshot" error message, Markus Armbruster, 2015/12/17
[Qemu-devel] [PATCH v2 12/23] error: Use error_prepend() where it makes obvious sense, Markus Armbruster, 2015/12/17
[Qemu-devel] [PATCH v2 11/23] error: Use error_reportf_err() where it makes obvious sense, Markus Armbruster, 2015/12/17