Re: [Qemu-devel] [PATCH v5 2/3] utils: provide size_to_str()

[Peter Xu <peterx@redhat.com>]

On Tue, May 09, 2017 at 04:50:26PM +0200, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > I stole the algorithm from print_type_size(). I didn't generalize it
> > since that's using [KM...]iB while here we need [KM...]B to finally
> > be able to stands for page sizes (and even more general).
> 
> Can you explain why we need units without the 'i' here?

Oops. I misunderstood XiB... My fault. Page sizes needs exactly "i".

> 
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  include/qemu-common.h |  1 +
> >  util/cutils.c         | 26 ++++++++++++++++++++++++++
> >  2 files changed, 27 insertions(+)
> >
> > diff --git a/include/qemu-common.h b/include/qemu-common.h
> > index d218821..d7d0448 100644
> > --- a/include/qemu-common.h
> > +++ b/include/qemu-common.h
> > @@ -145,6 +145,7 @@ void qemu_hexdump(const char *buf, FILE *fp, const char *prefix, size_t size);
> >  int parse_debug_env(const char *name, int max, int initial);
> >  
> >  const char *qemu_ether_ntoa(const MACAddr *mac);
> > +char *size_to_str(double val);
> >  void page_size_init(void);
> >  
> >  /* returns non-zero if dump is in progress, otherwise zero is
> > diff --git a/util/cutils.c b/util/cutils.c
> > index 50ad179..5aaf370 100644
> > --- a/util/cutils.c
> > +++ b/util/cutils.c
> > @@ -619,3 +619,29 @@ const char *qemu_ether_ntoa(const MACAddr *mac)
> >  
> >      return ret;
> >  }
> > +
> > +/*
> > + * Sample output:
> > + *
> > + * 1          -> "1 B"
> > + * 528        -> "0.516 KB"
> > + * 4096       -> "4 KB"
> > + * 2402958    -> "2.29 MB"
> > + * 1073741824 -> "1 GB"
> > + *
> > + * Please free the buffer after use.
> > + */
> > +char *size_to_str(double val)
> > +{
> > +    static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T', 'P', 'E' };
> > +    unsigned long div;
> > +    int i;
> > +
> > +    frexp(val, &i);
> 
> The ignored return value is in [0.5,1), and multiplying it by 2^i yields
> val.  i is close to the binary logarithm.
> 
> > +    i /= 10;
> 
> Now it's close to base-1024 logarithm.
> 
> Figuring this out requires too much thought for comfort.  Recommend
> steal the comment from print_type_size(), too.

Let me do it even simpler - I'll just move the whole logic in
print_type_size() into size_to_str(), then I'll refactor
print_type_size().

> 
> > +    assert(i < sizeof(suffixes));
> > +    div = 1ULL << (i * 10);
> > +
> > +    return g_strdup_printf("%0.3g %c%s", val / div,
> > +                           suffixes[i], i ? "B" : "");
> 
> The conditional is a bit confusing.  To avoid it, we could make
> suffixes[] an array of strings, with suffixes[0] = "".

I can fix this. Thanks reviewing!

-- 
Peter Xu