diff --git a/src/util/virstring.c b/src/util/virstring.c index 64c7259750..b5fc6380f6 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2012-2013 Red Hat, Inc. + * Copyright (C) 2012-2014 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -217,11 +217,23 @@ virStrToLong_ui(char const *s, char **end_ptr, int base, unsigned int *result) { unsigned long int val; char *p; - int err; + bool err = false; errno = 0; val = strtoul(s, &p, base); /* exempt from syntax-check */ - err = (errno || (!end_ptr && *p) || p == s || (unsigned int) val != val); + + /* This one's tricky. We _want_ to allow "-1" as shorthand for + * UINT_MAX regardless of whether long is 32-bit or 64-bit. But + * strtoul treats "-1" as ULONG_MAX, and going from ulong back + * to uint differs depending on the size of long. */ + if (sizeof(long) > sizeof(int) && memchr(s, '-', p - s)) { + if (-val > UINT_MAX) + err = true; + else + val &= 0xffffffff; + } + + err |= (errno || (!end_ptr && *p) || p == s || (unsigned int) val != val); if (end_ptr) *end_ptr = p; if (err) diff --git a/tests/virstringtest.c b/tests/virstringtest.c index a4ae966012..0380df136e 100644 --- a/tests/virstringtest.c +++ b/tests/virstringtest.c @@ -23,6 +23,8 @@ #include #include "testutils.h" +#include "intprops.h" +#include "verify.h" #include "virerror.h" #include "viralloc.h" #include "virfile.h" @@ -286,7 +288,7 @@ struct stringSearchData { }; static int -testStringSearch(const void *opaque ATTRIBUTE_UNUSED) +testStringSearch(const void *opaque) { const struct stringSearchData *data = opaque; char **matches = NULL; @@ -373,6 +375,93 @@ testStringReplace(const void *opaque ATTRIBUTE_UNUSED) } +struct stringToLongData { + const char *str; + const char *suffix; + int si; /* syntax-check doesn't like bare 'i' */ + int si_ret; + unsigned int ui; + int ui_ret; + /* No expected results for long: on 32-bit platforms, it is the + * same as int, on 64-bit platforms it is the same as long long */ + long long ll; + int ll_ret; + unsigned long long ull; + int ull_ret; +}; + +/* This test makes assumptions about our compilation platform that are + * not guaranteed by POSIX. Good luck to you if you are crazy enough + * to try and port libvirt to a platform with 16-bit int. */ +verify(sizeof(int) == 4); +verify(TYPE_TWOS_COMPLEMENT(int)); +verify(sizeof(long) == sizeof(int) || sizeof(long) == sizeof(long long)); +verify(TYPE_TWOS_COMPLEMENT(long)); +verify(sizeof(long long) == 8); +verify(TYPE_TWOS_COMPLEMENT(long long)); + +static int +testStringToLong(const void *opaque) +{ + const struct stringToLongData *data = opaque; + int ret = 0; + char *end; + long l; + unsigned long ul; + +#define TEST_ONE(Str, Suff, Type, Fn, Fmt, Exp, Exp_ret) \ + do { \ + Type value = 5; \ + int result; \ + end = (char *) "oops"; \ + result = virStrToLong_ ## Fn(Str, Suff ? &end : NULL, \ + 0, &value); \ + /* On failure, end is modified, value is unchanged */ \ + if (result != (Exp_ret)) { \ + fprintf(stderr, \ + "type " #Fn " returned %d expected %d\n", \ + result, Exp_ret); \ + ret = -1; \ + } \ + if (value != ((Exp_ret) ? 5 : Exp)) { \ + fprintf(stderr, \ + "type " #Fn " value " Fmt " expected " Fmt "\n", \ + value, ((Exp_ret) ? 5 : Exp)); \ + ret = -1; \ + } \ + if (Suff && STRNEQ_NULLABLE(Suff, end)) { \ + fprintf(stderr, \ + "type " #Fn " end '%s' expected '%s'\n", \ + NULLSTR(end), Suff); \ + ret = -1; \ + } \ + } while (0) + + TEST_ONE(data->str, data->suffix, int, i, "%d", + data->si, data->si_ret); + TEST_ONE(data->str, data->suffix, unsigned int, ui, "%u", + data->ui, data->ui_ret); + + /* We hate adding new API with 'long', and prefer 'int' or 'long + * long' instead, since platform-specific results are evil */ + l = (sizeof(int) == sizeof(long)) ? data->si : data->ll; + TEST_ONE(data->str, data->suffix, long, l, "%ld", + l, (sizeof(int) == sizeof(long)) ? data->si_ret : data->ll_ret); + ul = (sizeof(int) == sizeof(long)) ? data->ui : data->ull; + TEST_ONE(data->str, data->suffix, unsigned long, ul, "%lu", + ul, (sizeof(int) == sizeof(long)) ? data->ui_ret : data->ull_ret); + + TEST_ONE(data->str, data->suffix, long long, ll, "%lld", + data->ll, data->ll_ret); + TEST_ONE(data->str, data->suffix, unsigned long long, ull, "%llu", + data->ull, data->ull_ret); + +#undef TEST_ONE + + return ret; +} + + static int mymain(void) { @@ -493,6 +582,92 @@ mymain(void) TEST_REPLACE("fooooofoooo", "foo", "barwizzeek", "barwizzeekooobarwizzeekoo"); TEST_REPLACE("fooooofoooo", "foooo", "foo", "fooofoo"); +#define TEST_STRTOL(str, suff, i, i_ret, u, u_ret, \ + ll, ll_ret, ull, ull_ret) \ + do { \ + struct stringToLongData data = { \ + str, suff, i, i_ret, u, u_ret, ll, ll_ret, ull, ull_ret, \ + }; \ + if (virtTestRun("virStringToLong '" str "'", testStringToLong, \ + &data) < 0) \ + ret = -1; \ + } while (0) + + /* Start simple */ + TEST_STRTOL("0", NULL, 0, 0, 0U, 0, 0LL, 0, 0ULL, 0); + + /* All your base are belong to us */ + TEST_STRTOL("0x0", NULL, 0, 0, 0U, 0, 0LL, 0, 0ULL, 0); + TEST_STRTOL("0XaB", NULL, 171, 0, 171U, 0, 171LL, 0, 171ULL, 0); + TEST_STRTOL("010", NULL, 8, 0, 8U, 0, 8LL, 0, 8ULL, 0); + + /* Suffix handling */ + TEST_STRTOL("42", NULL, 42, 0, 42U, 0, 42LL, 0, 42ULL, 0); + TEST_STRTOL("42", "", 42, 0, 42U, 0, 42LL, 0, 42ULL, 0); + TEST_STRTOL("42.", NULL, 0, -1, 0U, -1, 0LL, -1, 0ULL, -1); + TEST_STRTOL("42.", ".", 42, 0, 42U, 0, 42LL, 0, 42ULL, 0); + + /* Blatant invalid input */ + TEST_STRTOL("", "", 0, -1, 0U, -1, 0LL, -1, 0ULL, -1); + TEST_STRTOL("", NULL, 0, -1, 0U, -1, 0LL, -1, 0ULL, -1); + TEST_STRTOL(" ", " ", 0, -1, 0U, -1, 0LL, -1, 0ULL, -1); + TEST_STRTOL(" ", NULL, 0, -1, 0U, -1, 0LL, -1, 0ULL, -1); + TEST_STRTOL(" -", " -", 0, -1, 0U, -1, 0LL, -1, 0ULL, -1); + TEST_STRTOL(" -", NULL, 0, -1, 0U, -1, 0LL, -1, 0ULL, -1); + TEST_STRTOL("a", "a", 0, -1, 0U, -1, 0LL, -1, 0ULL, -1); + TEST_STRTOL("a", NULL, 0, -1, 0U, -1, 0LL, -1, 0ULL, -1); + + /* Not a hex number, but valid when suffix expected */ + TEST_STRTOL(" 0x", NULL, 0, -1, 0U, -1, 0LL, -1, 0ULL, -1); + TEST_STRTOL(" 0x", "x", 0, 0, 0U, 0, 0LL, 0, 0ULL, 0); + + /* Upper bounds */ + TEST_STRTOL("2147483647", NULL, 2147483647, 0, 2147483647U, 0, + 2147483647LL, 0, 2147483647ULL, 0); + TEST_STRTOL("2147483648", NULL, 0, -1, 2147483648U, 0, + 2147483648LL, 0, 2147483648ULL, 0); + TEST_STRTOL("4294967295", NULL, 0, -1, 4294967295U, 0, + 4294967295LL, 0, 4294967295ULL, 0); + TEST_STRTOL("4294967296", NULL, 0, -1, 0U, -1, + 4294967296LL, 0, 4294967296ULL, 0); + TEST_STRTOL("9223372036854775807", NULL, 0, -1, 0U, -1, + 9223372036854775807LL, 0, 9223372036854775807ULL, 0); + TEST_STRTOL("9223372036854775808", NULL, 0, -1, 0U, -1, + 0LL, -1, 9223372036854775808ULL, 0); + TEST_STRTOL("18446744073709551615", NULL, 0, -1, 0U, -1, + 0LL, -1, 18446744073709551615ULL, 0); + TEST_STRTOL("18446744073709551616", NULL, 0, -1, 0U, -1, + 0LL, -1, 0ULL, -1); + TEST_STRTOL("18446744073709551616", "", 0, -1, 0U, -1, + 0LL, -1, 0ULL, -1); + + /* Negative bounds */ + TEST_STRTOL("-0", NULL, 0, 0, 0U, 0, 0LL, 0, 0ULL, 0); + TEST_STRTOL("-1", "", -1, 0, 4294967295U, 0, + -1LL, 0, 18446744073709551615ULL, 0); + TEST_STRTOL("-2147483647", NULL, -2147483647, 0, 2147483649U, 0, + -2147483647LL, 0, 18446744071562067969ULL, 0); + TEST_STRTOL("-2147483648", NULL, -2147483648, 0, 2147483648U, 0, + -2147483648LL, 0, 18446744071562067968ULL, 0); + TEST_STRTOL("-2147483649", NULL, 0, -1, 2147483647U, 0, + -2147483649LL, 0, 18446744071562067967ULL, 0); + TEST_STRTOL("-4294967295", NULL, 0, -1, 1U, 0, + -4294967295LL, 0, 18446744069414584321ULL, 0); + TEST_STRTOL("-4294967296", NULL, 0, -1, 0U, -1, + -4294967296LL, 0, 18446744069414584320ULL, 0); + TEST_STRTOL("-9223372036854775807", NULL, 0, -1, 0U, -1, + -9223372036854775807LL, 0, 9223372036854775809ULL, 0); + /* Bah, stupid gcc warning about -9223372036854775808LL being an + * unrepresentable integer constant */ + TEST_STRTOL("-9223372036854775808", NULL, 0, -1, 0U, -1, + 0x8000000000000000LL, 0, 9223372036854775808ULL, 0); + TEST_STRTOL("-9223372036854775809", NULL, 0, -1, 0U, -1, + 0LL, -1, 9223372036854775807ULL, 0); + TEST_STRTOL("-18446744073709551615", NULL, 0, -1, 0U, -1, + 0LL, -1, 1ULL, 0); + TEST_STRTOL("-18446744073709551616", NULL, 0, -1, 0U, -1, + 0LL, -1, 0ULL, -1); + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; }