Re: [PATCH v2 1/3] util/cutils: Introduce freq_to_str() to display Hertz units

[Philippe Mathieu-Daudé]

On 10/1/20 8:42 PM, Eduardo Habkost wrote:
> On Thu, Oct 01, 2020 at 06:43:20PM +0200, Philippe Mathieu-Daudé wrote:
> > Introduce freq_to_str() to convert frequency values in human
> > friendly units using the SI units for Hertz.
> >
> > Suggested-by: Luc Michel <luc@lmichel.fr>
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> >   include/qemu/cutils.h | 12 ++++++++++++
> >   util/cutils.c         | 14 ++++++++++++++
> >   2 files changed, 26 insertions(+)
> >
> > diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
> > index 3a86ec0321..4bbf4834ea 100644
> > --- a/include/qemu/cutils.h
> > +++ b/include/qemu/cutils.h
> > @@ -158,6 +158,18 @@ int qemu_strtosz_metric(const char *nptr, const char 
> > **end, uint64_t *result);
> >   
> >   char *size_to_str(uint64_t val);
> >   
> > +/**
> > + * freq_to_str:
> > + * @freq_hz: frequency to stringify
> > + *
> > + * Return human readable string for frequency @freq_hz.
> > + * Use SI units like KHz, MHz, and so forth.
> > + *
> > + * The caller is responsible for releasing the value returned
> > + * with g_free() after use.
> > + */
> > +char *freq_to_str(uint64_t freq_hz);
> > +
> >   /* used to print char* safely */
> >   #define STR_OR_NULL(str) ((str) ? (str) : "null")
> >   
> > diff --git a/util/cutils.c b/util/cutils.c
> > index 8da34e04b0..be4e43a9ef 100644
> > --- a/util/cutils.c
> > +++ b/util/cutils.c
> > @@ -885,6 +885,20 @@ char *size_to_str(uint64_t val)
> >       return g_strdup_printf("%0.3g %sB", (double)val / div, suffixes[i]);
> >   }
> >   
> > +char *freq_to_str(uint64_t freq_hz)
> > +{
> > +    static const char *const suffixes[] = { "", "K", "M", "G", "T", "P", "E" };
> > +    double freq = freq_hz;
> > +    size_t idx = 0;
> > +
> > +    while (freq >= 1000.0 && idx < ARRAY_SIZE(suffixes)) {
> > +        freq /= 1000.0;
> > +        idx++;
> > +    }
> > +
> > +    return g_strdup_printf("%0.3g %sHz", freq, suffixes[idx]);
>
> The only thing protecting this from out of bounds array access is
> the fact that UINT64_MAX is smaller than 1000E.  I wonder if this
> causes a Coverity warning.

Aren't we protected by the "idx < ARRAY_SIZE(suffixes)" check?

> > +}
> > +
> >   int qemu_pstrcmp0(const char **str1, const char **str2)
> >   {
> >       return g_strcmp0(*str1, *str2);
> > --
> > 2.26.2