All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: Hanna Czenczek <hreitz@redhat.com>
Subject: [PULL 10/21] cutils: Allow NULL endptr in parse_uint()
Date: Thu,  1 Jun 2023 17:02:54 -0500	[thread overview]
Message-ID: <20230601220305.2130121-11-eblake@redhat.com> (raw)
In-Reply-To: <20230601220305.2130121-1-eblake@redhat.com>

All the qemu_strto*() functions permit a NULL endptr, just like their
libc counterparts, leaving parse_uint() as the oddball that caused
SEGFAULT on NULL and required the user to call parse_uint_full()
instead.  Relax things for consistency, even though the testsuite is
the only impacted caller.  Add one more unit test to ensure even
parse_uint_full(NULL, 0, &value) works.  This also fixes our code to
uniformly favor EINVAL over ERANGE when both apply.

Also fixes a doc mismatch @v vs. a parameter named value.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-9-eblake@redhat.com>
---
 tests/unit/test-cutils.c | 18 ++++++++++++++++--
 util/cutils.c            | 34 ++++++++++++----------------------
 2 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index 65041bd3974..20ab0ecb673 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -270,14 +270,26 @@ static void test_parse_uint_full_correct(void)

 static void test_parse_uint_full_erange_junk(void)
 {
-    /* FIXME - inconsistent with qemu_strto* which favors EINVAL */
+    /* EINVAL has priority over ERANGE */
     uint64_t i = 999;
     const char *str = "-2junk";
     int r;

     r = parse_uint_full(str, 0, &i);

-    g_assert_cmpint(r, ==, -ERANGE /* FIXME -EINVAL */);
+    g_assert_cmpint(r, ==, -EINVAL);
+    g_assert_cmpuint(i, ==, 0);
+}
+
+static void test_parse_uint_full_null(void)
+{
+    uint64_t i = 999;
+    const char *str = NULL;
+    int r;
+
+    r = parse_uint_full(str, 0, &i);
+
+    g_assert_cmpint(r, ==, -EINVAL);
     g_assert_cmpuint(i, ==, 0);
 }

@@ -3328,6 +3340,8 @@ int main(int argc, char **argv)
                     test_parse_uint_full_correct);
     g_test_add_func("/cutils/parse_uint_full/erange_junk",
                     test_parse_uint_full_erange_junk);
+    g_test_add_func("/cutils/parse_uint_full/null",
+                    test_parse_uint_full_null);

     /* qemu_strtoi() tests */
     g_test_add_func("/cutils/qemu_strtoi/correct",
diff --git a/util/cutils.c b/util/cutils.c
index 0e279a531aa..56a2aced8d4 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -722,8 +722,7 @@ const char *qemu_strchrnul(const char *s, int c)
  * parse_uint:
  *
  * @s: String to parse
- * @endptr: Destination for pointer to first character not consumed, must
- * not be %NULL
+ * @endptr: Destination for pointer to first character not consumed
  * @base: integer base, between 2 and 36 inclusive, or 0
  * @value: Destination for parsed integer value
  *
@@ -737,7 +736,8 @@ const char *qemu_strchrnul(const char *s, int c)
  *
  * Set *@endptr to point right beyond the parsed integer (even if the integer
  * overflows or is negative, all digits will be parsed and *@endptr will
- * point right beyond them).
+ * point right beyond them).  If @endptr is %NULL, any trailing character
+ * instead causes a result of -EINVAL with *@value of 0.
  *
  * If the integer is negative, set *@value to 0, and return -ERANGE.
  * (If you want to allow negative numbers that wrap around within
@@ -784,7 +784,12 @@ int parse_uint(const char *s, const char **endptr, int base, uint64_t *value)

 out:
     *value = val;
-    *endptr = endp;
+    if (endptr) {
+        *endptr = endp;
+    } else if (s && *endp) {
+        r = -EINVAL;
+        *value = 0;
+    }
     return r;
 }

@@ -795,28 +800,13 @@ out:
  * @base: integer base, between 2 and 36 inclusive, or 0
  * @value: Destination for parsed integer value
  *
- * Parse unsigned integer from entire string
+ * Parse unsigned integer from entire string, rejecting any trailing slop.
  *
- * Have the same behavior of parse_uint(), but with an additional
- * check for additional data after the parsed number. If extra
- * characters are present after a non-overflowing parsed number, the
- * function will return -EINVAL, and *@v will be set to 0.
+ * Shorthand for parse_uint(s, NULL, base, value).
  */
 int parse_uint_full(const char *s, int base, uint64_t *value)
 {
-    const char *endp;
-    int r;
-
-    r = parse_uint(s, &endp, base, value);
-    if (r < 0) {
-        return r;
-    }
-    if (*endp) {
-        *value = 0;
-        return -EINVAL;
-    }
-
-    return 0;
+    return parse_uint(s, NULL, base, value);
 }

 int qemu_parse_fd(const char *param)
-- 
2.40.1



  parent reply	other threads:[~2023-06-01 22:05 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 ` Eric Blake [this message]
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 ` [PULL 18/21] cutils: Set value in all integral qemu_strto* " Eric Blake
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-11-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.