From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: Re: [PATCH 3/4] libxl/remus: Change the assert for info to an return Date: Mon, 1 Feb 2016 12:13:53 +0000 Message-ID: <20160201121353.GU25660@citrix.com> References: <1453843860-29591-1-git-send-email-konrad.wilk@oracle.com> <1453843860-29591-4-git-send-email-konrad.wilk@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aQDMk-0004LF-Sy for xen-devel@lists.xenproject.org; Mon, 01 Feb 2016 12:13:58 +0000 Content-Disposition: inline In-Reply-To: <1453843860-29591-4-git-send-email-konrad.wilk@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Konrad Rzeszutek Wilk Cc: wei.liu2@citrix.com, ian.campbell@citrix.com, Wen Congyang , Ian.Jackson@eu.citrix.com, xen-devel@lists.xenproject.org, Yang Hongyang List-Id: xen-devel@lists.xenproject.org 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 > CC: Yang Hongyang > Signed-off-by: Konrad Rzeszutek Wilk > --- > 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 >