| Message ID | 1286552914-27014-3-git-send-email-stefanha@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
Am 08.10.2010 17:48, schrieb Stefan Hajnoczi: > From: Anthony Liguori <aliguori@us.ibm.com> > > This common function converts byte counts to human-readable strings with > proper units. > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> I wonder how many similar implementations we have in the tree. I know at least of cvtstr() in cmd.c, which is used by qemu-io, but I would be surprised if it was the only one. Adding such a function to cutils.c sounds like a good idea, but we should probably try to change everything to use this common function. Kevin
Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> writes: > From: Anthony Liguori <aliguori@us.ibm.com> > > This common function converts byte counts to human-readable strings with > proper units. > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> > --- > 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);
Am 13.10.2010 11:15, schrieb Markus Armbruster: > Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> writes: > >> From: Anthony Liguori <aliguori@us.ibm.com> >> >> This common function converts byte counts to human-readable strings with >> proper units. >> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> >> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> >> --- >> 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"). Kevin
On 10/08/2010 05:48 PM, Stefan Hajnoczi wrote: > From: Anthony Liguori<aliguori@us.ibm.com> > > This common function converts byte counts to human-readable strings with > proper units. > > Signed-off-by: Anthony Liguori<aliguori@us.ibm.com> > Signed-off-by: Stefan Hajnoczi<stefanha@linux.vnet.ibm.com> > --- > 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) > +{ > + 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); > + } > +} This will show 1.5GB as 1GB. Need either floating point with a couple of digits of precision, or show 1.5GB as 1500MB. It also misuses SI prefixes. 1 GB means 10^9 bytes, not 2^30 bytes (with a common exception for RAM which is usually packaged in 1.125 times some power of two).
On Wed, Oct 13, 2010 at 11:28:42AM +0200, Kevin Wolf wrote: > Am 13.10.2010 11:15, schrieb Markus Armbruster: > > Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> writes: > > > >> From: Anthony Liguori <aliguori@us.ibm.com> > >> > >> This common function converts byte counts to human-readable strings with > >> proper units. > >> > >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > >> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> > >> --- > >> 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
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) +{ + 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); + } +} 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);