| Message ID | 1355458116-5692-1-git-send-email-lig.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
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. */ > @@ -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!
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.
在 2012-12-14五的 13:09 +0100，Markus Armbruster写道： > 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. > */ > > @@ -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! OK, will fix.
在 2012-12-14五的 14:45 +0100，Igor Mammedov写道： > 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. OK
diff --git a/cutils.c b/cutils.c index 4f0692f..da05c9e 100644 --- a/cutils.c +++ b/cutils.c @@ -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;
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(-)