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
> ---
> 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
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature