Re: [Qemu-devel] [PATCH v3 3/3] coccinelle: Remove unnecessary variables for function return value

[Markus Armbruster <armbru@redhat.com>]

Eduardo Habkost <ehabkost@redhat.com> writes:

> Use Coccinelle script to replace 'ret = E; return ret' with
> 'return E'. The script will do the substitution only when the
> function return type and variable type are the same.
>
> Sending as RFC because the patch looks more intrusive than the
> others. Probably better to split it per subsystem and let each
> maintainer review and apply it?

I guess you forgot to drop this paragraph.  Can do it on commit to
error-next.

> Manual fixups:
>
> * audio/audio.c: coding style of "read (...)" and "write (...)"
> * block/qcow2-cluster.c: wrap line to make it shorter
> * block/qcow2-refcount.c: change indentation of wrapped line
> * target-tricore/op_helper.c: fix coding style of
>   "remainder|quotient"
> * target-mips/dsp_helper.c: reverted changes because I don't
>   want to argue about checkpatch.pl
> * ui/qemu-pixman.c: fix line indentation
> * block/rbd.c: restore blank line between declarations and
>   statements
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
[...]
> diff --git a/scripts/coccinelle/return_directly.cocci b/scripts/coccinelle/return_directly.cocci
> new file mode 100644
> index 0000000..c52f4fc
> --- /dev/null
> +++ b/scripts/coccinelle/return_directly.cocci
> @@ -0,0 +1,21 @@
> +// replace 'R = X; return R;' with 'return R;'
> +
> +// remove assignment

Second comment feels redundant.  Can drop on commit to error-next.

> +@ removal @

Rule name "removal" is not used.  Can drop on commit to error-next.

> +identifier VAR;
> +expression E;
> +type T;
> +identifier F;
> +@@
> + T F(...)
> + {
> +     ...
> +-    T VAR;
> +     ... when != VAR
> +
> +-    VAR =
> ++    return
> +     E;
> +-    return VAR;
> +     ... when != VAR
> + }
[...]
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index b04bfaf..afc8d6b 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -154,11 +154,8 @@ static int l2_load(BlockDriverState *bs, uint64_t l2_offset,
>      uint64_t **l2_table)
>  {
>      BDRVQcow2State *s = bs->opaque;
> -    int ret;
> -

I'd prefer to keep this blank line.  Can touch up on commit to error-next.

> -    ret = qcow2_cache_get(bs, s->l2_table_cache, l2_offset, (void**) l2_table);
> -
> -    return ret;
> +    return qcow2_cache_get(bs, s->l2_table_cache, l2_offset,
> +                           (void **) l2_table);
>  }
>  
>  /*
[...]