Re: [PATCH 07/46] error: Avoid more error_propagate() when error is not used here

[Eric Blake]

On 6/24/20 11:43 AM, Markus Armbruster wrote:
> When all we do with an Error we receive into a local variable is
> propagating to somewhere else, we can just as well receive it there
> right away.  The previous commit did that for simple cases with
> Coccinelle.  Do it for a few more manually.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   blockdev.c     |  5 +----
>   hw/core/numa.c | 44 ++++++++++++++------------------------------
>   qdev-monitor.c | 11 ++++-------
>   3 files changed, 19 insertions(+), 41 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index b66863c42a..73736a4eaf 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1009,13 +1009,10 @@ DriveInfo *drive_new(QemuOpts *all_opts, 
> BlockInterfaceType block_default_type,
>       }
>   
>       /* Actual block device init: Functionality shared with blockdev-add */
> -    blk = blockdev_init(filename, bs_opts, &local_err);
> +    blk = blockdev_init(filename, bs_opts, errp);
>       bs_opts = NULL;
>       if (!blk) {
> -        error_propagate(errp, local_err);
>           goto fail;
> -    } else {
> -        assert(!local_err);

Loses an assertion that blockdev_init() doesn't mis-use errp, but I 
think the goal of your cleanup work has been to make it easier to prove 
any function follows the rules, so the assertion doesn't add much at 
this point.

> +++ b/qdev-monitor.c
> @@ -600,7 +600,6 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
>       const char *driver, *path;
>       DeviceState *dev = NULL;
>       BusState *bus = NULL;
> -    Error *err = NULL;
>       bool hide;
>   
>       driver = qemu_opt_get(opts, "driver");
> @@ -655,15 +654,14 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
>       dev = qdev_new(driver);
>   
>       /* Check whether the hotplug is allowed by the machine */
> -    if (qdev_hotplug && !qdev_hotplug_allowed(dev, &err)) {
> +    if (qdev_hotplug && !qdev_hotplug_allowed(dev, errp)) {
>           /* Error must be set in the machine hook */
> -        assert(err);

Another such case.

>           goto err_del_dev;
>       }
>   
>       if (!bus && qdev_hotplug && !qdev_get_machine_hotplug_handler(dev)) {
>           /* No bus, no machine hotplug handler --> device is not hotpluggable 
> */
> -        error_setg(&err, "Device '%s' can not be hotplugged on this machine",
> +        error_setg(errp, "Device '%s' can not be hotplugged on this machine",

Should we s/can not/cannot/ while touching this?

Reviewed-by: Eric Blake <eblake@redhat.com>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org