qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH v2 2/7] cutils: Add bytes_to_str() to format byte values
Date: Wed, 13 Oct 2010 11:58:40 +0100
User-agent: Mutt/1.5.20 (2009-06-14)

On Wed, Oct 13, 2010 at 11:28:42AM +0200, Kevin Wolf wrote:
> Am 13.10.2010 11:15, schrieb 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?
> 
> size_t would be rather small for disk images on 32 bit hosts.
> 
> > 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).
> 
> I suggest taking the output format from cvtstr in cmd.c, so that qemu-io
> output stays the same when switching to a common function (it says "10
> KiB" and "1 bytes").

I have a patch to replace bytes_to_str() with cvtstr().  We should
probably rename cvtstr() to sztostr() as suggested because that name is
more descriptive.

Stefan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]