[systemd-commits] 2 commits - TODO src/bootchart src/core src/journal src/login src/shared src/test src/tty-ask-password-agent

Lennart Poettering lennart at kemper.freedesktop.org
Wed Apr 24 20:07:11 PDT 2013


 TODO                                                |    8 ----
 src/bootchart/bootchart.c                           |    2 -
 src/core/load-dropin.c                              |    2 -
 src/core/load-fragment.c                            |    2 -
 src/core/main.c                                     |    2 -
 src/journal/journald-server.c                       |    4 +-
 src/login/logind.c                                  |   10 ++---
 src/shared/conf-parser.c                            |   17 ++++++--
 src/shared/conf-parser.h                            |    1 
 src/shared/install.c                                |    2 -
 src/shared/util.c                                   |    6 ++-
 src/shared/util.h                                   |   35 ++++++++++++++++--
 src/test/test-util.c                                |   38 +++++++++++++++++++-
 src/tty-ask-password-agent/tty-ask-password-agent.c |    2 -
 14 files changed, 98 insertions(+), 33 deletions(-)

New commits:
commit d6dd604b551987b411ec8930c23bd5c9c93ef864
Author: Lennart Poettering <lennart at poettering.net>
Date:   Thu Apr 25 00:04:02 2013 -0300

    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 3133ec7..80a591f 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 a6ec79a..38ee493 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 6575f56..d76f41e 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 83959c0..66a10ea 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) {

commit db5c0122853a9ecf1cc92e6593461932df2fa866
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Apr 24 19:53:16 2013 -0300

    conf-parser: restrict .include usage
    
    Disallow recursive .include, and make it unavailable in anything but
    unit files.

diff --git a/TODO b/TODO
index cfd42ce..3133ec7 100644
--- a/TODO
+++ b/TODO
@@ -100,10 +100,6 @@ Features:
      /lib/modules/$(uname -r)/modules.devname
   and apply ACLs to them if they have TAG=="uaccess" in udev rules.
 
-* matching against units is currently broken in journalctl. We really
-  need another AND level in the expressions,
-  i.e. sd_journal_add_conjunction().
-
 * add ConditionArchitecture= or so
 
 * teach ConditionKernelCommandLine= globs or regexes (in order to match foobar={no,0,off})
@@ -125,8 +121,6 @@ Features:
 
 * man: remove .include documentation, and instead push people to use .d/*.conf
 
-* disallow .include from included files
-
 * 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:
diff --git a/src/bootchart/bootchart.c b/src/bootchart/bootchart.c
index 002f3df..b733191 100644
--- a/src/bootchart/bootchart.c
+++ b/src/bootchart/bootchart.c
@@ -124,7 +124,7 @@ static void parse_conf(void) {
                 return;
 
         r = config_parse(NULL, BOOTCHART_CONF, f,
-                         NULL, config_item_table_lookup, (void*) items, true, NULL);
+                         NULL, config_item_table_lookup, (void*) items, true, false, NULL);
         if (r < 0)
                 log_warning("Failed to parse configuration file: %s", strerror(-r));
 
diff --git a/src/core/load-dropin.c b/src/core/load-dropin.c
index 67774d5..0318296 100644
--- a/src/core/load-dropin.c
+++ b/src/core/load-dropin.c
@@ -200,7 +200,7 @@ int unit_load_dropin(Unit *u) {
         STRV_FOREACH(f, u->dropin_paths) {
                 r = config_parse(u->id, *f, NULL,
                                  UNIT_VTABLE(u)->sections, config_item_perf_lookup,
-                                 (void*) load_fragment_gperf_lookup, false, u);
+                                 (void*) load_fragment_gperf_lookup, false, false, u);
                 if (r < 0)
                         return r;
         }
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 3d23372..e2015ed 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -2270,7 +2270,7 @@ static int load_from_path(Unit *u, const char *path) {
                 /* Now, parse the file contents */
                 r = config_parse(u->id, filename, f, UNIT_VTABLE(u)->sections,
                                  config_item_perf_lookup,
-                                 (void*) load_fragment_gperf_lookup, false, u);
+                                 (void*) load_fragment_gperf_lookup, false, true, u);
                 if (r < 0)
                         goto finish;
 
diff --git a/src/core/main.c b/src/core/main.c
index ab2ac00..695e232 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -677,7 +677,7 @@ static int parse_config_file(void) {
                 return 0;
         }
 
-        r = config_parse(NULL, fn, f, "Manager\0", config_item_table_lookup, (void*) items, false, NULL);
+        r = config_parse(NULL, fn, f, "Manager\0", config_item_table_lookup, (void*) items, false, false, NULL);
         if (r < 0)
                 log_warning("Failed to parse configuration file: %s", strerror(-r));
 
diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
index 8eab5ad..1b5a22b 100644
--- a/src/journal/journald-server.c
+++ b/src/journal/journald-server.c
@@ -1271,7 +1271,7 @@ static int server_parse_proc_cmdline(Server *s) {
 }
 
 static int server_parse_config_file(Server *s) {
-        static const char *fn = "/etc/systemd/journald.conf";
+        static const char fn[] = "/etc/systemd/journald.conf";
         _cleanup_fclose_ FILE *f = NULL;
         int r;
 
@@ -1287,7 +1287,7 @@ static int server_parse_config_file(Server *s) {
         }
 
         r = config_parse(NULL, fn, f, "Journal\0", config_item_perf_lookup,
-                         (void*) journald_gperf_lookup, false, s);
+                         (void*) journald_gperf_lookup, false, false, s);
         if (r < 0)
                 log_warning("Failed to parse configuration file: %s", strerror(-r));
 
diff --git a/src/login/logind.c b/src/login/logind.c
index 536612c..5a39440 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -1683,13 +1683,12 @@ int manager_run(Manager *m) {
 }
 
 static int manager_parse_config_file(Manager *m) {
-        FILE *f;
-        const char *fn;
+        static const char fn[] = "/etc/systemd/logind.conf";
+        _cleanup_fclose_ FILE *f = NULL;
         int r;
 
         assert(m);
 
-        fn = "/etc/systemd/logind.conf";
         f = fopen(fn, "re");
         if (!f) {
                 if (errno == ENOENT)
@@ -1699,12 +1698,11 @@ static int manager_parse_config_file(Manager *m) {
                 return -errno;
         }
 
-        r = config_parse(NULL, fn, f, "Login\0", config_item_perf_lookup, (void*) logind_gperf_lookup, false, m);
+        r = config_parse(NULL, fn, f, "Login\0", config_item_perf_lookup,
+                         (void*) logind_gperf_lookup, false, false, m);
         if (r < 0)
                 log_warning("Failed to parse configuration file: %s", strerror(-r));
 
-        fclose(f);
-
         return r;
 }
 
diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c
index 3d14c58..2303d9a 100644
--- a/src/shared/conf-parser.c
+++ b/src/shared/conf-parser.c
@@ -70,7 +70,7 @@ int log_syntax_internal(const char *unit, int level,
                                         "ERRNO=%d", error > 0 ? error : EINVAL,
                                         "MESSAGE=[%s:%u] %s", config_file, config_line, msg,
                                         NULL);
-        log_info("logged here: '%s': %d", msg, r);
+
         return r;
 }
 
@@ -199,6 +199,7 @@ static int parse_line(const char* unit,
                       ConfigItemLookup lookup,
                       void *table,
                       bool relaxed,
+                      bool allow_include,
                       char **section,
                       char *l,
                       void *userdata) {
@@ -219,13 +220,19 @@ static int parse_line(const char* unit,
                 return 0;
 
         if (startswith(l, ".include ")) {
-                _cleanup_free_ char *fn;
+                _cleanup_free_ char *fn = NULL;
+
+                if (!allow_include) {
+                        log_syntax(unit, LOG_ERR, filename, line, EBADMSG,
+                                   ".include not allowed here. Ignoring.");
+                        return 0;
+                }
 
                 fn = file_in_same_dir(filename, strstrip(l+9));
                 if (!fn)
                         return -ENOMEM;
 
-                return config_parse(unit, fn, NULL, sections, lookup, table, relaxed, userdata);
+                return config_parse(unit, fn, NULL, sections, lookup, table, relaxed, false, userdata);
         }
 
         if (*l == '[') {
@@ -299,11 +306,12 @@ int config_parse(const char *unit,
                  ConfigItemLookup lookup,
                  void *table,
                  bool relaxed,
+                 bool allow_include,
                  void *userdata) {
 
-        unsigned line = 0;
         _cleanup_free_ char *section = NULL, *continuation = NULL;
         _cleanup_fclose_ FILE *ours = NULL;
+        unsigned line = 0;
         int r;
 
         assert(filename);
@@ -370,6 +378,7 @@ int config_parse(const char *unit,
                                lookup,
                                table,
                                relaxed,
+                               allow_include,
                                &section,
                                p,
                                userdata);
diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h
index 9ea84e6..08428a5 100644
--- a/src/shared/conf-parser.h
+++ b/src/shared/conf-parser.h
@@ -87,6 +87,7 @@ int config_parse(const char *unit,
                  ConfigItemLookup lookup,
                  void *table,
                  bool relaxed,
+                 bool allow_include,
                  void *userdata);
 
 /* Generic parsers */
diff --git a/src/shared/install.c b/src/shared/install.c
index b22019d..edf4d2a 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1015,7 +1015,7 @@ static int unit_file_load(
         }
 
         r = config_parse(NULL, path, f, NULL,
-                         config_item_table_lookup, (void*) items, true, info);
+                         config_item_table_lookup, (void*) items, true, true, info);
         if (r < 0)
                 return r;
 
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c
index 6888a64..f463662 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -275,7 +275,7 @@ static int parse_password(const char *filename, char **wall) {
                 return -errno;
         }
 
-        r = config_parse(NULL, filename, f, NULL, config_item_table_lookup, (void*) items, true, NULL);
+        r = config_parse(NULL, filename, f, NULL, config_item_table_lookup, (void*) items, true, false, NULL);
         if (r < 0) {
                 log_error("Failed to parse password file %s: %s", filename, strerror(-r));
                 goto finish;



More information about the systemd-commits mailing list