Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
cutils: Set value in all integral qemu_strto* error paths
Our goal in writing qemu_strtoi() and friends is to have an interface
harder to abuse than libc's strtol().  Leaving the return value
uninitialized on some but not all error paths does not lend itself
well to this goal; and our documentation wasn't helpful on what to
expect.

Note that the previous patch changed all qemu_strtosz() EINVAL error
paths to slam value to 0 rather than stay uninitialized, even when the
EINVAL eror occurs because of trailing junk.  But for the remaining
integral qemu_strto*, it's easier to return the parsed value than to
force things back to zero, in part because of how check_strtox_error
works; in part because people expect that from libc strto* (while
there is no libc strtosz to compare to), and in part because doing so
creates less churn in the testsuite.

Here, the list of affected callers is much longer ('git grep
"qemu_strto[ui]" "*.c" "**/*.c" | grep -v tests/ |wc -l' outputs 107,
although a few of those are the implementation in in cutils.c), so
touching as little as possible is the wisest course of action.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-17-eblake@redhat.com>
  • Loading branch information
ebblake committed Jun 1, 2023
1 parent f7bec30 commit 9f7e421
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 28 deletions.
24 changes: 12 additions & 12 deletions tests/unit/test-cutils.c
Expand Up @@ -320,7 +320,7 @@ static void test_qemu_strtoi_null(void)
err = qemu_strtoi(NULL, &endptr, 0, &res);

g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 999);
g_assert_cmpint(res, ==, 0);
g_assert_null(endptr);
}

Expand Down Expand Up @@ -661,7 +661,7 @@ static void test_qemu_strtoi_full_null(void)
err = qemu_strtoi(NULL, &endptr, 0, &res);

g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 999);
g_assert_cmpint(res, ==, 0);
g_assert_null(endptr);
}

Expand Down Expand Up @@ -764,7 +764,7 @@ static void test_qemu_strtoui_null(void)
err = qemu_strtoui(NULL, &endptr, 0, &res);

g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpuint(res, ==, 999);
g_assert_cmpuint(res, ==, 0);
g_assert_null(endptr);
}

Expand Down Expand Up @@ -1102,7 +1102,7 @@ static void test_qemu_strtoui_full_null(void)
err = qemu_strtoui(NULL, NULL, 0, &res);

g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpuint(res, ==, 999);
g_assert_cmpuint(res, ==, 0);
}

static void test_qemu_strtoui_full_empty(void)
Expand Down Expand Up @@ -1202,7 +1202,7 @@ static void test_qemu_strtol_null(void)
err = qemu_strtol(NULL, &endptr, 0, &res);

g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 999);
g_assert_cmpint(res, ==, 0);
g_assert_null(endptr);
}

Expand Down Expand Up @@ -1516,7 +1516,7 @@ static void test_qemu_strtol_full_null(void)
err = qemu_strtol(NULL, &endptr, 0, &res);

g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 999);
g_assert_cmpint(res, ==, 0);
g_assert_null(endptr);
}

Expand Down Expand Up @@ -1619,7 +1619,7 @@ static void test_qemu_strtoul_null(void)
err = qemu_strtoul(NULL, &endptr, 0, &res);

g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpuint(res, ==, 999);
g_assert_cmpuint(res, ==, 0);
g_assert_null(endptr);
}

Expand Down Expand Up @@ -1932,7 +1932,7 @@ static void test_qemu_strtoul_full_null(void)
err = qemu_strtoul(NULL, NULL, 0, &res);

g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpuint(res, ==, 999);
g_assert_cmpuint(res, ==, 0);
}

static void test_qemu_strtoul_full_empty(void)
Expand Down Expand Up @@ -2032,7 +2032,7 @@ static void test_qemu_strtoi64_null(void)
err = qemu_strtoi64(NULL, &endptr, 0, &res);

g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 999);
g_assert_cmpint(res, ==, 0);
g_assert_null(endptr);
}

Expand Down Expand Up @@ -2322,7 +2322,7 @@ static void test_qemu_strtoi64_full_null(void)
err = qemu_strtoi64(NULL, NULL, 0, &res);

g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpint(res, ==, 999);
g_assert_cmpint(res, ==, 0);
}

static void test_qemu_strtoi64_full_empty(void)
Expand Down Expand Up @@ -2425,7 +2425,7 @@ static void test_qemu_strtou64_null(void)
err = qemu_strtou64(NULL, &endptr, 0, &res);

g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpuint(res, ==, 999);
g_assert_cmpuint(res, ==, 0);
g_assert_null(endptr);
}

Expand Down Expand Up @@ -2714,7 +2714,7 @@ static void test_qemu_strtou64_full_null(void)
err = qemu_strtou64(NULL, NULL, 0, &res);

g_assert_cmpint(err, ==, -EINVAL);
g_assert_cmpuint(res, ==, 999);
g_assert_cmpuint(res, ==, 0);
}

static void test_qemu_strtou64_full_empty(void)
Expand Down
42 changes: 26 additions & 16 deletions util/cutils.c
Expand Up @@ -384,12 +384,13 @@ static int check_strtox_error(const char *nptr, char *ep,
*
* @nptr may be null, and no conversion is performed then.
*
* If no conversion is performed, store @nptr in *@endptr and return
* -EINVAL.
* If no conversion is performed, store @nptr in *@endptr, 0 in
* @result, and return -EINVAL.
*
* If @endptr is null, and the string isn't fully converted, return
* -EINVAL. This is the case when the pointer that would be stored in
* a non-null @endptr points to a character other than '\0'.
* -EINVAL with @result set to the parsed value. This is the case
* when the pointer that would be stored in a non-null @endptr points
* to a character other than '\0'.
*
* If the conversion overflows @result, store INT_MAX in @result,
* and return -ERANGE.
Expand All @@ -410,6 +411,7 @@ int qemu_strtoi(const char *nptr, const char **endptr, int base,

assert((unsigned) base <= 36 && base != 1);
if (!nptr) {
*result = 0;
if (endptr) {
*endptr = nptr;
}
Expand Down Expand Up @@ -439,12 +441,13 @@ int qemu_strtoi(const char *nptr, const char **endptr, int base,
*
* @nptr may be null, and no conversion is performed then.
*
* If no conversion is performed, store @nptr in *@endptr and return
* -EINVAL.
* If no conversion is performed, store @nptr in *@endptr, 0 in
* @result, and return -EINVAL.
*
* If @endptr is null, and the string isn't fully converted, return
* -EINVAL. This is the case when the pointer that would be stored in
* a non-null @endptr points to a character other than '\0'.
* -EINVAL with @result set to the parsed value. This is the case
* when the pointer that would be stored in a non-null @endptr points
* to a character other than '\0'.
*
* If the conversion overflows @result, store UINT_MAX in @result,
* and return -ERANGE.
Expand All @@ -465,6 +468,7 @@ int qemu_strtoui(const char *nptr, const char **endptr, int base,

assert((unsigned) base <= 36 && base != 1);
if (!nptr) {
*result = 0;
if (endptr) {
*endptr = nptr;
}
Expand Down Expand Up @@ -508,12 +512,13 @@ int qemu_strtoui(const char *nptr, const char **endptr, int base,
*
* @nptr may be null, and no conversion is performed then.
*
* If no conversion is performed, store @nptr in *@endptr and return
* -EINVAL.
* If no conversion is performed, store @nptr in *@endptr, 0 in
* @result, and return -EINVAL.
*
* If @endptr is null, and the string isn't fully converted, return
* -EINVAL. This is the case when the pointer that would be stored in
* a non-null @endptr points to a character other than '\0'.
* -EINVAL with @result set to the parsed value. This is the case
* when the pointer that would be stored in a non-null @endptr points
* to a character other than '\0'.
*
* If the conversion overflows @result, store LONG_MAX in @result,
* and return -ERANGE.
Expand All @@ -530,6 +535,7 @@ int qemu_strtol(const char *nptr, const char **endptr, int base,

assert((unsigned) base <= 36 && base != 1);
if (!nptr) {
*result = 0;
if (endptr) {
*endptr = nptr;
}
Expand All @@ -550,12 +556,13 @@ int qemu_strtol(const char *nptr, const char **endptr, int base,
*
* @nptr may be null, and no conversion is performed then.
*
* If no conversion is performed, store @nptr in *@endptr and return
* -EINVAL.
* If no conversion is performed, store @nptr in *@endptr, 0 in
* @result, and return -EINVAL.
*
* If @endptr is null, and the string isn't fully converted, return
* -EINVAL. This is the case when the pointer that would be stored in
* a non-null @endptr points to a character other than '\0'.
* -EINVAL with @result set to the parsed value. This is the case
* when the pointer that would be stored in a non-null @endptr points
* to a character other than '\0'.
*
* If the conversion overflows @result, store ULONG_MAX in @result,
* and return -ERANGE.
Expand All @@ -573,6 +580,7 @@ int qemu_strtoul(const char *nptr, const char **endptr, int base,

assert((unsigned) base <= 36 && base != 1);
if (!nptr) {
*result = 0;
if (endptr) {
*endptr = nptr;
}
Expand Down Expand Up @@ -601,6 +609,7 @@ int qemu_strtoi64(const char *nptr, const char **endptr, int base,

assert((unsigned) base <= 36 && base != 1);
if (!nptr) {
*result = 0;
if (endptr) {
*endptr = nptr;
}
Expand Down Expand Up @@ -628,6 +637,7 @@ int qemu_strtou64(const char *nptr, const char **endptr, int base,

assert((unsigned) base <= 36 && base != 1);
if (!nptr) {
*result = 0;
if (endptr) {
*endptr = nptr;
}
Expand Down

0 comments on commit 9f7e421

Please sign in to comment.