From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 12/14] libxl: Check xc_sharing_* for proper return values. Date: Wed, 18 Mar 2015 16:36:40 +0000 Message-ID: <1426696600.14291.81.camel@citrix.com> References: <1426520383-20855-1-git-send-email-konrad.wilk@oracle.com> <1426520383-20855-13-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.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YYH8S-0003wK-C8 for xen-devel@lists.xenproject.org; Wed, 18 Mar 2015 16:48:00 +0000 In-Reply-To: <1426520383-20855-13-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: 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 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? > 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") ) > {