Re: [Qemu-devel] [PATCH v2 12/23] error: Use error_prepend() where it makes obvious sense

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2908 bytes --]

On 12/17/2015 09:49 AM, Markus Armbruster wrote:
> Done with this Coccinelle semantic patch
> 
>     @@
>     expression FMT, E1, E2;
>     expression list ARGS;
>     @@
>     -    error_setg(E1, FMT, ARGS, error_get_pretty(E2));
>     +    error_propagate(E1, E2);/*###*/
>     +    error_prepend(E1, FMT/*@@@*/, ARGS);
> 
> followed by manual cleanup, first because I can't figure out how to
> make Coccinelle transform strings, and second to get rid of now
> superfluous error_propagate().

Trying to understand this...

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

> +++ b/block/qed.c
> @@ -1611,9 +1611,8 @@ static void bdrv_qed_invalidate_cache(BlockDriverState *bs, Error **errp)
>      memset(s, 0, sizeof(BDRVQEDState));
>      ret = bdrv_qed_open(bs, NULL, bs->open_flags, &local_err);
>      if (local_err) {
> -        error_setg(errp, "Could not reopen qed layer: %s",
> -                   error_get_pretty(local_err));
> -        error_free(local_err);
> +        error_propagate(errp, local_err);
> +        error_prepend(errp, "Could not reopen qed layer: ");

...error_propagate() was not always superfluous...

>          return;
>      } else if (ret < 0) {
>          error_setg_errno(errp, -ret, "Could not reopen qed layer");
> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> index c57f293..df77ba8 100644
> --- a/hw/block/dataplane/virtio-blk.c
> +++ b/hw/block/dataplane/virtio-blk.c
> @@ -142,7 +142,6 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
>                                    Error **errp)
>  {
>      VirtIOBlockDataPlane *s;
> -    Error *local_err = NULL;
>      BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
>      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>  
> @@ -163,11 +162,8 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
>      /* If dataplane is (re-)enabled while the guest is running there could be
>       * block jobs that can conflict.
>       */
> -    if (blk_op_is_blocked(conf->conf.blk, BLOCK_OP_TYPE_DATAPLANE,
> -                          &local_err)) {
> -        error_setg(errp, "cannot start dataplane thread: %s",
> -                   error_get_pretty(local_err));
> -        error_free(local_err);
> +    if (blk_op_is_blocked(conf->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) {
> +        error_prepend(errp, "cannot start dataplane thread: ");

...but when the only use of a local error was just to rewrite its
message, then the local error variable and error_propagate() call can
just go away instead.  Okay, makes sense now, and no, I don't know how
to make Coccinelle do it either.

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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]