Re: [PATCH 12/14] libxl: Check xc_sharing_* for proper return values.

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

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 <konrad.wilk@oracle.com>
> > ---
> >  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") )
> >      {
> 
>