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 3133ec7bdb9ca0d70bc2487365298bf6b27c6841..80a591f01779df73186023fc37c3c81c2bade7c4 100644 (file)
--- a/TODO
+++ b/TODO
@@ -121,8 +121,6 @@ Features:
 
 * man: remove .include documentation, and instead push people to use .d/*.conf
 
 
 * 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
 * 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 a6ec79a29222042be9467d2cbc3410c71899a8a8..38ee493a814a4e84f4a930557e6e8271745ba855 100644 (file)
--- 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);
 
         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;
 
         if (!x || x == s || *x || errno)
                 return errno ? -errno : -EINVAL;

diff --git a/src/shared/util.h b/src/shared/util.h
index 6575f5681199108799b54dfa5fb5059538dce9b8..d76f41e777c1d87882304998bb7c39fc0e54fde7 100644 (file)
--- 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 <sys/resource.h>
 #include <stddef.h>
 #include <unistd.h>

+#include <locale.h>

 
 #include "macro.h"
 #include "time-util.h"
 
 #include "macro.h"
 #include "time-util.h"


@@ -643,19 +644,19 @@ static inline void _reset_errno_(int *saved_errno) {
         errno = *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;
 };
 
         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)                                            \
         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)
 
              !_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_;                                                    \
         })
                 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 83959c0950194e0aa1e0dc10984ffa188bc3272e..66a10ead461489cc628fb070dd42c7b8e0ef1975 100644 (file)
--- 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 <string.h>
 #include <unistd.h>
 #include <fcntl.h>

+#include <locale.h>

 
 #include "util.h"
 
 
 #include "util.h"
 


@@ -145,13 +146,48 @@ static void test_safe_atolli(void) {
 static void test_safe_atod(void) {
         int r;
         double d;
 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("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);
         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) {
 }
 
 static void test_strappend(void) {