[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 01/10] error: New convenience function error_
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 01/10] error: New convenience function error_report_err() |
Date: |
Thu, 12 Feb 2015 08:26:45 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
On 02/12/2015 06:33 AM, Markus Armbruster wrote:
> I've typed error_report("%s", error_get_pretty(ERR)) too many times
> already, and I've fixed too many instances of qerror_report_err(ERR)
> to error_report("%s", error_get_pretty(ERR)) as well. Capture the
> pattern in a convenience function.
>
> Since it's almost invariably followed by error_free(), stuff that into
> the convenience function as well.
>
> The next patch will put it to use.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> include/qapi/error.h | 5 +++++
> util/error.c | 6 ++++++
> 2 files changed, 11 insertions(+)
> +++ b/util/error.c
> @@ -152,6 +152,12 @@ const char *error_get_pretty(Error *err)
> return err->msg;
> }
>
> +void error_report_err(Error *err)
> +{
> + error_report("%s", error_get_pretty(err));
> + error_free(err);
When I read v1, I wondered if it would make sense to allow:
Error *local_err = NULL;
error_report_err(local_err);
as a no-op, so that calling code can unconditionally use this function
rather than always burying it inside an 'if (problem)'. But in
reviewing the rest of the patches, I wasn't sure it would save very many
lines, and it also seems like it would be a bit more confusing to see a
call to an error report function when there is no error to report.
So in the opposite direction of thought, I wonder if you should add:
assert(err);
and enforce that this function is only ever used on real error messages,
especially since error_get_pretty segfaults if called on no error.
But I can also live without the assert, so:
Reviewed-by: Eric Blake <address@hidden>
--
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/10] Clean up around error_get_pretty(), qerror_report_err(), Markus Armbruster, 2015/02/12
- [Qemu-devel] [PATCH v2 01/10] error: New convenience function error_report_err(), Markus Armbruster, 2015/02/12
- Re: [Qemu-devel] [PATCH v2 01/10] error: New convenience function error_report_err(),
Eric Blake <=
- [Qemu-devel] [PATCH v2 08/10] vl: Avoid qerror_report_err() outside QMP command handlers, Markus Armbruster, 2015/02/12
- [Qemu-devel] [PATCH v2 07/10] tpm: Avoid qerror_report_err() outside QMP command handlers, Markus Armbruster, 2015/02/12
- [Qemu-devel] [PATCH v2 04/10] monitor: Avoid qerror_report_err() outside QMP command handlers, Markus Armbruster, 2015/02/12
- [Qemu-devel] [PATCH v2 03/10] monitor: Clean up around monitor_handle_fd_param(), Markus Armbruster, 2015/02/12
- [Qemu-devel] [PATCH v2 02/10] error: Use error_report_err() where appropriate, Markus Armbruster, 2015/02/12
- [Qemu-devel] [PATCH v2 10/10] qemu-char: Avoid qerror_report_err() outside QMP command handlers, Markus Armbruster, 2015/02/12
- [Qemu-devel] [PATCH v2 06/10] numa: Avoid qerror_report_err() outside QMP command handlers, Markus Armbruster, 2015/02/12
- [Qemu-devel] [PATCH v2 05/10] net: Avoid qerror_report_err() outside QMP command handlers, Markus Armbruster, 2015/02/12