qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: Hanna Czenczek <hreitz@redhat.com>
Subject: [PULL 18/21] cutils: Set value in all integral qemu_strto* error paths
Date: Thu,  1 Jun 2023 17:03:02 -0500	[thread overview]
Message-ID: <20230601220305.2130121-19-eblake@redhat.com> (raw)
In-Reply-To: <20230601220305.2130121-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
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>
---
 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 2189ebc92f3..e5b780672d1 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -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);
 }

@@ -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);
 }

@@ -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);
 }

@@ -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)
@@ -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);
 }

@@ -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);
 }

@@ -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);
 }

@@ -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)
@@ -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);
 }

@@ -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)
@@ -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);
 }

@@ -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)
diff --git a/util/cutils.c b/util/cutils.c
index c5530a5c2be..edfb71a2171 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.
@@ -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;
         }
@@ -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.
@@ -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;
         }
@@ -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.
@@ -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;
         }
@@ -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.
@@ -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;
         }
@@ -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;
         }
@@ -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;
         }
-- 
2.40.1



  parent reply	other threads:[~2023-06-01 22:06 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-01 22:02 [PULL 00/21] NBD and miscellaneous patches for 2023-06-01 Eric Blake
2023-06-01 22:02 ` [PULL 01/21] iotests: Fix test 104 under NBD Eric Blake
2023-06-01 22:02 ` [PULL 02/21] qcow2: Explicit mention of padding bytes Eric Blake
2023-06-01 22:02 ` [PULL 03/21] test-cutils: Avoid g_assert in unit tests Eric Blake
2023-06-01 22:02 ` [PULL 04/21] test-cutils: Use g_assert_cmpuint where appropriate Eric Blake
2023-06-01 22:02 ` [PULL 05/21] test-cutils: Test integral qemu_strto* value on failures Eric Blake
2023-06-01 22:02 ` [PULL 06/21] test-cutils: Test more integer corner cases Eric Blake
2023-06-02 12:34   ` Eric Blake
2023-06-01 22:02 ` [PULL 07/21] cutils: Fix wraparound parsing in qemu_strtoui Eric Blake
2023-06-03  8:17   ` Michael Tokarev
2023-06-05 13:32     ` Eric Blake
2023-06-01 22:02 ` [PULL 08/21] cutils: Document differences between parse_uint and qemu_strtou64 Eric Blake
2023-06-01 22:02 ` [PULL 09/21] cutils: Adjust signature of parse_uint[_full] Eric Blake
2023-06-02  6:16   ` Markus Armbruster
2023-06-02 12:22     ` Eric Blake
2023-06-01 22:02 ` [PULL 10/21] cutils: Allow NULL endptr in parse_uint() Eric Blake
2023-06-01 22:02 ` [PULL 11/21] test-cutils: Add coverage of qemu_strtod Eric Blake
2023-06-01 22:02 ` [PULL 12/21] test-cutils: Prepare for upcoming semantic change in qemu_strtosz Eric Blake
2023-06-01 22:02 ` [PULL 13/21] test-cutils: Refactor qemu_strtosz tests for less boilerplate Eric Blake
2023-06-01 22:02 ` [PULL 14/21] cutils: Allow NULL str in qemu_strtosz Eric Blake
2023-06-01 22:02 ` [PULL 15/21] numa: Check for qemu_strtosz_MiB error Eric Blake
2023-06-01 22:03 ` [PULL 16/21] test-cutils: Add more coverage to qemu_strtosz Eric Blake
2023-06-01 22:03 ` [PULL 17/21] cutils: Set value in all qemu_strtosz* error paths Eric Blake
2023-06-01 22:03 ` Eric Blake [this message]
2023-06-01 22:03 ` [PULL 19/21] cutils: Use parse_uint in qemu_strtosz for negative rejection Eric Blake
2023-06-01 22:03 ` [PULL 20/21] cutils: Improve qemu_strtod* error paths Eric Blake
2023-06-01 22:03 ` [PULL 21/21] cutils: Improve qemu_strtosz handling of fractions Eric Blake
2023-06-02  3:58 ` [PULL 00/21] NBD and miscellaneous patches for 2023-06-01 Richard Henderson
2023-06-02 12:27   ` Eric Blake
2023-06-02  6:32 ` Conclusion of yet another expensive UI folly (was: [PULL 00/21] NBD and miscellaneous patches for 2023-06-01) Markus Armbruster
2023-06-02 12:29   ` Eric Blake
2023-06-02 13:02     ` Conclusion of yet another expensive UI folly Markus Armbruster

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=20230601220305.2130121-19-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).