From: Lennart Poettering <lennart@poettering.net>
Date: Thu, 25 Apr 2013 03:04:02 +0000 (-0300)
Subject: util: rework safe_atod() to be locale-independent
X-Git-Tag: v203~82
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=d6dd604b551987b411ec8930c23bd5c9c93ef864

util: rework safe_atod() to be locale-independent

This adds some syntactic sugar with a macro RUN_WITH_LOCALE() that reset
the thread-specific locale temporarily.
---

diff --git a/TODO b/TODO
index 3133ec7bd..80a591f01 100644
--- a/TODO
+++ b/TODO
@@ -121,8 +121,6 @@ Features:
 
 * man: remove .include documentation, and instead push people to use .d/*.conf
 
-* safe_atod() is too naive, as it is vulnerable to locale parameters, should be locale independent.
-
 * think about requeuing jobs when daemon-reload is issued? usecase:
   the initrd issues a reload after fstab from the host is accessible
   and we might want to requeue the mounts local-fs acquired through
diff --git a/src/shared/util.c b/src/shared/util.c
index a6ec79a29..38ee493a8 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -372,8 +372,10 @@ int safe_atod(const char *s, double *ret_d) {
         assert(s);
         assert(ret_d);
 
-        errno = 0;
-        d = strtod(s, &x);
+        RUN_WITH_LOCALE(LC_NUMERIC_MASK, "C") {
+                errno = 0;
+                d = strtod(s, &x);
+        }
 
         if (!x || x == s || *x || errno)
                 return errno ? -errno : -EINVAL;
diff --git a/src/shared/util.h b/src/shared/util.h
index 6575f5681..d76f41e77 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -38,6 +38,7 @@
 #include <sys/resource.h>
 #include <stddef.h>
 #include <unistd.h>
+#include <locale.h>
 
 #include "macro.h"
 #include "time-util.h"
@@ -643,19 +644,19 @@ static inline void _reset_errno_(int *saved_errno) {
         errno = *saved_errno;
 }
 
-#define PROTECT_ERRNO __attribute__((cleanup(_reset_errno_))) int _saved_errno_ = errno
+#define PROTECT_ERRNO _cleanup_(_reset_errno_) int _saved_errno_ = errno
 
-struct umask_struct {
+struct _umask_struct_ {
         mode_t mask;
         bool quit;
 };
 
-static inline void _reset_umask_(struct umask_struct *s) {
+static inline void _reset_umask_(struct _umask_struct_ *s) {
         umask(s->mask);
 };
 
 #define RUN_WITH_UMASK(mask)                                            \
-        for (__attribute__((cleanup(_reset_umask_))) struct umask_struct _saved_umask_ = { umask(mask), false }; \
+        for (_cleanup_(_reset_umask_) struct _umask_struct_ _saved_umask_ = { umask(mask), false }; \
              !_saved_umask_.quit ;                                      \
              _saved_umask_.quit = true)
 
@@ -706,3 +707,29 @@ int unlink_noerrno(const char *path);
                 sprintf(_r_, "/proc/%lu/" field, (unsigned long) _pid_); \
                 _r_;                                                    \
         })
+
+struct _locale_struct_ {
+        locale_t saved_locale;
+        locale_t new_locale;
+        bool quit;
+};
+
+static inline void _reset_locale_(struct _locale_struct_ *s) {
+        PROTECT_ERRNO;
+        if (s->saved_locale != (locale_t) 0)
+                uselocale(s->saved_locale);
+        if (s->new_locale != (locale_t) 0)
+                freelocale(s->new_locale);
+}
+
+#define RUN_WITH_LOCALE(mask, loc) \
+        for (_cleanup_(_reset_locale_) struct _locale_struct_ _saved_locale_ = { (locale_t) 0, (locale_t) 0, false }; \
+             ({                                                         \
+                     if (!_saved_locale_.quit) {                        \
+                             PROTECT_ERRNO;                             \
+                             _saved_locale_.new_locale = newlocale((mask), (loc), (locale_t) 0); \
+                             if (_saved_locale_.new_locale != (locale_t) 0)     \
+                                     _saved_locale_.saved_locale = uselocale(_saved_locale_.new_locale); \
+                     }                                                  \
+                     !_saved_locale_.quit; }) ;                         \
+             _saved_locale_.quit = true)
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 83959c095..66a10ead4 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -23,6 +23,7 @@
 #include <string.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <locale.h>
 
 #include "util.h"
 
@@ -145,13 +146,48 @@ static void test_safe_atolli(void) {
 static void test_safe_atod(void) {
         int r;
         double d;
+        char *e;
+
+        r = safe_atod("junk", &d);
+        assert_se(r == -EINVAL);
 
         r = safe_atod("0.2244", &d);
         assert_se(r == 0);
         assert_se(abs(d - 0.2244) < 0.000001);
 
-        r = safe_atod("junk", &d);
+        r = safe_atod("0,5", &d);
         assert_se(r == -EINVAL);
+
+        errno = 0;
+        strtod("0,5", &e);
+        assert_se(*e == ',');
+
+        /* Check if this really is locale independent */
+        setlocale(LC_NUMERIC, "de_DE.utf8");
+
+        r = safe_atod("0.2244", &d);
+        assert_se(r == 0);
+        assert_se(abs(d - 0.2244) < 0.000001);
+
+        r = safe_atod("0,5", &d);
+        assert_se(r == -EINVAL);
+
+        errno = 0;
+        assert_se(abs(strtod("0,5", &e) - 0.5) < 0.00001);
+
+        /* And check again, reset */
+        setlocale(LC_NUMERIC, "C");
+
+        r = safe_atod("0.2244", &d);
+        assert_se(r == 0);
+        assert_se(abs(d - 0.2244) < 0.000001);
+
+        r = safe_atod("0,5", &d);
+        assert_se(r == -EINVAL);
+
+        errno = 0;
+        strtod("0,5", &e);
+        assert_se(*e == ',');
 }
 
 static void test_strappend(void) {