Re: [PATCH 4/5] libxl: claim: Print the values in 'xl info' unconditionally

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

On Mon, May 13, 2013 at 11:17:31AM +0100, Ian Campbell wrote:
> On Fri, 2013-05-10 at 22:00 +0100, Konrad Rzeszutek Wilk wrote:
> > During the review of "libxl: Change claim_mode from bool to int."
> > Ian Campbell suggested that the xl info should print the
> > claim information irregardless of the global claim_mode value.
> > 
> > Suggested-by: Ian Campbell <Ian.Campbell@citrix.com>
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > ---
> >  tools/libxl/xl_cmdimpl.c | 6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> > 
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index c3e1183..bb7a7af 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -4604,11 +4604,7 @@ static void output_physinfo(void)
> >          printf("sharing_freed_memory   : %"PRIu64"\n", info.sharing_freed_pages / i);
> >          printf("sharing_used_memory    : %"PRIu64"\n", info.sharing_used_frames / i);
> >      }
> > -    /*
> > -     * Only if enabled (claim_mode=1) or there are outstanding claims.
> > -     */
> > -    if (claim_mode || info.outstanding_pages)
> > -        printf("outstanding_claims     : %ld\n", info.outstanding_pages / i);
> > +    printf("outstanding_claims     : %ld\n", info.outstanding_pages / i);
> 
> I here is only initialised within the previous "if (vinfo)" (the tail of
> which is right above).
> 
> This printf should probably therefore have always been inside that same
> block. (the horrible use of the variable i as something other than a
> loop iterator is probably at least partly to blame for the confusion)
> 
> This patch isn't making this any worse so I'll Ack + apply but perhaps
> you could send a follow up to fix this?

Naturally!

> 
> >  
> >      if (!libxl_get_freecpus(ctx, &cpumap)) {
> >          libxl_for_each_bit(i, cpumap)
> 
>