On Tue, May 09, 2017 at 04:50:26PM +0200, Markus Armbruster wrote:
> Peter Xu <pet...@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 <pet...@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
