12:08 p.m.
The commits add locale support for virStrToDouble() due to differences between
the mantissa separator in different languages. For example, kernel always uses
dot to separate mantissa. An user who is using pt_BR locale (for example) uses
comma as a separator. So, this user will have problems to parse a kernel
settings using strtod() function.
One of commits move the virDoubleToStr() to virstring.* to share locale
global variables. Joining the two functions makes more sense.
Julio Faracco (2):
util: moving virDoubleToStr() from virutil to virstring.
util: fix locale problem with virStrToDouble().
src/util/virstring.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++
src/util/virstring.h | 3 ++
src/util/virutil.c | 63 ----------------------------------------
src/util/virutil.h | 3 --
4 files changed, 84 insertions(+), 66 deletions(-)
--
2.7.4
12:08 p.m.
New subject: [libvirt] [PATCH v3 1/2] util: moving virDoubleToStr() from virutil to virstring.
The function virDoubleToStr() is defined in virutil.* and virStrToDouble() is
defined in virstring.*. Joining both functions into the same file makes more
sense.
Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com>
---
src/util/virstring.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++
src/util/virstring.h | 3 +++
src/util/virutil.c | 63 --------------------------------------------------
src/util/virutil.h | 3 ---
4 files changed, 68 insertions(+), 66 deletions(-)
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 089b539..9c12f7b 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -28,6 +28,7 @@
#include "base64.h"
#include "c-ctype.h"
#include "virstring.h"
+#include "virthread.h"
#include "viralloc.h"
#include "virbuffer.h"
#include "virerror.h"
@@ -516,6 +517,24 @@ virStrToLong_ullp(char const *s, char **end_ptr, int base,
return 0;
}
+/* In case thread-safe locales are available */
+#if HAVE_NEWLOCALE
+
+static locale_t virLocale;
+
+static int
+virLocaleOnceInit(void)
+{
+ virLocale = newlocale(LC_ALL_MASK, "C", (locale_t)0);
+ if (!virLocale)
+ return -1;
+ return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virLocale);
+#endif
+
+
int
virStrToDouble(char const *s,
char **end_ptr,
@@ -536,6 +555,52 @@ virStrToDouble(char const *s,
return 0;
}
+/**
+ * virDoubleToStr
+ *
+ * converts double to string with C locale (thread-safe).
+ *
+ * Returns -1 on error, size of the string otherwise.
+ */
+int
+virDoubleToStr(char **strp, double number)
+{
+ int ret = -1;
+
+#if HAVE_NEWLOCALE
+
+ locale_t old_loc;
+
+ if (virLocaleInitialize() < 0)
+ goto error;
+
+ old_loc = uselocale(virLocale);
+ ret = virAsprintf(strp, "%lf", number);
+ uselocale(old_loc);
+
+#else
+
+ char *radix, *tmp;
+ struct lconv *lc;
+
+ if ((ret = virAsprintf(strp, "%lf", number) < 0))
+ goto error;
+
+ lc = localeconv();
+ radix = lc->decimal_point;
+ tmp = strstr(*strp, radix);
+ if (tmp) {
+ *tmp = '.';
+ if (strlen(radix) > 1)
+ memmove(tmp + 1, tmp + strlen(radix), strlen(*strp) - (tmp - *strp));
+ }
+
+#endif /* HAVE_NEWLOCALE */
+ error:
+ return ret;
+}
+
+
int
virVasprintfInternal(bool report,
int domcode,
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 0038a40..5eaaaea 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -109,6 +109,9 @@ int virStrToDouble(char const *s,
double *result)
ATTRIBUTE_RETURN_CHECK;
+int virDoubleToStr(char **strp, double number)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+
void virSkipSpaces(const char **str) ATTRIBUTE_NONNULL(1);
void virSkipSpacesAndBackslash(const char **str) ATTRIBUTE_NONNULL(1);
void virTrimSpaces(char *str, char **endp) ATTRIBUTE_NONNULL(1);
diff --git a/src/util/virutil.c b/src/util/virutil.c
index aba7c6d..d7e01d4 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -75,7 +75,6 @@
#include "virlog.h"
#include "virbuffer.h"
#include "viralloc.h"
-#include "virthread.h"
#include "verify.h"
#include "virfile.h"
#include "vircommand.h"
@@ -437,68 +436,6 @@ int virEnumFromString(const char *const*types,
return -1;
}
-/* In case thread-safe locales are available */
-#if HAVE_NEWLOCALE
-
-static locale_t virLocale;
-
-static int
-virLocaleOnceInit(void)
-{
- virLocale = newlocale(LC_ALL_MASK, "C", (locale_t)0);
- if (!virLocale)
- return -1;
- return 0;
-}
-
-VIR_ONCE_GLOBAL_INIT(virLocale)
-#endif
-
-/**
- * virDoubleToStr
- *
- * converts double to string with C locale (thread-safe).
- *
- * Returns -1 on error, size of the string otherwise.
- */
-int
-virDoubleToStr(char **strp, double number)
-{
- int ret = -1;
-
-#if HAVE_NEWLOCALE
-
- locale_t old_loc;
-
- if (virLocaleInitialize() < 0)
- goto error;
-
- old_loc = uselocale(virLocale);
- ret = virAsprintf(strp, "%lf", number);
- uselocale(old_loc);
-
-#else
-
- char *radix, *tmp;
- struct lconv *lc;
-
- if ((ret = virAsprintf(strp, "%lf", number) < 0))
- goto error;
-
- lc = localeconv();
- radix = lc->decimal_point;
- tmp = strstr(*strp, radix);
- if (tmp) {
- *tmp = '.';
- if (strlen(radix) > 1)
- memmove(tmp + 1, tmp + strlen(radix), strlen(*strp) - (tmp - *strp));
- }
-
-#endif /* HAVE_NEWLOCALE */
- error:
- return ret;
-}
-
/**
* Format @val as a base-10 decimal number, in the
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 86e9051..4938255 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -63,9 +63,6 @@ int virParseNumber(const char **str);
int virParseVersionString(const char *str, unsigned long *version,
bool allowMissing);
-int virDoubleToStr(char **strp, double number)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
-
char *virFormatIntDecimal(char *buf, size_t buflen, int val)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
--
2.7.4
12:08 p.m.
New subject: [libvirt] [PATCH v3 2/2] util: fix locale problem with virStrToDouble().
This commit fixes a locale problem with locales that use comma as a mantissa
separator. Example: 12.34 en_US = 12,34 pt_BR. Since strtod() is a non-safe
function, virStrToDouble() will have problems to parse double numbers from
kernel settings and other double numbers from static files (XMLs, JSONs, etc).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1457634
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1457481
Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com>
---
src/util/virstring.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 9c12f7b..1bd6777 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -534,7 +534,13 @@ virLocaleOnceInit(void)
VIR_ONCE_GLOBAL_INIT(virLocale);
#endif
-
+/**
+ * virStrToDouble
+ *
+ * converts string with C locale (thread-safe) to double.
+ *
+ * Returns -1 on error or returns 0 on success.
+ */
int
virStrToDouble(char const *s,
char **end_ptr,
@@ -545,7 +551,17 @@ virStrToDouble(char const *s,
int err;
errno = 0;
+#if HAVE_NEWLOCALE
+ locale_t old_loc;
+ if (virLocaleInitialize() < 0)
+ return -1;
+
+ old_loc = uselocale(virLocale);
+#endif
val = strtod(s, &p); /* exempt from syntax-check */
+#if HAVE_NEWLOCALE
+ uselocale(old_loc);
+#endif
err = (errno || (!end_ptr && *p) || p == s);
if (end_ptr)
*end_ptr = p;
--
2.7.4
4:30 a.m.
On Wed, Jun 21, 2017 at 02:08:27PM -0300, Julio Faracco wrote:
The commits add locale support for virStrToDouble() due to differences
between
the mantissa separator in different languages. For example, kernel always uses
dot to separate mantissa. An user who is using pt_BR locale (for example) uses
comma as a separator. So, this user will have problems to parse a kernel
settings using strtod() function.
One of commits move the virDoubleToStr() to virstring.* to share locale
global variables. Joining the two functions makes more sense.
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
I'll push it in a minute. Thanks for the patches and patience!
Julio Faracco (2):
util: moving virDoubleToStr() from virutil to virstring.
util: fix locale problem with virStrToDouble().
src/util/virstring.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++
src/util/virstring.h | 3 ++
src/util/virutil.c | 63 ----------------------------------------
src/util/virutil.h | 3 --
4 files changed, 84 insertions(+), 66 deletions(-)
--
2.7.4
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
6:12 a.m.
On Thu, Jun 22, 2017 at 11:30:06 +0200, Martin Kletzander wrote:
On Wed, Jun 21, 2017 at 02:08:27PM -0300, Julio Faracco wrote:
> The commits add locale support for virStrToDouble() due to differences between
> the mantissa separator in different languages. For example, kernel always uses
> dot to separate mantissa. An user who is using pt_BR locale (for example) uses
> comma as a separator. So, this user will have problems to parse a kernel
> settings using strtod() function.
>
> One of commits move the virDoubleToStr() to virstring.* to share locale
> global variables. Joining the two functions makes more sense.
>
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
I'll push it in a minute. Thanks for the patches and patience!
Since this broke build and will require fixing. I'd prefer that the
code to set and revert the locale will be wrapped into a function rather
than scattering conditionally compiled code through the code base.
6:23 a.m.
On Thu, Jun 22, 2017 at 01:12:44PM +0200, Peter Krempa wrote:
On Thu, Jun 22, 2017 at 11:30:06 +0200, Martin Kletzander wrote:
> On Wed, Jun 21, 2017 at 02:08:27PM -0300, Julio Faracco wrote:
> > The commits add locale support for virStrToDouble() due to differences between
> > the mantissa separator in different languages. For example, kernel always uses
> > dot to separate mantissa. An user who is using pt_BR locale (for example) uses
> > comma as a separator. So, this user will have problems to parse a kernel
> > settings using strtod() function.
> >
> > One of commits move the virDoubleToStr() to virstring.* to share locale
> > global variables. Joining the two functions makes more sense.
> >
>
> Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
>
> I'll push it in a minute. Thanks for the patches and patience!
Since this broke build and will require fixing. I'd prefer that the
code to set and revert the locale will be wrapped into a function rather
than scattering conditionally compiled code through the code base.
I'm working on that, but either we need more functions to add
conditionally, or just remove the conditionally compiled code from just
one of those two functions. I'll post a fix in a while that fixes and
cleans up more stuff, so we'll see and can talk on that patch.
3:55 p.m.
Twos question related to this topic:
1. Is virstringtest missing some important tests from virstring.c? I'm
not seeing vir{StrToDouble|DoubleToStr}...
2. Should locale be considered during the test phase too?
2017-06-22 8:23 GMT-03:00 Martin Kletzander <mkletzan(a)redhat.com>:
On Thu, Jun 22, 2017 at 01:12:44PM +0200, Peter Krempa wrote:
>
> On Thu, Jun 22, 2017 at 11:30:06 +0200, Martin Kletzander wrote:
>>
>> On Wed, Jun 21, 2017 at 02:08:27PM -0300, Julio Faracco wrote:
>> > The commits add locale support for virStrToDouble() due to differences
>> > between
>> > the mantissa separator in different languages. For example, kernel
>> > always uses
>> > dot to separate mantissa. An user who is using pt_BR locale (for
>> > example) uses
>> > comma as a separator. So, this user will have problems to parse a
>> > kernel
>> > settings using strtod() function.
>> >
>> > One of commits move the virDoubleToStr() to virstring.* to share locale
>> > global variables. Joining the two functions makes more sense.
>> >
>>
>> Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
>>
>> I'll push it in a minute. Thanks for the patches and patience!
>
>
> Since this broke build and will require fixing. I'd prefer that the
> code to set and revert the locale will be wrapped into a function rather
> than scattering conditionally compiled code through the code base.
I'm working on that, but either we need more functions to add
conditionally, or just remove the conditionally compiled code from just
one of those two functions. I'll post a fix in a while that fixes and
cleans up more stuff, so we'll see and can talk on that patch.
3:17 a.m.
On Thu, Jun 22, 2017 at 05:55:12PM -0300, Julio Faracco wrote:
Twos question related to this topic:
1. Is virstringtest missing some important tests from virstring.c? I'm
not seeing vir{StrToDouble|DoubleToStr}...
2. Should locale be considered during the test phase too?
Yep, it would be nice to have a test for that. We can certainly do
that, it's just that nobody did that yet. Patches are welcome, though ;)
2712
days inactive
2714
days old
7 comments
3 participants
participants (3)
-
Julio Faracco -
Martin Kletzander -
Peter Krempa