[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 07/12] qapi: add human mode to StringOutputVisit
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 07/12] qapi: add human mode to StringOutputVisitor |
Date: |
Thu, 30 Jan 2014 07:09:59 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 |
On 01/30/2014 06:09 AM, Paolo Bonzini wrote:
> This will be used by "info qtree". For numbers it prints both the
> decimal and hex values. For sizes it rounds to the nearest power
> of 2^10. For strings, it puts quotes around the string and separates
> NULL and empty string.
Nice idea! But needs a v2.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> include/qapi/string-output-visitor.h | 2 +-
> include/qom/object.h | 3 +-
> qapi/string-output-visitor.c | 55
> ++++++++++++++++++++++++++++++++++--
> qdev-monitor.c | 2 +-
> qom/object.c | 4 +--
> tests/test-string-output-visitor.c | 2 +-
> tests/test-visitor-serialization.c | 2 +-
> 7 files changed, 60 insertions(+), 10 deletions(-)
>
> +static void print_type_size(Visitor *v, uint64_t *obj, const char *name,
> + Error **errp)
> +{
> + StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
> + static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T' };
Any reason you don't include 'P' and 'E' at the end of this array? We
parse them just fine (at least in some contexts, going by the fact that
qemu-common.h has a STRTOSZ_DEFSUFFIX_ value for them), so we also ought
to output them.
> + uint64_t div, val;
> + char *out;
> + int i;
> +
> + if (!sov->human) {
> + out = g_strdup_printf("%llu", (long long) *obj);
(unsigned long long) would look better for the cast.
> + string_output_set(sov, out);
> + return;
> + }
> +
> + val = *obj;
> +
> + /* Compute floor(log2(val)). */
> + i = 64 - clz64(val);
> +
> + /* Find the power of 1024 that we'll display as the units. */
> + i /= 10;
> + if (i >= ARRAY_SIZE(suffixes)) {
> + i = ARRAY_SIZE(suffixes) - 1;
> + }
> + div = 1ULL << (i * 10);
> +
> + out = g_strdup_printf("%0.03f%c", (double)val/div, suffixes[i]);
If val < 1024, this gives dumb output: 1.000B. Should you special case
bytes?
Also, I like how your int printout was both decimal and hex; but here
you are throwing away information (and the bigger the number, the more
we lose as to how much got rounded away). I'd rather see this as:
"%0.03f%c (%llu)"
so that we also have access to the unrounded exact amount.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH 02/12] qdev: sizes are now parsed by StringInputVisitor, (continued)
- [Qemu-devel] [PATCH 03/12] qdev: remove legacy parsers for hex8/32/64, Paolo Bonzini, 2014/01/30
- [Qemu-devel] [PATCH 04/12] qdev: legacy properties are now read-only, Paolo Bonzini, 2014/01/30
- [Qemu-devel] [PATCH 05/12] qdev: legacy properties are just strings, Paolo Bonzini, 2014/01/30
- [Qemu-devel] [PATCH 06/12] qdev: inline qdev_prop_parse, Paolo Bonzini, 2014/01/30
- [Qemu-devel] [PATCH 07/12] qapi: add human mode to StringOutputVisitor, Paolo Bonzini, 2014/01/30
- Re: [Qemu-devel] [PATCH 07/12] qapi: add human mode to StringOutputVisitor,
Eric Blake <=
- [Qemu-devel] [PATCH 08/12] qdev: use human mode in "info qtree", Paolo Bonzini, 2014/01/30
- [Qemu-devel] [PATCH 09/12] qdev: remove most legacy printers, Paolo Bonzini, 2014/01/30
- [Qemu-devel] [PATCH 10/12] qdev: remove hex8/32/64 property types, Paolo Bonzini, 2014/01/30
- [Qemu-devel] [PATCH 11/12] qdev: add enum property types to QAPI schema, Paolo Bonzini, 2014/01/30