svn commit: r211304 - head/lib/libutil

[mdf at FreeBSD.org]

On Sat, Aug 14, 2010 at 2:34 PM, Dag-Erling Smorgrav <des at freebsd.org> wrote:
> Author: des
> Date: Sat Aug 14 14:34:36 2010
> New Revision: 211304
> URL: http://svn.freebsd.org/changeset/base/211304
>
> Log:
>  Simplify expand_number() by combining the (unrolled) loop with the
>  switch.  Since expand_number() does not accept negative numbers, switch
>  from int64_t to uint64_t; this makes it easier to check for overflow.
>
>  MFC after:    3 weeks
>
> Modified:
>  head/lib/libutil/expand_number.c
>  head/lib/libutil/libutil.h
>
> Modified: head/lib/libutil/expand_number.c
> ==============================================================================
> --- head/lib/libutil/expand_number.c    Sat Aug 14 14:18:02 2010        (r211303)
> +++ head/lib/libutil/expand_number.c    Sat Aug 14 14:34:36 2010        (r211304)
> @@ -37,7 +37,7 @@ __FBSDID("$FreeBSD$");
>
>  /*
>  * Convert an expression of the following forms to a int64_t.
> - *     1) A positive decimal number.
> + *     1) A positive decimal number.
>  *     2) A positive decimal number followed by a 'b' or 'B' (mult by 1).
>  *     3) A positive decimal number followed by a 'k' or 'K' (mult by 1 << 10).
>  *     4) A positive decimal number followed by a 'm' or 'M' (mult by 1 << 20).
> @@ -47,14 +47,12 @@ __FBSDID("$FreeBSD$");
>  *     8) A positive decimal number followed by a 'e' or 'E' (mult by 1 << 60).
>  */
>  int
> -expand_number(const char *buf, int64_t *num)
> +expand_number(const char *buf, uint64_t *num)
>  {
> -       static const char unit[] = "bkmgtpe";
> -       char *endptr, s;
> -       int64_t number;
> -       int i;
> +       uint64_t number;
> +       char *endptr;
>
> -       number = strtoimax(buf, &endptr, 0);
> +       number = strtoumax(buf, &endptr, 0);
>
>        if (endptr == buf) {
>                /* No valid digits. */
> @@ -68,15 +66,23 @@ expand_number(const char *buf, int64_t *
>                return (0);
>        }
>
> -       s = tolower(*endptr);
> -       switch (s) {
> -       case 'b':
> -       case 'k':
> -       case 'm':
> -       case 'g':
> -       case 't':
> -       case 'p':
> +#define SHIFT(n, b)                                                    \
> +       do { if ((n << b) < n) goto overflow; n <<= b; } while (0)

I think it's possible for the number to overflow but also not shrink.
e.g. 0x12345678T.

Perhaps if (((n << b) >> b) != n) would be better.

Thanks,
matthew

> +
> +       switch (tolower((unsigned char)*endptr)) {
>        case 'e':
> +               SHIFT(number, 10);
> +       case 'p':
> +               SHIFT(number, 10);
> +       case 't':
> +               SHIFT(number, 10);
> +       case 'g':
> +               SHIFT(number, 10);
> +       case 'm':
> +               SHIFT(number, 10);
> +       case 'k':
> +               SHIFT(number, 10);
> +       case 'b':
>                break;
>        default:
>                /* Unrecognized unit. */
> @@ -84,17 +90,11 @@ expand_number(const char *buf, int64_t *
>                return (-1);
>        }
>
> -       for (i = 0; unit[i] != '\0'; i++) {
> -               if (s == unit[i])
> -                       break;
> -               if ((number < 0 && (number << 10) > number) ||
> -                   (number >= 0 && (number << 10) < number)) {
> -                       errno = ERANGE;
> -                       return (-1);
> -               }
> -               number <<= 10;
> -       }
> -
>        *num = number;
>        return (0);
> +
> +overflow:
> +       /* Overflow */
> +       errno = ERANGE;
> +       return (-1);
>  }
>
> Modified: head/lib/libutil/libutil.h
> ==============================================================================
> --- head/lib/libutil/libutil.h  Sat Aug 14 14:18:02 2010        (r211303)
> +++ head/lib/libutil/libutil.h  Sat Aug 14 14:34:36 2010        (r211304)
> @@ -109,7 +109,7 @@ int forkpty(int *_amaster, char *_name,
>                     struct termios *_termp, struct winsize *_winp);
>  int    humanize_number(char *_buf, size_t _len, int64_t _number,
>            const char *_suffix, int _scale, int _flags);
> -int    expand_number(const char *_buf, int64_t *_num);
> +int    expand_number(const char *_buf, uint64_t *_num);
>  const char *uu_lockerr(int _uu_lockresult);
>  int    uu_lock(const char *_ttyname);
>  int    uu_unlock(const char *_ttyname);
>