Re: [Qemu-devel] [PATCH v2 2/3] error: Remove unnecessary local_err variables

[Eric Blake <eblake@redhat.com>]

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

On 06/10/2016 02:12 PM, Eduardo Habkost wrote:
> This patch simplifies code that uses a local_err variable just to
> immediately use it for an error_propagate() call.
> 
> Coccinelle patch used to perform the changes added to
> scripts/coccinelle/remove_local_err.cocci.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  block.c                                   |  8 ++------
>  block/raw-posix.c                         |  8 ++------
>  block/raw_bsd.c                           |  4 +---
>  blockdev.c                                | 16 +++++-----------
>  hw/s390x/s390-virtio-ccw.c                |  5 +----
>  hw/s390x/virtio-ccw.c                     | 28 +++++++---------------------
>  scripts/coccinelle/remove_local_err.cocci | 27 +++++++++++++++++++++++++++
>  target-i386/cpu.c                         |  4 +---
>  8 files changed, 46 insertions(+), 54 deletions(-)
>  create mode 100644 scripts/coccinelle/remove_local_err.cocci
> 

> +++ b/block.c
> @@ -294,14 +294,12 @@ typedef struct CreateCo {
>  
>  static void coroutine_fn bdrv_create_co_entry(void *opaque)
>  {
> -    Error *local_err = NULL;
>      int ret;
>  
>      CreateCo *cco = opaque;
>      assert(cco->drv);
>  
> -    ret = cco->drv->bdrv_create(cco->filename, cco->opts, &local_err);
> -    error_propagate(&cco->err, local_err);
> +    ret = cco->drv->bdrv_create(cco->filename, cco->opts, &cco->err);
>      cco->ret = ret;

This hunk doesn't get simplified by 3/3; you may want to consider a
manual followup to drop 'int ret' and just assign
cco->drv->bdrv_create() directly to cco->ret.  But doesn't change this
patch.


> +++ b/blockdev.c
> @@ -3654,7 +3654,6 @@ void qmp_blockdev_mirror(const char *device, const char *target,
>      BlockBackend *blk;
>      BlockDriverState *target_bs;
>      AioContext *aio_context;
> -    Error *local_err = NULL;
>  
>      blk = blk_by_name(device);
>      if (!blk) {
> @@ -3678,16 +3677,11 @@ void qmp_blockdev_mirror(const char *device, const char *target,
>  
>      bdrv_set_aio_context(target_bs, aio_context);
>  
> -    blockdev_mirror_common(bs, target_bs,
> -                           has_replaces, replaces, sync,
> -                           has_speed, speed,
> -                           has_granularity, granularity,
> -                           has_buf_size, buf_size,
> -                           has_on_source_error, on_source_error,
> -                           has_on_target_error, on_target_error,
> -                           true, true,
> -                           &local_err);
> -    error_propagate(errp, local_err);
> +    blockdev_mirror_common(bs, target_bs, has_replaces, replaces, sync,
> +                           has_speed, speed, has_granularity, granularity,
> +                           has_buf_size, buf_size, has_on_source_error,
> +                           on_source_error, has_on_target_error,
> +                           on_target_error, true, true, errp);

Coccinelle messes a bit with the formatting (the old way explicitly
tried to pair related has_foo with foo). But I'm going to mess with it
again with my qapi patches for passing a boxed parameter rather than
lots of arguments, so don't worry about it.

> +++ b/scripts/coccinelle/remove_local_err.cocci
> @@ -0,0 +1,27 @@
> +// Replace unnecessary usage of local_err variable with
> +// direct usage of errp argument
> +
> +@@
> +expression list ARGS;
> +expression F2;
> +identifier LOCAL_ERR;
> +expression ERRP;
> +idexpression V;
> +typedef Error;
> +expression I;
> +@@
> + {
> +     ...
> +-    Error *LOCAL_ERR;
> +     ... when != LOCAL_ERR
> +(
> +-    F2(ARGS, &LOCAL_ERR);
> +-    error_propagate(ERRP, LOCAL_ERR);
> ++    F2(ARGS, ERRP);
> +|
> +-    V = F2(ARGS, &LOCAL_ERR);
> +-    error_propagate(ERRP, LOCAL_ERR);
> ++    V = F2(ARGS, ERRP);
> +)
> +     ... when != LOCAL_ERR
> + }

Looks good.
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 --]