[Top][All Lists]
[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
[Qemu-devel] [PATCH v2 2/7] cutils: Add bytes_to_str() to format byte values, Stefan Hajnoczi, 2010/10/08
[Qemu-devel] [PATCH v2 1/7] qcow2: Make get_bits_from_size() common, Stefan Hajnoczi, 2010/10/08
[Qemu-devel] [PATCH v2 5/7] qed: Table, L2 cache, and cluster functions, Stefan Hajnoczi, 2010/10/08
[Qemu-devel] [PATCH v2 7/7] qed: Consistency check support, Stefan Hajnoczi, 2010/10/08
[Qemu-devel] Re: [PATCH v2 0/7] qed: Add QEMU Enhanced Disk format, Kevin Wolf, 2010/10/11
Re: [Qemu-devel] [PATCH v2 0/7] qed: Add QEMU Enhanced Disk format, Stefan Hajnoczi, 2010/10/16