From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: hreitz@redhat.com
Subject: [PATCH 09/11] cutils: Set value in all integral qemu_strto* error paths
Date: Mon, 8 May 2023 15:03:41 -0500 [thread overview]
Message-ID: <20230508200343.791450-10-eblake@redhat.com> (raw)
In-Reply-To: <20230508200343.791450-1-eblake@redhat.com>
Our goal in writing qemu_strtoi() and friends is to have an interface
harder to abuse than libc's strtol(). Leaving the return value
initialized on some error paths does not lend itself well to this
goal; and our documentation wasn't helpful on the matter.
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; and 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 87,
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>
---
tests/unit/test-cutils.c | 24 +++++++++++------------
util/cutils.c | 42 +++++++++++++++++++++++++---------------
2 files changed, 38 insertions(+), 28 deletions(-)
diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 9cf00a810e4..2cb33e41ae4 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -250,7 +250,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);
}
@@ -479,7 +479,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);
}
@@ -557,7 +557,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);
}
@@ -784,7 +784,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)
@@ -860,7 +860,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);
}
@@ -1087,7 +1087,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);
}
@@ -1165,7 +1165,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);
}
@@ -1390,7 +1390,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)
@@ -1466,7 +1466,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);
}
@@ -1691,7 +1691,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)
@@ -1769,7 +1769,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);
}
@@ -1994,7 +1994,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)
diff --git a/util/cutils.c b/util/cutils.c
index 8bacf349383..83948926ec9 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -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.
@@ -407,6 +408,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;
}
@@ -436,12 +438,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.
@@ -460,6 +463,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;
}
@@ -495,12 +499,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.
@@ -517,6 +522,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;
}
@@ -537,12 +543,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.
@@ -560,6 +567,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;
}
@@ -588,6 +596,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;
}
@@ -613,6 +622,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;
}
--
2.40.1
next prev parent reply other threads:[~2023-05-08 20:06 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-08 20:03 [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Eric Blake
2023-05-08 20:03 ` [PATCH 01/11] test-cutils: Avoid g_assert in unit tests Eric Blake
2023-05-08 20:03 ` [PATCH 02/11] test-cutils: Use g_assert_cmpuint where appropriate Eric Blake
2023-05-08 20:03 ` [PATCH 03/11] test-cutils: Test integral qemu_strto* value on failures Eric Blake
2023-05-08 20:03 ` [PATCH 04/11] test-cutils: Add coverage of qemu_strtod Eric Blake
2023-05-08 20:03 ` [PATCH 05/11] test-cutils: Prepare for upcoming semantic change in qemu_strtosz Eric Blake
2023-05-08 20:03 ` [PATCH 06/11] test-cutils: Add more coverage to qemu_strtosz Eric Blake
2023-05-09 12:31 ` Hanna Czenczek
2023-05-09 12:42 ` Hanna Czenczek
2023-05-09 16:06 ` Eric Blake
2023-05-09 15:15 ` Hanna Czenczek
2023-05-09 15:50 ` Eric Blake
2023-05-09 16:10 ` Eric Blake
2023-05-08 20:03 ` [PATCH 07/11] numa: Check for qemu_strtosz_MiB error Eric Blake
2023-05-08 21:15 ` Eric Blake
2023-05-08 20:03 ` [PATCH 08/11] cutils: Set value in all qemu_strtosz* error paths Eric Blake
2023-05-08 20:03 ` Eric Blake [this message]
2023-05-08 20:03 ` [PATCH 10/11] cutils: Improve qemu_strtod* " Eric Blake
2023-05-08 20:03 ` [PATCH 11/11] cutils: Improve qemu_strtosz handling of fractions Eric Blake
2023-05-08 21:21 ` Eric Blake
2023-05-09 17:54 ` Hanna Czenczek
2023-05-09 21:28 ` Eric Blake
2023-05-10 7:46 ` Hanna Czenczek
2023-05-10 7:48 ` Hanna Czenczek
2023-05-09 17:55 ` [PATCH 00/11] Fix qemu_strtosz() read-out-of-bounds Hanna Czenczek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230508200343.791450-10-eblake@redhat.com \
--to=eblake@redhat.com \
--cc=hreitz@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).