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

[Markus Armbruster <armbru@redhat.com>]

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?

>
> 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.

> +    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] = "".

> +}