[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/9] error: New convenience function error_repor
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 1/9] error: New convenience function error_report_err() |
Date: |
Tue, 10 Feb 2015 10:20:23 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
On 02/10/2015 09:34 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.
>
> @@ -2234,8 +2225,7 @@ static int sd_snapshot_create(BlockDriverState *bs,
> QEMUSnapshotInfo *sn_info)
>
> ret = do_sd_create(s, &new_vid, 1, &local_err);
> if (ret < 0) {
> - error_report("%s", error_get_pretty(local_err));;
> - error_free(local_err);
> + error_report_err(local_err);
> error_report("failed to create inode for snapshot. %s",
> strerror(errno));
Pre-existing bug, so maybe worth a separate patch. This looks fishy:
are we guaranteed that errno is unchanged by error_report_err()? On the
surface, error_vreport() and friends do NOT try to preserve errno; maybe
your new function should guarantee that errno is not clobbered? (in
libvirt, we've explicitly made error-reporting convenience functions
document that they do not clobber errno, because it is much easier for
callers to report an error then return an errno value without having to
save errno locally)
> @@ -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);
> +}
> +
If it were me, I'd split this patch in two, one that introduces the new
function (with no clients), and the other which is a strict Coccinelle
touchup to use it, so that readers don't have to hunt for the meat of
the change.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH 3/9] monitor: Avoid qerror_report_err() outside QMP command handlers, (continued)
- [Qemu-devel] [PATCH 3/9] monitor: Avoid qerror_report_err() outside QMP command handlers, Markus Armbruster, 2015/02/10
- [Qemu-devel] [PATCH 6/9] tpm: Avoid qerror_report_err() outside QMP command handlers, Markus Armbruster, 2015/02/10
- [Qemu-devel] [PATCH 2/9] monitor: Clean up around monitor_handle_fd_param(), Markus Armbruster, 2015/02/10
- [Qemu-devel] [PATCH 1/9] error: New convenience function error_report_err(), Markus Armbruster, 2015/02/10
- Re: [Qemu-devel] [PATCH 1/9] error: New convenience function error_report_err(), Markus Armbruster, 2015/02/11
- Re: [Qemu-devel] [PATCH 1/9] error: New convenience function error_report_err(),
Eric Blake <=
[Qemu-devel] [PATCH 7/9] vl: Avoid qerror_report_err() outside QMP command handlers, Markus Armbruster, 2015/02/10
[Qemu-devel] [PATCH 5/9] numa: Avoid qerror_report_err() outside QMP command handlers, Markus Armbruster, 2015/02/10
[Qemu-devel] [PATCH 9/9] qemu-char: Avoid qerror_report_err() outside QMP command handlers, Markus Armbruster, 2015/02/10
[Qemu-devel] [PATCH 8/9] qemu-img: Avoid qerror_report_err() outside QMP command handlers, Markus Armbruster, 2015/02/10
[Qemu-devel] [PATCH 4/9] net: Avoid qerror_report_err() outside QMP command handlers, Markus Armbruster, 2015/02/10
Re: [Qemu-devel] [PATCH 0/9] Clean up around error_get_pretty(), qerror_report_err(), Eric Blake, 2015/02/11