diff mbox

[v5,2/3] utils: provide size_to_str()

Message ID 1494329105-4411-3-git-send-email-peterx@redhat.com
State New
Headers show

Commit Message

Peter Xu May 9, 2017, 11:25 a.m. UTC
I stole the algorithm from print_type_size(). I didn't generalize it
since that's using [KM...]iB while here we need [KM...]B to finally
be able to stands for page sizes (and even more general).

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 include/qemu-common.h |  1 +
 util/cutils.c         | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

Comments

Markus Armbruster May 9, 2017, 2:50 p.m. UTC | #1
Peter Xu <peterx@redhat.com> writes:

> I stole the algorithm from print_type_size(). I didn't generalize it
> since that's using [KM...]iB while here we need [KM...]B to finally
> be able to stands for page sizes (and even more general).

Can you explain why we need units without the 'i' here?

>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  include/qemu-common.h |  1 +
>  util/cutils.c         | 26 ++++++++++++++++++++++++++
>  2 files changed, 27 insertions(+)
>
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index d218821..d7d0448 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -145,6 +145,7 @@ void qemu_hexdump(const char *buf, FILE *fp, const char *prefix, size_t size);
>  int parse_debug_env(const char *name, int max, int initial);
>  
>  const char *qemu_ether_ntoa(const MACAddr *mac);
> +char *size_to_str(double val);
>  void page_size_init(void);
>  
>  /* returns non-zero if dump is in progress, otherwise zero is
> diff --git a/util/cutils.c b/util/cutils.c
> index 50ad179..5aaf370 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -619,3 +619,29 @@ const char *qemu_ether_ntoa(const MACAddr *mac)
>  
>      return ret;
>  }
> +
> +/*
> + * Sample output:
> + *
> + * 1          -> "1 B"
> + * 528        -> "0.516 KB"
> + * 4096       -> "4 KB"
> + * 2402958    -> "2.29 MB"
> + * 1073741824 -> "1 GB"
> + *
> + * Please free the buffer after use.
> + */
> +char *size_to_str(double val)
> +{
> +    static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T', 'P', 'E' };
> +    unsigned long div;
> +    int i;
> +
> +    frexp(val, &i);

The ignored return value is in [0.5,1), and multiplying it by 2^i yields
val.  i is close to the binary logarithm.

> +    i /= 10;

Now it's close to base-1024 logarithm.

Figuring this out requires too much thought for comfort.  Recommend
steal the comment from print_type_size(), too.

> +    assert(i < sizeof(suffixes));
> +    div = 1ULL << (i * 10);
> +
> +    return g_strdup_printf("%0.3g %c%s", val / div,
> +                           suffixes[i], i ? "B" : "");

The conditional is a bit confusing.  To avoid it, we could make
suffixes[] an array of strings, with suffixes[0] = "".

> +}
Peter Xu May 10, 2017, 3:32 a.m. UTC | #2
On Tue, May 09, 2017 at 04:50:26PM +0200, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > I stole the algorithm from print_type_size(). I didn't generalize it
> > since that's using [KM...]iB while here we need [KM...]B to finally
> > be able to stands for page sizes (and even more general).
> 
> Can you explain why we need units without the 'i' here?

Oops. I misunderstood XiB... My fault. Page sizes needs exactly "i".

> 
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  include/qemu-common.h |  1 +
> >  util/cutils.c         | 26 ++++++++++++++++++++++++++
> >  2 files changed, 27 insertions(+)
> >
> > diff --git a/include/qemu-common.h b/include/qemu-common.h
> > index d218821..d7d0448 100644
> > --- a/include/qemu-common.h
> > +++ b/include/qemu-common.h
> > @@ -145,6 +145,7 @@ void qemu_hexdump(const char *buf, FILE *fp, const char *prefix, size_t size);
> >  int parse_debug_env(const char *name, int max, int initial);
> >  
> >  const char *qemu_ether_ntoa(const MACAddr *mac);
> > +char *size_to_str(double val);
> >  void page_size_init(void);
> >  
> >  /* returns non-zero if dump is in progress, otherwise zero is
> > diff --git a/util/cutils.c b/util/cutils.c
> > index 50ad179..5aaf370 100644
> > --- a/util/cutils.c
> > +++ b/util/cutils.c
> > @@ -619,3 +619,29 @@ const char *qemu_ether_ntoa(const MACAddr *mac)
> >  
> >      return ret;
> >  }
> > +
> > +/*
> > + * Sample output:
> > + *
> > + * 1          -> "1 B"
> > + * 528        -> "0.516 KB"
> > + * 4096       -> "4 KB"
> > + * 2402958    -> "2.29 MB"
> > + * 1073741824 -> "1 GB"
> > + *
> > + * Please free the buffer after use.
> > + */
> > +char *size_to_str(double val)
> > +{
> > +    static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T', 'P', 'E' };
> > +    unsigned long div;
> > +    int i;
> > +
> > +    frexp(val, &i);
> 
> The ignored return value is in [0.5,1), and multiplying it by 2^i yields
> val.  i is close to the binary logarithm.
> 
> > +    i /= 10;
> 
> Now it's close to base-1024 logarithm.
> 
> Figuring this out requires too much thought for comfort.  Recommend
> steal the comment from print_type_size(), too.

Let me do it even simpler - I'll just move the whole logic in
print_type_size() into size_to_str(), then I'll refactor
print_type_size().

> 
> > +    assert(i < sizeof(suffixes));
> > +    div = 1ULL << (i * 10);
> > +
> > +    return g_strdup_printf("%0.3g %c%s", val / div,
> > +                           suffixes[i], i ? "B" : "");
> 
> The conditional is a bit confusing.  To avoid it, we could make
> suffixes[] an array of strings, with suffixes[0] = "".

I can fix this. Thanks reviewing!
diff mbox

Patch

diff --git a/include/qemu-common.h b/include/qemu-common.h
index d218821..d7d0448 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -145,6 +145,7 @@  void qemu_hexdump(const char *buf, FILE *fp, const char *prefix, size_t size);
 int parse_debug_env(const char *name, int max, int initial);
 
 const char *qemu_ether_ntoa(const MACAddr *mac);
+char *size_to_str(double val);
 void page_size_init(void);
 
 /* returns non-zero if dump is in progress, otherwise zero is
diff --git a/util/cutils.c b/util/cutils.c
index 50ad179..5aaf370 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -619,3 +619,29 @@  const char *qemu_ether_ntoa(const MACAddr *mac)
 
     return ret;
 }
+
+/*
+ * Sample output:
+ *
+ * 1          -> "1 B"
+ * 528        -> "0.516 KB"
+ * 4096       -> "4 KB"
+ * 2402958    -> "2.29 MB"
+ * 1073741824 -> "1 GB"
+ *
+ * Please free the buffer after use.
+ */
+char *size_to_str(double val)
+{
+    static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T', 'P', 'E' };
+    unsigned long div;
+    int i;
+
+    frexp(val, &i);
+    i /= 10;
+    assert(i < sizeof(suffixes));
+    div = 1ULL << (i * 10);
+
+    return g_strdup_printf("%0.3g %c%s", val / div,
+                           suffixes[i], i ? "B" : "");
+}