On Thu, May 11, 2017 at 02:05:26PM +0100, Dr. David Alan Gilbert wrote: > * Peter Xu ([email protected]) wrote: > > Moving the algorithm from print_type_size() into size_to_str() so that > > other component can also leverage it. With that, refactor > > print_type_size(). > > > > Reviewed-by: Markus Armbruster <[email protected]> > > Signed-off-by: Peter Xu <[email protected]> > > --- > > include/qemu-common.h | 1 + > > qapi/string-output-visitor.c | 22 ++++++---------------- > > util/cutils.c | 26 ++++++++++++++++++++++++++ > > 3 files changed, 33 insertions(+), 16 deletions(-) > > > > 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/qapi/string-output-visitor.c b/qapi/string-output-visitor.c > > index 94ac821..53c2175 100644 > > --- a/qapi/string-output-visitor.c > > +++ b/qapi/string-output-visitor.c > > @@ -211,10 +211,8 @@ static void print_type_size(Visitor *v, const char > > *name, uint64_t *obj, > > Error **errp) > > { > > StringOutputVisitor *sov = to_sov(v); > > - static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T', 'P', 'E' }; > > - uint64_t div, val; > > - char *out; > > - int i; > > + uint64_t val; > > + char *out, *psize; > > > > if (!sov->human) { > > out = g_strdup_printf("%"PRIu64, *obj); > > @@ -223,19 +221,11 @@ static void print_type_size(Visitor *v, const char > > *name, uint64_t *obj, > > } > > > > val = *obj; > > - > > - /* The exponent (returned in i) minus one gives us > > - * floor(log2(val * 1024 / 1000). The correction makes us > > - * switch to the higher power when the integer part is >= 1000. > > - */ > > - frexp(val / (1000.0 / 1024.0), &i); > > - i = (i - 1) / 10; > > - assert(i < ARRAY_SIZE(suffixes)); > > - div = 1ULL << (i * 10); > > - > > - out = g_strdup_printf("%"PRIu64" (%0.3g %c%s)", val, > > - (double)val/div, suffixes[i], i ? "iB" : ""); > > + psize = size_to_str(val); > > + out = g_strdup_printf("%"PRIu64" (%s)", val, psize); > > string_output_set(sov, out); > > + > > + g_free(psize); > > } > > > > static void print_type_bool(Visitor *v, const char *name, bool *obj, > > diff --git a/util/cutils.c b/util/cutils.c > > index 50ad179..fa5ddec 100644 > > --- a/util/cutils.c > > +++ b/util/cutils.c > > @@ -619,3 +619,29 @@ const char *qemu_ether_ntoa(const MACAddr *mac) > > > > return ret; > > } > > + > > +/* > > + * Return human readable string for size @val. > > + * @val must be between (-1000Eib, 1000EiB), exclusively. > > + * Use IEC binary units like KiB, MiB, and so forth. > > + * Caller is responsible for passing it to g_free(). > > + */ > > +char *size_to_str(double val) > > +{ > > + static const char *suffixes[] = { "", "Ki", "Mi", "Gi", "Ti", "Pi", > > "Ei" }; > > + unsigned long div; > > + int i; > > + > > + /* > > + * The exponent (returned in i) minus one gives us > > + * floor(log2(val * 1024 / 1000). The correction makes us > > + * switch to the higher power when the integer part is >= 1000. > > + * (see e41b509d68afb1f for more info) > > + */ > > + frexp(val / (1000.0 / 1024.0), &i); > > + i = (i - 1) / 10; > > + assert(i < ARRAY_SIZE(suffixes)); > > Because your code takes a double, where as Paolo's only takes uint's, > I think you have to be careful to check that i >= 0 as well, for > example, what would happen if someone did size_to_str(1.0/100.0) ?
Yes, you are right. Let me stick to uint64_t then, after all double helps little in dump sizes... /me chose an unwise parameter type. And then it's safe to directly remove the assert() in this patch, because even UINT64_MAX is only 16 EiB. Then I can further drop the last patch... I'll resend. Thanks, -- Peter Xu
