Re: [Qemu-devel] [PATCH 1/2] cutils:change strtosz_suffix_unit function

[Igor Mammedov <imammedo@redhat.com>]

On Fri, 14 Dec 2012 13:09:17 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> liguang <lig.fnst@cn.fujitsu.com> writes:
> 
> > if value to be translated is larger than INT64_MAX,
> > this function will not be convenient for caller to
> > be aware of it, so change a little for this.
> >
> > Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> > ---
> >  cutils.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/cutils.c b/cutils.c
> > index 4f0692f..da05c9e 100644
> > --- a/cutils.c
> > +++ b/cutils.c
>    /*
>     * Convert string to bytes, allowing either B/b for bytes, K/k for KB,
>     * M/m for MB, G/g for GB or T/t for TB. End pointer will be returned
>     * in *end, if not NULL. Return -1 on error.
>     */
Size is not the only user of it, this function is used to convert KHz,... in
target-i386/cpu.c, i.e. unit might be something else than 1024. That's why
there is question about generalizing strtosz_suffix_unit() in future instead of
duplicating suffixed int parsing. And specialization for size with
unit=1024 and enforcing limits could be made in strtosz_suffix() which is
used by current size users.


> > @@ -219,7 +219,7 @@ static int64_t suffix_mul(char suffix, int64_t unit)
> >  int64_t strtosz_suffix_unit(const char *nptr, char **end,
> >                              const char default_suffix, int64_t unit)
> >  {
> > -    int64_t retval = -1;
> > +    int64_t retval = EINVAL;
> >      char *endptr;
> >      unsigned char c;
> >      int mul_required = 0;
> > @@ -246,6 +246,7 @@ int64_t strtosz_suffix_unit(const char *nptr, char **end,
> >          goto fail;
> >      }
> >      if ((val * mul >= INT64_MAX) || val < 0) {
> > +        retval = ERANGE;
> >          goto fail;
> >      }
> >      retval = val * mul;
> 
> Your error codes aren't negative, and you failed to update the function
> comment!
Returning negative here is fine for now form target-i386/cpu.c pov.


-- 
Regards,
  Igor