| Message ID | 1442241198-11235-2-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 14 September 2015 at 15:33, Paolo Bonzini <pbonzini@redhat.com> wrote: > Linux returns 0 if no conversion was made, while OS X and presumably > the BSDs return EINVAL. The OS X convention rejects more invalid > inputs, so convert to it and adjust the test case. > > Windows returns 1 from strtoul and strtoull (instead of -1) for > negative out-of-range input; fix it up. > > Reported-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > tests/test-cutils.c | 57 +++++++++++++++-------------------------------------- > util/cutils.c | 24 +++++++++++++++++----- > 2 files changed, 35 insertions(+), 46 deletions(-) > > diff --git a/tests/test-cutils.c b/tests/test-cutils.c > index a7a15a5..0046c61 100644 > --- a/tests/test-cutils.c > +++ b/tests/test-cutils.c > @@ -264,9 +264,7 @@ static void test_qemu_strtol_empty(void) > > err = qemu_strtol(str, &endptr, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert_cmpint(res, ==, 0); > - g_assert(endptr == str); > + g_assert_cmpint(err, ==, -EINVAL); > } > > static void test_qemu_strtol_whitespace(void) > @@ -279,9 +277,7 @@ static void test_qemu_strtol_whitespace(void) > > err = qemu_strtol(str, &endptr, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert_cmpint(res, ==, 0); > - g_assert(endptr == str); > + g_assert_cmpint(err, ==, -EINVAL); > } > > static void test_qemu_strtol_invalid(void) > @@ -294,9 +290,7 @@ static void test_qemu_strtol_invalid(void) > > err = qemu_strtol(str, &endptr, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert_cmpint(res, ==, 0); > - g_assert(endptr == str); > + g_assert_cmpint(err, ==, -EINVAL); > } > > static void test_qemu_strtol_trailing(void) > @@ -478,8 +472,7 @@ static void test_qemu_strtol_full_empty(void) > > err = qemu_strtol(str, NULL, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert_cmpint(res, ==, 0); > + g_assert_cmpint(err, ==, -EINVAL); > } > > static void test_qemu_strtol_full_negative(void) > @@ -555,9 +548,7 @@ static void test_qemu_strtoul_empty(void) > > err = qemu_strtoul(str, &endptr, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert_cmpint(res, ==, 0); > - g_assert(endptr == str); > + g_assert_cmpint(err, ==, -EINVAL); > } > > static void test_qemu_strtoul_whitespace(void) > @@ -570,9 +561,7 @@ static void test_qemu_strtoul_whitespace(void) > > err = qemu_strtoul(str, &endptr, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert_cmpint(res, ==, 0); > - g_assert(endptr == str); > + g_assert_cmpint(err, ==, -EINVAL); > } > > static void test_qemu_strtoul_invalid(void) > @@ -585,8 +574,7 @@ static void test_qemu_strtoul_invalid(void) > > err = qemu_strtoul(str, &endptr, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert(endptr == str); > + g_assert_cmpint(err, ==, -EINVAL); > } > > static void test_qemu_strtoul_trailing(void) > @@ -765,8 +753,7 @@ static void test_qemu_strtoul_full_empty(void) > > err = qemu_strtoul(str, NULL, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert_cmpint(res, ==, 0); > + g_assert_cmpint(err, ==, -EINVAL); > } > static void test_qemu_strtoul_full_negative(void) > { > @@ -840,9 +827,7 @@ static void test_qemu_strtoll_empty(void) > > err = qemu_strtoll(str, &endptr, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert_cmpint(res, ==, 0); > - g_assert(endptr == str); > + g_assert_cmpint(err, ==, -EINVAL); > } > > static void test_qemu_strtoll_whitespace(void) > @@ -855,9 +840,7 @@ static void test_qemu_strtoll_whitespace(void) > > err = qemu_strtoll(str, &endptr, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert_cmpint(res, ==, 0); > - g_assert(endptr == str); > + g_assert_cmpint(err, ==, -EINVAL); > } > > static void test_qemu_strtoll_invalid(void) > @@ -870,8 +853,7 @@ static void test_qemu_strtoll_invalid(void) > > err = qemu_strtoll(str, &endptr, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert(endptr == str); > + g_assert_cmpint(err, ==, -EINVAL); > } > > static void test_qemu_strtoll_trailing(void) > @@ -1050,8 +1032,7 @@ static void test_qemu_strtoll_full_empty(void) > > err = qemu_strtoll(str, NULL, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert_cmpint(res, ==, 0); > + g_assert_cmpint(err, ==, -EINVAL); > } > > static void test_qemu_strtoll_full_negative(void) > @@ -1128,9 +1109,7 @@ static void test_qemu_strtoull_empty(void) > > err = qemu_strtoull(str, &endptr, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert_cmpint(res, ==, 0); > - g_assert(endptr == str); > + g_assert_cmpint(err, ==, -EINVAL); > } > > static void test_qemu_strtoull_whitespace(void) > @@ -1143,9 +1122,7 @@ static void test_qemu_strtoull_whitespace(void) > > err = qemu_strtoull(str, &endptr, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert_cmpint(res, ==, 0); > - g_assert(endptr == str); > + g_assert_cmpint(err, ==, -EINVAL); > } > > static void test_qemu_strtoull_invalid(void) > @@ -1158,8 +1135,7 @@ static void test_qemu_strtoull_invalid(void) > > err = qemu_strtoull(str, &endptr, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert(endptr == str); > + g_assert_cmpint(err, ==, -EINVAL); > } > > static void test_qemu_strtoull_trailing(void) > @@ -1338,8 +1314,7 @@ static void test_qemu_strtoull_full_empty(void) > > err = qemu_strtoull(str, NULL, 0, &res); > > - g_assert_cmpint(err, ==, 0); > - g_assert_cmpint(res, ==, 0); > + g_assert_cmpint(err, ==, -EINVAL); > } > > static void test_qemu_strtoull_full_negative(void) > diff --git a/util/cutils.c b/util/cutils.c > index 67c50e5..84ceaff 100644 > --- a/util/cutils.c > +++ b/util/cutils.c > @@ -361,9 +361,15 @@ int64_t strtosz(const char *nptr, char **end) > /** > * Helper function for qemu_strto*l() functions. > */ > -static int check_strtox_error(const char **next, char *endptr, > +static int check_strtox_error(const char *p, char *endptr, const char **next, > int err) > { > + /* If no conversion was performed, prefer BSD behavior over glibc > + * behavior. > + */ > + if (err == 0 && endptr == p) { > + err = EINVAL; > + } > if (!next && *endptr) { > return -EINVAL; > } > @@ -412,7 +418,7 @@ int qemu_strtol(const char *nptr, const char **endptr, int base, > } else { > errno = 0; > *result = strtol(nptr, &p, base); > - err = check_strtox_error(endptr, p, errno); > + err = check_strtox_error(nptr, p, endptr, errno); > } > return err; > } > @@ -443,7 +449,11 @@ int qemu_strtoul(const char *nptr, const char **endptr, int base, > } else { > errno = 0; > *result = strtoul(nptr, &p, base); > - err = check_strtox_error(endptr, p, errno); > + /* Windows returns 1 for negative out-of-range values. */ > + if (errno == ERANGE) { > + *result = -1; > + } > + err = check_strtox_error(nptr, p, endptr, errno); > } > return err; > } > @@ -466,7 +476,7 @@ int qemu_strtoll(const char *nptr, const char **endptr, int base, > } else { > errno = 0; > *result = strtoll(nptr, &p, base); > - err = check_strtox_error(endptr, p, errno); > + err = check_strtox_error(nptr, p, endptr, errno); > } > return err; > } > @@ -489,7 +499,11 @@ int qemu_strtoull(const char *nptr, const char **endptr, int base, > } else { > errno = 0; > *result = strtoull(nptr, &p, base); > - err = check_strtox_error(endptr, p, errno); > + /* Windows returns 1 for negative out-of-range values. */ > + if (errno == ERANGE) { > + *result = -1; > + } > + err = check_strtox_error(nptr, p, endptr, errno); > } > return err; > } > -- > 2.5.0 > >
On 14 September 2015 at 15:47, Peter Maydell <peter.maydell@linaro.org> wrote: > On 14 September 2015 at 15:33, Paolo Bonzini <pbonzini@redhat.com> wrote: >> Windows returns 1 from strtoul and strtoull (instead of -1) for >> negative out-of-range input; fix it up. No problem with this patch, just wanted to say 'wat?' at this. (and then fumbled a blank reply with my mail client, oh well) -- PMM
On 14/09/2015 16:48, Peter Maydell wrote: >>> Windows returns 1 from strtoul and strtoull (instead of -1) for >>> >> negative out-of-range input; fix it up. > No problem with this patch, just wanted to say 'wat?' at this. Indeed. I only tested with Wine, but it cannot be a mistake: http://marc.info/?l=wine-patches&m=138011287312090&w=2 There are even test cases. :-) Paolo
diff --git a/tests/test-cutils.c b/tests/test-cutils.c index a7a15a5..0046c61 100644 --- a/tests/test-cutils.c +++ b/tests/test-cutils.c @@ -264,9 +264,7 @@ static void test_qemu_strtol_empty(void) err = qemu_strtol(str, &endptr, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert_cmpint(res, ==, 0); - g_assert(endptr == str); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtol_whitespace(void) @@ -279,9 +277,7 @@ static void test_qemu_strtol_whitespace(void) err = qemu_strtol(str, &endptr, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert_cmpint(res, ==, 0); - g_assert(endptr == str); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtol_invalid(void) @@ -294,9 +290,7 @@ static void test_qemu_strtol_invalid(void) err = qemu_strtol(str, &endptr, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert_cmpint(res, ==, 0); - g_assert(endptr == str); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtol_trailing(void) @@ -478,8 +472,7 @@ static void test_qemu_strtol_full_empty(void) err = qemu_strtol(str, NULL, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert_cmpint(res, ==, 0); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtol_full_negative(void) @@ -555,9 +548,7 @@ static void test_qemu_strtoul_empty(void) err = qemu_strtoul(str, &endptr, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert_cmpint(res, ==, 0); - g_assert(endptr == str); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtoul_whitespace(void) @@ -570,9 +561,7 @@ static void test_qemu_strtoul_whitespace(void) err = qemu_strtoul(str, &endptr, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert_cmpint(res, ==, 0); - g_assert(endptr == str); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtoul_invalid(void) @@ -585,8 +574,7 @@ static void test_qemu_strtoul_invalid(void) err = qemu_strtoul(str, &endptr, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert(endptr == str); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtoul_trailing(void) @@ -765,8 +753,7 @@ static void test_qemu_strtoul_full_empty(void) err = qemu_strtoul(str, NULL, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert_cmpint(res, ==, 0); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtoul_full_negative(void) { @@ -840,9 +827,7 @@ static void test_qemu_strtoll_empty(void) err = qemu_strtoll(str, &endptr, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert_cmpint(res, ==, 0); - g_assert(endptr == str); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtoll_whitespace(void) @@ -855,9 +840,7 @@ static void test_qemu_strtoll_whitespace(void) err = qemu_strtoll(str, &endptr, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert_cmpint(res, ==, 0); - g_assert(endptr == str); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtoll_invalid(void) @@ -870,8 +853,7 @@ static void test_qemu_strtoll_invalid(void) err = qemu_strtoll(str, &endptr, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert(endptr == str); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtoll_trailing(void) @@ -1050,8 +1032,7 @@ static void test_qemu_strtoll_full_empty(void) err = qemu_strtoll(str, NULL, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert_cmpint(res, ==, 0); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtoll_full_negative(void) @@ -1128,9 +1109,7 @@ static void test_qemu_strtoull_empty(void) err = qemu_strtoull(str, &endptr, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert_cmpint(res, ==, 0); - g_assert(endptr == str); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtoull_whitespace(void) @@ -1143,9 +1122,7 @@ static void test_qemu_strtoull_whitespace(void) err = qemu_strtoull(str, &endptr, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert_cmpint(res, ==, 0); - g_assert(endptr == str); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtoull_invalid(void) @@ -1158,8 +1135,7 @@ static void test_qemu_strtoull_invalid(void) err = qemu_strtoull(str, &endptr, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert(endptr == str); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtoull_trailing(void) @@ -1338,8 +1314,7 @@ static void test_qemu_strtoull_full_empty(void) err = qemu_strtoull(str, NULL, 0, &res); - g_assert_cmpint(err, ==, 0); - g_assert_cmpint(res, ==, 0); + g_assert_cmpint(err, ==, -EINVAL); } static void test_qemu_strtoull_full_negative(void) diff --git a/util/cutils.c b/util/cutils.c index 67c50e5..84ceaff 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -361,9 +361,15 @@ int64_t strtosz(const char *nptr, char **end) /** * Helper function for qemu_strto*l() functions. */ -static int check_strtox_error(const char **next, char *endptr, +static int check_strtox_error(const char *p, char *endptr, const char **next, int err) { + /* If no conversion was performed, prefer BSD behavior over glibc + * behavior. + */ + if (err == 0 && endptr == p) { + err = EINVAL; + } if (!next && *endptr) { return -EINVAL; } @@ -412,7 +418,7 @@ int qemu_strtol(const char *nptr, const char **endptr, int base, } else { errno = 0; *result = strtol(nptr, &p, base); - err = check_strtox_error(endptr, p, errno); + err = check_strtox_error(nptr, p, endptr, errno); } return err; } @@ -443,7 +449,11 @@ int qemu_strtoul(const char *nptr, const char **endptr, int base, } else { errno = 0; *result = strtoul(nptr, &p, base); - err = check_strtox_error(endptr, p, errno); + /* Windows returns 1 for negative out-of-range values. */ + if (errno == ERANGE) { + *result = -1; + } + err = check_strtox_error(nptr, p, endptr, errno); } return err; } @@ -466,7 +476,7 @@ int qemu_strtoll(const char *nptr, const char **endptr, int base, } else { errno = 0; *result = strtoll(nptr, &p, base); - err = check_strtox_error(endptr, p, errno); + err = check_strtox_error(nptr, p, endptr, errno); } return err; } @@ -489,7 +499,11 @@ int qemu_strtoull(const char *nptr, const char **endptr, int base, } else { errno = 0; *result = strtoull(nptr, &p, base); - err = check_strtox_error(endptr, p, errno); + /* Windows returns 1 for negative out-of-range values. */ + if (errno == ERANGE) { + *result = -1; + } + err = check_strtox_error(nptr, p, endptr, errno); } return err; }
Linux returns 0 if no conversion was made, while OS X and presumably the BSDs return EINVAL. The OS X convention rejects more invalid inputs, so convert to it and adjust the test case. Windows returns 1 from strtoul and strtoull (instead of -1) for negative out-of-range input; fix it up. Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- tests/test-cutils.c | 57 +++++++++++++++-------------------------------------- util/cutils.c | 24 +++++++++++++++++----- 2 files changed, 35 insertions(+), 46 deletions(-)