[PATCH 2/2] virstring: Prefer strtoll_l() and strtoull_l() whenever possible
Michal Privoznik
mprivozn at redhat.com
Mon Jun 19 12:03:48 UTC 2023
It's weird that in 2023 there's no reliable and portable way to
parse strings, but okay.
We started with strtol(). Except, it doesn't work the same on
Linux and Windows. On Windows it behaves a bit different when it
comes to parsing strings with base = 0. So we've switched to
g_ascii_strtoll() which promised to solve this. And it did.
Except, it introduced another problem. I don't really understand
why, but I've seen random test failures and all of them claimed
inability to parse a number (specifically <memory/> from the
hard coded domain XML from test driver, which IMO is just a
coincidence because it's the first number we parse).
Anyway, it's not 100% reproducible and usually occurs when my
system is under heavy load, but there's a way to reproduce it:
$ meson test -C _build --repeat 2000 virshtest virsh-vcpupin
The nature of the problem suggests there's a race condition
somewhere. I suspected glib but I don't know their code well to
prove it.
Anyway, using strtoll_l() directly and taking glib out of the
picture solves the issue.
Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
meson.build | 1 +
src/util/virstring.c | 82 +++++++++++++++++++++++++++++++++++++++++---
2 files changed, 79 insertions(+), 4 deletions(-)
diff --git a/meson.build b/meson.build
index aa391e7178..cd713795f5 100644
--- a/meson.build
+++ b/meson.build
@@ -596,6 +596,7 @@ functions = [
'sched_setscheduler',
'setgroups',
'setrlimit',
+ 'strtoll_l',
'symlink',
'sysctlbyname',
]
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 1a13570d30..076e885624 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -127,8 +127,17 @@ virStrToLong_i(char const *s, char **end_ptr, int base, int *result)
char *p;
int err;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ if (virLocaleInitialize() < 0)
+ return -1;
+#endif
+
errno = 0;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ val = strtoll_l(s, &p, base, virLocaleRaw);
+#else
val = g_ascii_strtoll(s, &p, base);
+#endif
err = (errno || (!end_ptr && *p) || p == s || (int) val != val);
if (end_ptr)
*end_ptr = p;
@@ -148,13 +157,23 @@ virStrToLong_ui(char const *s, char **end_ptr, int base, unsigned int *result)
char *p;
bool err = false;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ if (virLocaleInitialize() < 0)
+ return -1;
+#endif
+
errno = 0;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ val = strtoull_l(s, &p, base, virLocaleRaw);
+#else
val = g_ascii_strtoull(s, &p, base);
+#endif
/* 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
- * g_ascii_strtoull treats "-1" as ULLONG_MAX, and going from ullong back
- * to uint differs depending on the size of uint. */
+ * strtoull_l/g_ascii_strtoull treats "-1" as ULLONG_MAX, and
+ * going from ullong back to uint differs depending on the size of
+ * uint. */
if (memchr(s, '-', p - s)) {
if (-val > UINT_MAX)
err = true;
@@ -180,8 +199,17 @@ virStrToLong_uip(char const *s, char **end_ptr, int base, unsigned int *result)
char *p;
bool err = false;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ if (virLocaleInitialize() < 0)
+ return -1;
+#endif
+
errno = 0;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ val = strtoull_l(s, &p, base, virLocaleRaw);
+#else
val = g_ascii_strtoull(s, &p, base);
+#endif
err = (memchr(s, '-', p - s) ||
errno || (!end_ptr && *p) || p == s || (unsigned int) val != val);
if (end_ptr)
@@ -204,13 +232,23 @@ virStrToLong_ul(char const *s, char **end_ptr, int base, unsigned long *result)
char *p;
bool err = false;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ if (virLocaleInitialize() < 0)
+ return -1;
+#endif
+
errno = 0;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ val = strtoull_l(s, &p, base, virLocaleRaw);
+#else
val = g_ascii_strtoull(s, &p, base);
+#endif
/* This one's tricky. We _want_ to allow "-1" as shorthand for
* ULONG_MAX regardless of whether long is 32-bit or 64-bit. But
- * g_ascii_strtoull treats "-1" as ULLONG_MAX, and going from ullong back
- * to ulong differs depending on the size of ulong. */
+ * strtoull_l/g_ascii_strtoull treats "-1" as ULLONG_MAX, and
+ * going from ullong back to ulong differs depending on the size
+ * of ulong. */
if (memchr(s, '-', p - s)) {
if (-val > ULONG_MAX)
err = true;
@@ -237,8 +275,17 @@ virStrToLong_ulp(char const *s, char **end_ptr, int base,
char *p;
int err;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ if (virLocaleInitialize() < 0)
+ return -1;
+#endif
+
errno = 0;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ val = strtoull_l(s, &p, base, virLocaleRaw);
+#else
val = g_ascii_strtoull(s, &p, base);
+#endif
err = (memchr(s, '-', p - s) ||
errno || (!end_ptr && *p) || p == s || (unsigned long) val != val);
if (end_ptr)
@@ -257,8 +304,17 @@ virStrToLong_ll(char const *s, char **end_ptr, int base, long long *result)
char *p;
int err;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ if (virLocaleInitialize() < 0)
+ return -1;
+#endif
+
errno = 0;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ val = strtoll_l(s, &p, base, virLocaleRaw);
+#else
val = g_ascii_strtoll(s, &p, base);
+#endif
err = (errno || (!end_ptr && *p) || p == s);
if (end_ptr)
*end_ptr = p;
@@ -279,8 +335,17 @@ virStrToLong_ull(char const *s, char **end_ptr, int base,
char *p;
int err;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ if (virLocaleInitialize() < 0)
+ return -1;
+#endif
+
errno = 0;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ val = strtoull_l(s, &p, base, virLocaleRaw);
+#else
val = g_ascii_strtoull(s, &p, base);
+#endif
err = (errno || (!end_ptr && *p) || p == s);
if (end_ptr)
*end_ptr = p;
@@ -300,8 +365,17 @@ virStrToLong_ullp(char const *s, char **end_ptr, int base,
char *p;
int err;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ if (virLocaleInitialize() < 0)
+ return -1;
+#endif
+
errno = 0;
+#if defined(WITH_STRTOLL_L) && defined(WITH_NEWLOCALE)
+ val = strtoull_l(s, &p, base, virLocaleRaw);
+#else
val = g_ascii_strtoull(s, &p, base);
+#endif
err = (memchr(s, '-', p - s) ||
errno || (!end_ptr && *p) || p == s);
if (end_ptr)
--
2.39.3
More information about the libvir-list
mailing list