Re: [Qemu-block] [Qemu-devel] [PATCH 4/6] hw/block: Fix the return type

[Stefan Hajnoczi]

On Wed, Jul 26, 2017 at 08:02:53PM +0800, Mao Zhongyi wrote:
> When the function no success value to transmit, it usually make the
> function return void. It has turned out not to be a success, because
> it means that the extra local_err variable and error_propagate() will
> be needed. It leads to cumbersome code, therefore, transmit success/
> failure in the return value is worth.
> 
> So fix the return type of blkconf_apply_backend_options(),
> blkconf_geometry() and virtio_blk_data_plane_create() to avoid it.
> 
> Cc: John Snow <address@hidden>
> Cc: Kevin Wolf <address@hidden>
> Cc: Max Reitz <address@hidden>
> Cc: Stefan Hajnoczi <address@hidden>
> 
> Signed-off-by: Mao Zhongyi <address@hidden>
> ---
>  hw/block/block.c                | 21 ++++++++++++---------
>  hw/block/dataplane/virtio-blk.c | 16 +++++++++-------
>  hw/block/dataplane/virtio-blk.h |  6 +++---
>  include/hw/block/block.h        | 10 +++++-----
>  4 files changed, 29 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/block/block.c b/hw/block/block.c
> index 27878d0..717bd0e 100644
> --- a/hw/block/block.c
> +++ b/hw/block/block.c
> @@ -51,8 +51,8 @@ void blkconf_blocksizes(BlockConf *conf)
>      }
>  }
>  
> -void blkconf_apply_backend_options(BlockConf *conf, bool readonly,
> -                                   bool resizable, Error **errp)
> +int blkconf_apply_backend_options(BlockConf *conf, bool readonly,
> +                                  bool resizable, Error **errp)

I'm not a fan of these changes because it makes inconsistencies between
the return value and the errp argument possible (e.g. returning success
but setting errp, or returning failure without setting errp).

If you really want to do this please use bool as the return type instead
of int.  int can be abused to return error information that should
really be in the Error object.

Stefan

signature.asc
Description: PGP signature