On Fri, 2010-10-22 at 20:16 -0300, Davidlohr Bueso wrote:
> On Fri, 2010-10-22 at 22:15 +0200, Karel Zak wrote:
> > On Fri, Oct 22, 2010 at 04:40:22PM -0300, Davidlohr Bueso wrote:
> > > On Fri, 2010-10-22 at 21:31 +0200, Karel Zak wrote:
> > > > On Fri, Oct 22, 2010 at 03:54:11PM -0300, Davidlohr Bueso wrote:
> > > > > [PATCH] strutils: general purpose string handling functions
> > > > >
> > > > > This patch replaces a few functions used throught the source:
> > > > > * Renames getnum (from schedutils) to strtol_on_err
> > > >
> > > > What about "strtol_or_err()" ?
> > >
> > > Don't quite follow, we had decided that that would be the name for the
> > > getnum() replacement.
> >
> > Yep, we have talked about "strtol ***or*** err", but I see "on" in
> > your patch ;-)
>
> Resending correction below. Thanks.
[snip]
> -static int do_scale_by_power (uintmax_t *x, int base, int power)
> -{
> - while (power--) {
This assumes power >= 0
> - if (UINTMAX_MAX / base < *x)
> - return -2;
> - *x *= base;
This assumes base > 0
> - }
> - return 0;
> -}
So I would suggest:
static int do_scale_by_power (uintmax_t *x, unsigned int base, unsigned int power)
{
switch (base)
{
case 0:
*x = 0;
break;
case 1:
break;
default:
while (power-- > 0) {
uintmax_t y = *x * base;
if (y < *x)
return -2;
*x = y;
}
}
return 0;
}
Which also avoids the expensive division and is guaranteed by the C
standard wrt unsigned integer arithmetic overflow. If the function were
declared inline the switch could normally be optimized out for literal
bases.
[snip]
> +int strtosize(const char *str, uintmax_t *res)
> +{
> + char *p;
> + uintmax_t x;
> + int base = 1024, rc = 0;
> +
> + *res = 0;
> +
> + if (!str || !*str)
> + goto err;
> +
> + /* Only positive numbers are acceptable
> + *
> + * Note that this check is not perfect, it would be better to
> + * use lconv->negative_sign. But coreutils use the same solution,
> + * so it's probably good enough...
> + */
> + p = (char *) str;
Const casts subvert compiler checks and are generally unnecessary if the
program design is OK. This would benefit from an additional local const
char*.
HTH
-- Lawrence Rust
--
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
