Re: [Qemu-devel] [PATCH v2 2/7] cutils: Add bytes_to_str() to format byte values

[Markus Armbruster]

Stefan Hajnoczi <address@hidden> writes:

> From: Anthony Liguori <address@hidden>
>
> This common function converts byte counts to human-readable strings with
> proper units.
>
> Signed-off-by: Anthony Liguori <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>  cutils.c      |   15 +++++++++++++++
>  qemu-common.h |    1 +
>  2 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/cutils.c b/cutils.c
> index 6c32198..5041203 100644
> --- a/cutils.c
> +++ b/cutils.c
> @@ -301,3 +301,18 @@ int get_bits_from_size(size_t size)
>      return __builtin_ctzl(size);
>  #endif
>  }
> +
> +void bytes_to_str(char *buffer, size_t buffer_len, uint64_t size)

Why is the size argument uint64_t and not size_t?

The name bytes_to_str() suggests you're formatting a sequence of bytes.
What about sztostr()?  Matches Jes's strtosz().

> +{
> +    if (size < (1ULL << 10)) {
> +        snprintf(buffer, buffer_len, "%" PRIu64 " byte(s)", size);
> +    } else if (size < (1ULL << 20)) {
> +        snprintf(buffer, buffer_len, "%" PRIu64 " KB(s)", size >> 10);
> +    } else if (size < (1ULL << 30)) {
> +        snprintf(buffer, buffer_len, "%" PRIu64 " MB(s)", size >> 20);
> +    } else if (size < (1ULL << 40)) {
> +        snprintf(buffer, buffer_len, "%" PRIu64 " GB(s)", size >> 30);
> +    } else {
> +        snprintf(buffer, buffer_len, "%" PRIu64 " TB(s)", size >> 40);
> +    }

Sure you want to truncate rather than round?

The "(s)" sure are ugly.  We don't usually add plural-s after a unit: we
write ten milliseconds as 10ms, not 10ms(s).

Suggest to return the length of the resulting string, as returned by
snprintf().

> +}
> diff --git a/qemu-common.h b/qemu-common.h
> index e0ca398..80ae834 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -154,6 +154,7 @@ int qemu_fls(int i);
>  int qemu_fdatasync(int fd);
>  int fcntl_setfl(int fd, int flag);
>  int get_bits_from_size(size_t size);
> +void bytes_to_str(char *buffer, size_t buffer_len, uint64_t size);
>  
>  /* path.c */
>  void init_paths(const char *prefix);