From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH 12/14] libxl: Check xc_sharing_* for proper return values. Date: Wed, 18 Mar 2015 14:03:29 -0400 Message-ID: <20150318180329.GC25199@l.oracle.com> References: <1426520383-20855-1-git-send-email-konrad.wilk@oracle.com> <1426520383-20855-13-git-send-email-konrad.wilk@oracle.com> <1426696600.14291.81.camel@citrix.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 1YYIJg-0006ij-1n for xen-devel@lists.xenproject.org; Wed, 18 Mar 2015 18:03:40 +0000 Content-Disposition: inline In-Reply-To: <1426696600.14291.81.camel@citrix.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: Ian Campbell Cc: xen-devel@lists.xenproject.org, ian.jackson@eu.citrix.com, wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On Wed, Mar 18, 2015 at 04:36:40PM +0000, Ian Campbell wrote: > On Mon, 2015-03-16 at 11:39 -0400, Konrad Rzeszutek Wilk wrote: > > If there is a negative return value - check for that and > > also use errno for the proper error value. > > Was xc_sharing_freed_pages fixed earlier in the series (which is > strictly speaking a bisection hazard, but nevermind) or was the existing > check for l == -E... just buggy? The existing check for ENOSYS was buggy. > > > Signed-off-by: Konrad Rzeszutek Wilk > > --- > > tools/libxl/libxl.c | 4 ++-- > > tools/tests/mem-sharing/memshrtool.c | 12 ++++++++++-- > > 2 files changed, 12 insertions(+), 4 deletions(-) > > > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > > index 94b4d59..99a99e9 100644 > > --- a/tools/libxl/libxl.c > > +++ b/tools/libxl/libxl.c > > @@ -5024,7 +5024,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo) > > physinfo->scrub_pages = xcphysinfo.scrub_pages; > > physinfo->outstanding_pages = xcphysinfo.outstanding_pages; > > l = xc_sharing_freed_pages(ctx->xch); > > - if (l == -ENOSYS) { > > + if (l < 0 && errno == ENOSYS) { > > l = 0; > > } else if (l < 0) { > > LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, l, > > @@ -5033,7 +5033,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo) > > } > > physinfo->sharing_freed_pages = l; > > l = xc_sharing_used_frames(ctx->xch); > > - if (l == -ENOSYS) { > > + if (l < 0 && errno == ENOSYS) { > > l = 0; > > } else if (l < 0) { > > LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, l, > > diff --git a/tools/tests/mem-sharing/memshrtool.c b/tools/tests/mem-sharing/memshrtool.c > > index db44294..d1dd8a2 100644 > > --- a/tools/tests/mem-sharing/memshrtool.c > > +++ b/tools/tests/mem-sharing/memshrtool.c > > @@ -55,11 +55,19 @@ int main(int argc, const char** argv) > > > > if( !strcasecmp(cmd, "info") ) > > { > > + long rc; > > if( argc != 2 ) > > return usage(argv[0]); > > > > - printf("used = %ld\n", xc_sharing_used_frames(xch)); > > - printf("freed = %ld\n", xc_sharing_freed_pages(xch)); > > + rc = xc_sharing_freed_pages(xch); > > + if ( rc < 0 ) > > + return errno; > > Just return 1 please. > > > + > > + printf("used = %ld\n", rc); > > + rc = xc_sharing_used_frames(xch); > > + if ( rc < 0 ) > > + return errno; > > Likewise. > > > + printf("freed = %ld\n", rc); > > } > > else if( !strcasecmp(cmd, "enable") ) > > { > >