| Message ID | 1494505358-15287-5-git-send-email-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/11/2017 07:22 AM, Peter Xu wrote: > It's not very safe to assert in size_to_str(). Let's be inclusive. > > It naturally allows negative values. Now it won't even limit on the > size, as long as double would allow. > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > util/cutils.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) Since you just introduced this in 2/4, I'd rather see the two squashed together if we decide this is the interface we want to support.
Eric Blake <eblake@redhat.com> writes: > On 05/11/2017 07:22 AM, Peter Xu wrote: >> It's not very safe to assert in size_to_str(). Let's be inclusive. >> >> It naturally allows negative values. Now it won't even limit on the >> size, as long as double would allow. >> >> Signed-off-by: Peter Xu <peterx@redhat.com> >> --- >> util/cutils.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) > > Since you just introduced this in 2/4, I'd rather see the two squashed > together if we decide this is the interface we want to support. PATCH 2 factors it out of print_type_size(). Doing just that and improving only on top makes some sense.
On 05/11/2017 01:22 PM, Markus Armbruster wrote: > Eric Blake <eblake@redhat.com> writes: > >> On 05/11/2017 07:22 AM, Peter Xu wrote: >>> It's not very safe to assert in size_to_str(). Let's be inclusive. >>> >>> It naturally allows negative values. Now it won't even limit on the >>> size, as long as double would allow. >>> >>> Signed-off-by: Peter Xu <peterx@redhat.com> >>> --- >>> util/cutils.c | 9 +++++++-- >>> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> Since you just introduced this in 2/4, I'd rather see the two squashed >> together if we decide this is the interface we want to support. > > PATCH 2 factors it out of print_type_size(). Doing just that and > improving only on top makes some sense. Okay. (Moral of the story: I should have done more than just glance at 2 before commenting on 4)
diff --git a/util/cutils.c b/util/cutils.c index fa5ddec..8df0963 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -622,7 +622,7 @@ const char *qemu_ether_ntoa(const MACAddr *mac) /* * Return human readable string for size @val. - * @val must be between (-1000Eib, 1000EiB), exclusively. + * @val can be any value. * Use IEC binary units like KiB, MiB, and so forth. * Caller is responsible for passing it to g_free(). */ @@ -640,7 +640,12 @@ char *size_to_str(double val) */ frexp(val / (1000.0 / 1024.0), &i); i = (i - 1) / 10; - assert(i < ARRAY_SIZE(suffixes)); + + /* Use the biggest possible suffix */ + if (i > ARRAY_SIZE(suffixes) - 1) { + i = ARRAY_SIZE(suffixes) - 1; + } + div = 1ULL << (i * 10); return g_strdup_printf("%0.3g %sB", val / div, suffixes[i]);
It's not very safe to assert in size_to_str(). Let's be inclusive. It naturally allows negative values. Now it won't even limit on the size, as long as double would allow. Signed-off-by: Peter Xu <peterx@redhat.com> --- util/cutils.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)