Re: [Xen-devel] [PATCH 3/4] libxl/remus: Change the assert for info to an return

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Jan 26, 2016 at 04:30:59PM -0500, Konrad Rzeszutek Wilk wrote:
> The assert(info) is after quite a lot of manipulations
> on 'info' - which makes the assert pointless because if
> info was NULL it would have crashed earlier.
> 

This sounds sensible.

> Remove it and make it an return. Also since most of the
> error paths are for the same rc, unify them.
> 
> CC: Wen Congyang <wency@xxxxxxxxxxxxxx>
> CC: Yang Hongyang <hongyang.yang@xxxxxxxxxxxx>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> ---
>  tools/libxl/libxl.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 548e2e2..228b241 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -847,13 +847,14 @@ int libxl_domain_remus_start(libxl_ctx *ctx, 
> libxl_domain_remus_info *info,
>  {
>      AO_CREATE(ctx, domid, ao_how);
>      libxl__domain_suspend_state *dss;
> -    int rc;
> +    int rc = ERROR_FAIL;

This violates coding style:

 83   * If the function is to return a libxl error value, `rc' is
 84     used to contain the error code, but it is NOT initialised:
 85             int rc;

>  
>      libxl_domain_type type = libxl__domain_type(gc, domid);
> -    if (type == LIBXL_DOMAIN_TYPE_INVALID) {
> -        rc = ERROR_FAIL;
> +    if (type == LIBXL_DOMAIN_TYPE_INVALID)
> +        goto out;
> +
> +    if (!info)
>          goto out;
> -    }
>  
>      libxl_defbool_setdefault(&info->allow_unsafe, false);
>      libxl_defbool_setdefault(&info->blackhole, false);
> @@ -867,7 +868,6 @@ int libxl_domain_remus_start(libxl_ctx *ctx, 
> libxl_domain_remus_info *info,
>           !libxl_defbool_val(info->diskbuf))) {
>          LOG(ERROR, "Unsafe mode must be enabled to replicate to /dev/null,"
>                     "disable network buffering and disk replication");
> -        rc = ERROR_FAIL;
>          goto out;
>      }
>  
> @@ -883,8 +883,7 @@ int libxl_domain_remus_start(libxl_ctx *ctx, 
> libxl_domain_remus_info *info,
>      dss->debug = 0;
>      dss->remus = info;
>  
> -    assert(info);
> -
> +    rc = 0;
>      /* Point of no return */
>      libxl__remus_setup(egc, dss);
>      return AO_INPROGRESS;
> -- 
> 2.1.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel