| Message ID | 1284648749-18479-3-git-send-email-Jes.Sorensen@redhat.com |
|---|---|
| State | New |
| Headers | show |
Jes.Sorensen@redhat.com writes: > From: Jes Sorensen <Jes.Sorensen@redhat.com> > > Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com> > --- > cutils.c | 34 ++++++++++++++++++++++++++-------- > 1 files changed, 26 insertions(+), 8 deletions(-) > > diff --git a/cutils.c b/cutils.c > index dabbed4..67767a9 100644 > --- a/cutils.c > +++ b/cutils.c > @@ -259,34 +259,52 @@ int fcntl_setfl(int fd, int flag) > */ > uint64_t strtobytes(const char *nptr, char **end) > { > - uint64_t value; > + uint64_t retval = 0; > char *endptr; > + int mul_required = 0; > + double val, mul = 1; > + > + endptr = (char *)nptr + strspn(nptr, " 0123456789"); > + if (*endptr == '.') { > + mul_required = 1; > + } > + > + val = strtod(nptr, &endptr); > + > + if (val < 0) > + goto fail; I pointed out several problems with the use of strtoull() in PATCH 1/5, and now you go ahead and switch over to strtod() in PATCH 2/5. Gee, thanks! For penance, I want you to peruse strtod(3) and figure out its correct usage ;) > > - value = strtoll(nptr, &endptr, 0); > switch (*endptr++) { > case 'K': > case 'k': > - value <<= 10; > + mul = 1 << 10; > break; > case 0: > + case ' ': > + if (mul_required) { > + goto fail; > + } I wouldn't have bothered catching this case. Instead, I'd have checked that the final conversion to uint64_t is exact. But since you coded it already, feel free to keep it. > case 'M': > case 'm': > - value <<= 20; > + mul = 1ULL << 20; > break; > case 'G': > case 'g': > - value <<= 30; > + mul = 1ULL << 30; > break; > case 'T': > case 't': > - value <<= 40; > + mul = 1ULL << 40; > break; > default: > - value = 0; > + goto fail; > } > > + retval = (uint64_t)(val * mul); > + What if the conversion overflows uint64_t? Shouldn't we catch that? > if (end) > *end = endptr; > > - return value; > +fail: > + return retval; > }
diff --git a/cutils.c b/cutils.c index dabbed4..67767a9 100644 --- a/cutils.c +++ b/cutils.c @@ -259,34 +259,52 @@ int fcntl_setfl(int fd, int flag) */ uint64_t strtobytes(const char *nptr, char **end) { - uint64_t value; + uint64_t retval = 0; char *endptr; + int mul_required = 0; + double val, mul = 1; + + endptr = (char *)nptr + strspn(nptr, " 0123456789"); + if (*endptr == '.') { + mul_required = 1; + } + + val = strtod(nptr, &endptr); + + if (val < 0) + goto fail; - value = strtoll(nptr, &endptr, 0); switch (*endptr++) { case 'K': case 'k': - value <<= 10; + mul = 1 << 10; break; case 0: + case ' ': + if (mul_required) { + goto fail; + } case 'M': case 'm': - value <<= 20; + mul = 1ULL << 20; break; case 'G': case 'g': - value <<= 30; + mul = 1ULL << 30; break; case 'T': case 't': - value <<= 40; + mul = 1ULL << 40; break; default: - value = 0; + goto fail; } + retval = (uint64_t)(val * mul); + if (end) *end = endptr; - return value; +fail: + return retval; }