[systemd-commits] 7 commits - Makefile.am man/journalctl.xml src/bootchart src/journal src/shared src/test src/udev TODO

Zbigniew Jędrzejewski-Szmek zbyszek at kemper.freedesktop.org
Thu Mar 28 20:53:39 PDT 2013


 Makefile.am                |   15 ++-
 TODO                       |    2 
 man/journalctl.xml         |   14 ++
 src/bootchart/store.c      |   11 +-
 src/journal/catalog.c      |  214 ++++++++++++++++++++++++---------------------
 src/journal/catalog.h      |   16 ++-
 src/journal/journalctl.c   |   47 +++++++--
 src/journal/sd-journal.c   |    4 
 src/journal/test-catalog.c |   99 +++++++++++++++++++-
 src/shared/conf-files.c    |    2 
 src/shared/conf-files.h    |    2 
 src/shared/path-util.h     |    5 -
 src/shared/util.c          |   19 +++
 src/shared/util.h          |    1 
 src/shared/utmp-wtmp.c     |    4 
 src/test/test-path-util.c  |   85 +++++++++++++++++
 src/udev/udevadm-hwdb.c    |    2 
 src/udev/udevd.c           |    6 +
 18 files changed, 402 insertions(+), 146 deletions(-)

New commits:
commit 76877b46b652fbfcf8618458556178717b815cd8
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Mar 28 22:03:53 2013 -0400

    tests: add some silly tests for path-util.c

diff --git a/Makefile.am b/Makefile.am
index 2eae877..86ad168 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1071,6 +1071,7 @@ noinst_tests += \
 	test-env-replace \
 	test-strbuf \
 	test-strv \
+	test-path-util \
 	test-strxcpyx \
 	test-unit-name \
 	test-unit-file \
@@ -1259,6 +1260,12 @@ test_strv_LDADD = \
 	libsystemd-units.la \
 	libsystemd-id128-internal.la
 
+test_path_util_SOURCES = \
+	src/test/test-path-util.c
+
+test_path_util_LDADD = \
+	libsystemd-shared.la
+
 test_strxcpyx_SOURCES = \
 	src/test/test-strxcpyx.c
 
diff --git a/src/shared/path-util.h b/src/shared/path-util.h
index ff52394..9347bc3 100644
--- a/src/shared/path-util.h
+++ b/src/shared/path-util.h
@@ -1,7 +1,6 @@
 /*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
 
-#ifndef foopathutilhfoo
-#define foopathutilhfoo
+#pragma once
 
 /***
   This file is part of systemd.
@@ -41,5 +40,3 @@ char **path_strv_canonicalize_uniq(char **l);
 
 int path_is_mount_point(const char *path, bool allow_symlink);
 int path_is_read_only_fs(const char *path);
-
-#endif
diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
new file mode 100644
index 0000000..2bca5ef
--- /dev/null
+++ b/src/test/test-path-util.c
@@ -0,0 +1,85 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2013 Zbigniew Jędrzejewski-Szmek
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "path-util.h"
+#include "util.h"
+#include "macro.h"
+
+
+static void test_path(void) {
+        assert_se(path_equal("/goo", "/goo"));
+        assert_se(path_equal("//goo", "/goo"));
+        assert_se(path_equal("//goo/////", "/goo"));
+        assert_se(path_equal("goo/////", "goo"));
+
+        assert_se(path_equal("/goo/boo", "/goo//boo"));
+        assert_se(path_equal("//goo/boo", "/goo/boo//"));
+
+        assert_se(path_equal("/", "///"));
+
+        assert_se(!path_equal("/x", "x/"));
+        assert_se(!path_equal("x/", "/"));
+
+        assert_se(!path_equal("/x/./y", "x/y"));
+        assert_se(!path_equal("x/.y", "x/y"));
+
+        assert_se(path_is_absolute("/"));
+        assert_se(!path_is_absolute("./"));
+
+        assert_se(is_path("/dir"));
+        assert_se(is_path("a/b"));
+        assert_se(!is_path("."));
+
+        assert_se(streq(path_get_file_name("./aa/bb/../file.da."), "file.da."));
+        assert_se(streq(path_get_file_name("/aa///.file"), ".file"));
+        assert_se(streq(path_get_file_name("/aa///file..."), "file..."));
+        assert_se(streq(path_get_file_name("file.../"), "."));
+
+#define test_parent(x, y) {                     \
+                char *z;                        \
+                int r = path_get_parent(x, &z); \
+                assert_se(r==0);                \
+                assert_se(streq(z, y));         \
+        }
+
+        test_parent("./aa/bb/../file.da.", "./aa/bb/..");
+        test_parent("/aa///.file", "/aa///");
+        test_parent("/aa///file...", "/aa///");
+        test_parent("file.../", "file...");
+
+        assert_se(path_is_mount_point("/", true));
+        assert_se(path_is_mount_point("/", false));
+
+        {
+                char p1[] = "aaa/bbb////ccc";
+                char p2[] = "//aaa/.////ccc";
+                char p3[] = "/./";
+
+                assert(path_equal(path_kill_slashes(p1), "aaa/bbb/ccc"));
+                assert(path_equal(path_kill_slashes(p2), "/aaa/./ccc"));
+                assert(path_equal(path_kill_slashes(p3), "/./"));
+        }
+}
+
+int main(void) {
+        test_path();
+        return 0;
+}

commit 13cbf3a5f0cf4a1d89413d0ffc4a9067b1d6d1a8
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Mar 28 21:44:00 2013 -0400

    journalctl: support --root for message catalogs

diff --git a/TODO b/TODO
index e54f84b..0dbaad5 100644
--- a/TODO
+++ b/TODO
@@ -144,8 +144,6 @@ Features:
 
 * timedate: have global on/off switches for auto-time (NTP), and auto-timezone that connman can subscribe to.
 
-* support --root= in msgcatalog compiler
-
 * Honour "-" prefix for InaccessibleDirectories= and ReadOnlyDirectories= to
   suppress errors of the specified path doesn't exist
 
diff --git a/man/journalctl.xml b/man/journalctl.xml
index 6b4b757..144b54f 100644
--- a/man/journalctl.xml
+++ b/man/journalctl.xml
@@ -448,6 +448,20 @@
                         </varlistentry>
 
                         <varlistentry>
+                                <term><option>--root=<replaceable>ROOT</replaceable></option></term>
+
+                                <listitem><para>Takes a directory path
+                                as argument. If specified journalctl
+                                will operate on catalog file hierarchy
+                                underneath the specified directory
+                                instead of the root directory
+                                (e.g. <option>--update-catalog</option>
+                                will create
+                                <filename><replaceable>ROOT</replaceable>/var/lib/systemd/catalog/database</filename>).
+                                </para></listitem>
+                        </varlistentry>
+
+                        <varlistentry>
                                 <term><option>--new-id128</option></term>
 
                                 <listitem><para>Instead of showing
diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index 03daafe..3ae6482 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -86,6 +86,7 @@ static bool arg_unit_system;
 static const char *arg_field = NULL;
 static bool arg_catalog = false;
 static bool arg_reverse = false;
+static const char *arg_root = NULL;
 
 static enum {
         ACTION_SHOW,
@@ -125,6 +126,7 @@ static int help(void) {
                "     --no-pager          Do not pipe output into a pager\n"
                "  -m --merge             Show entries from all available journals\n"
                "  -D --directory=PATH    Show journal files from directory\n"
+               "     --root=ROOT         Operate on catalog files underneath the root ROOT\n"
 #ifdef HAVE_GCRYPT
                "     --interval=TIME     Time interval for changing the FSS sealing key\n"
                "     --verify-key=KEY    Specify FSS verification key\n"
@@ -155,6 +157,7 @@ static int parse_argv(int argc, char *argv[]) {
                 ARG_NO_PAGER,
                 ARG_NO_TAIL,
                 ARG_NEW_ID128,
+                ARG_ROOT,
                 ARG_HEADER,
                 ARG_FULL,
                 ARG_SETUP_KEYS,
@@ -186,6 +189,7 @@ static int parse_argv(int argc, char *argv[]) {
                 { "merge",        no_argument,       NULL, 'm'              },
                 { "this-boot",    no_argument,       NULL, 'b'              },
                 { "directory",    required_argument, NULL, 'D'              },
+                { "root",         required_argument, NULL, ARG_ROOT         },
                 { "header",       no_argument,       NULL, ARG_HEADER       },
                 { "priority",     required_argument, NULL, 'p'              },
                 { "setup-keys",   no_argument,       NULL, ARG_SETUP_KEYS   },
@@ -318,6 +322,10 @@ static int parse_argv(int argc, char *argv[]) {
                         arg_directory = optarg;
                         break;
 
+                case ARG_ROOT:
+                        arg_root = optarg;
+                        break;
+
                 case 'c':
                         arg_cursor = optarg;
                         break;
@@ -1024,18 +1032,25 @@ int main(int argc, char *argv[]) {
             arg_action == ACTION_LIST_CATALOG ||
             arg_action == ACTION_DUMP_CATALOG) {
 
+                char _cleanup_free_ *database;
+                database =  strjoin(arg_root, "/", CATALOG_DATABASE, NULL);
+                if (!database) {
+                        r = log_oom();
+                        goto finish;
+                }
+
                 if (arg_action == ACTION_UPDATE_CATALOG) {
-                        r = catalog_update(CATALOG_DATABASE, NULL, catalog_file_dirs);
+                        r = catalog_update(database, arg_root, catalog_file_dirs);
                         if (r < 0)
                                 log_error("Failed to list catalog: %s", strerror(-r));
                 } else {
                         bool oneline = arg_action == ACTION_LIST_CATALOG;
 
                         if (optind < argc)
-                                r = catalog_list_items(stdout, CATALOG_DATABASE,
+                                r = catalog_list_items(stdout, database,
                                                        oneline, argv + optind);
                         else
-                                r = catalog_list(stdout, CATALOG_DATABASE, oneline);
+                                r = catalog_list(stdout, database, oneline);
                         if (r < 0)
                                 log_error("Failed to list catalog: %s", strerror(-r));
                 }

commit 844ec79b3c2f246114ea316ebe1f36044bdb688e
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Mar 28 20:17:24 2013 -0400

    catalog: open up catalog internals
    
    In order to write tests for the catalog functions, they
    are made non-static and start taking a 'database' parameter,
    which is the name of a file with the preprocessed catalog
    entries.
    
    This makes it possible to make test-catalog part of the
    normal test suite, since it now only operates on files
    in /tmp.
    
    Some more tests are added.

diff --git a/Makefile.am b/Makefile.am
index 923f81b..2eae877 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -138,7 +138,7 @@ AM_CPPFLAGS = \
 	-DUSER_CONFIG_FILE=\"$(pkgsysconfdir)/user.conf\" \
 	-DUSER_CONFIG_UNIT_PATH=\"$(pkgsysconfdir)/user\" \
 	-DUSER_DATA_UNIT_PATH=\"$(userunitdir)\" \
-	-DCATALOG_PATH=\"$(catalogstatedir)\" \
+	-DCATALOG_DATABASE=\"$(catalogstatedir)/database\" \
 	-DSYSTEMD_CGROUP_AGENT_PATH=\"$(rootlibexecdir)/systemd-cgroups-agent\" \
 	-DSYSTEMD_BINARY_PATH=\"$(rootlibexecdir)/systemd\" \
 	-DSYSTEMD_SHUTDOWN_BINARY_PATH=\"$(rootlibexecdir)/systemd-shutdown\" \
@@ -2748,8 +2748,7 @@ UNINSTALL_DATA_HOOKS += \
 	catalog-remove-hook
 
 noinst_PROGRAMS += \
-	test-journal-enum \
-	test-catalog
+	test-journal-enum
 
 noinst_tests += \
 	test-journal \
@@ -2758,7 +2757,8 @@ noinst_tests += \
 	test-journal-match \
 	test-journal-stream \
 	test-journal-verify \
-	test-mmap-cache
+	test-mmap-cache \
+	test-catalog
 
 pkginclude_HEADERS += \
 	src/systemd/sd-journal.h \
diff --git a/src/journal/catalog.c b/src/journal/catalog.c
index b2c684a..ebf0622 100644
--- a/src/journal/catalog.c
+++ b/src/journal/catalog.c
@@ -26,6 +26,7 @@
 #include <string.h>
 #include <sys/mman.h>
 #include <locale.h>
+#include <libgen.h>
 
 #include "util.h"
 #include "log.h"
@@ -39,7 +40,7 @@
 #include "mkdir.h"
 #include "catalog.h"
 
-static const char * const conf_file_dirs[] = {
+const char * const catalog_file_dirs[] = {
         "/usr/local/lib/systemd/catalog/",
         "/usr/lib/systemd/catalog/",
         NULL
@@ -62,7 +63,7 @@ typedef struct CatalogItem {
         le64_t offset;
 } CatalogItem;
 
-static unsigned catalog_hash_func(const void *p) {
+unsigned catalog_hash_func(const void *p) {
         const CatalogItem *i = p;
 
         assert_cc(sizeof(unsigned) == sizeof(uint8_t)*4);
@@ -86,7 +87,7 @@ static unsigned catalog_hash_func(const void *p) {
                 string_hash_func(i->language);
 }
 
-static int catalog_compare_func(const void *a, const void *b) {
+int catalog_compare_func(const void *a, const void *b) {
         const CatalogItem *i = a, *j = b;
         unsigned k;
 
@@ -138,7 +139,7 @@ static int finish_item(
         return 0;
 }
 
-static int import_file(Hashmap *h, struct strbuf *sb, const char *path) {
+int catalog_import_file(Hashmap *h, struct strbuf *sb, const char *path) {
         _cleanup_fclose_ FILE *f = NULL;
         _cleanup_free_ char *payload = NULL;
         unsigned n = 0;
@@ -271,34 +272,99 @@ static int import_file(Hashmap *h, struct strbuf *sb, const char *path) {
         return 0;
 }
 
-#define CATALOG_DATABASE CATALOG_PATH "/database"
+static long write_catalog(const char *database, Hashmap *h, struct strbuf *sb,
+                          CatalogItem *items, size_t n) {
+        CatalogHeader header;
+        _cleanup_fclose_ FILE *w = NULL;
+        int r;
+        char _cleanup_free_ *d, *p = NULL;
+        size_t k;
+
+        d = dirname_malloc(database);
+        if (!d)
+                return log_oom();
+
+        r = mkdir_p(d, 0775);
+        if (r < 0) {
+                log_error("Recursive mkdir %s: %s", d, strerror(-r));
+                return r;
+        }
+
+        r = fopen_temporary(database, &w, &p);
+        if (r < 0) {
+                log_error("Failed to open database for writing: %s: %s",
+                          database, strerror(-r));
+                return r;
+        }
+
+        zero(header);
+        memcpy(header.signature, CATALOG_SIGNATURE, sizeof(header.signature));
+        header.header_size = htole64(ALIGN_TO(sizeof(CatalogHeader), 8));
+        header.catalog_item_size = htole64(sizeof(CatalogItem));
+        header.n_items = htole64(hashmap_size(h));
 
-int catalog_update(void) {
+        r = -EIO;
+
+        k = fwrite(&header, 1, sizeof(header), w);
+        if (k != sizeof(header)) {
+                log_error("%s: failed to write header.", p);
+                goto error;
+        }
+
+        k = fwrite(items, 1, n * sizeof(CatalogItem), w);
+        if (k != n * sizeof(CatalogItem)) {
+                log_error("%s: failed to write database.", p);
+                goto error;
+        }
+
+        k = fwrite(sb->buf, 1, sb->len, w);
+        if (k != sb->len) {
+                log_error("%s: failed to write strings.", p);
+                goto error;
+        }
+
+        fflush(w);
+
+        if (ferror(w)) {
+                log_error("%s: failed to write database.", p);
+                goto error;
+        }
+
+        fchmod(fileno(w), 0644);
+
+        if (rename(p, database) < 0) {
+                log_error("rename (%s -> %s) failed: %m", p, database);
+                r = -errno;
+                goto error;
+        }
+
+        return ftell(w);
+
+error:
+        unlink(p);
+        return r;
+}
+
+int catalog_update(const char* database, const char* root, const char* const* dirs) {
         _cleanup_strv_free_ char **files = NULL;
-        _cleanup_fclose_ FILE *w = NULL;
-        _cleanup_free_ char *p = NULL;
         char **f;
         Hashmap *h;
         struct strbuf *sb = NULL;
         _cleanup_free_ CatalogItem *items = NULL;
         CatalogItem *i;
-        CatalogHeader header;
-        size_t k;
         Iterator j;
         unsigned n;
-        int r;
+        long r;
 
         h = hashmap_new(catalog_hash_func, catalog_compare_func);
-        if (!h)
-                return -ENOMEM;
-
         sb = strbuf_new();
-        if (!sb) {
+
+        if (!h || !sb) {
                 r = log_oom();
                 goto finish;
         }
 
-        r = conf_files_list_strv(&files, ".catalog", NULL, (const char **) conf_file_dirs);
+        r = conf_files_list_strv(&files, ".catalog", root, dirs);
         if (r < 0) {
                 log_error("Failed to get catalog files: %s", strerror(-r));
                 goto finish;
@@ -306,7 +372,7 @@ int catalog_update(void) {
 
         STRV_FOREACH(f, files) {
                 log_debug("reading file '%s'", *f);
-                import_file(h, sb, *f);
+                catalog_import_file(h, sb, *f);
         }
 
         if (hashmap_size(h) <= 0) {
@@ -335,79 +401,25 @@ int catalog_update(void) {
         assert(n == hashmap_size(h));
         qsort(items, n, sizeof(CatalogItem), catalog_compare_func);
 
-        r = mkdir_p(CATALOG_PATH, 0775);
-        if (r < 0) {
-                log_error("Recursive mkdir %s: %s", CATALOG_PATH, strerror(-r));
-                goto finish;
-        }
-
-        r = fopen_temporary(CATALOG_DATABASE, &w, &p);
-        if (r < 0) {
-                log_error("Failed to open database for writing: %s: %s",
-                          CATALOG_DATABASE, strerror(-r));
-                goto finish;
-        }
-
-        zero(header);
-        memcpy(header.signature, CATALOG_SIGNATURE, sizeof(header.signature));
-        header.header_size = htole64(ALIGN_TO(sizeof(CatalogHeader), 8));
-        header.catalog_item_size = htole64(sizeof(CatalogItem));
-        header.n_items = htole64(hashmap_size(h));
-
-        k = fwrite(&header, 1, sizeof(header), w);
-        if (k != sizeof(header)) {
-                log_error("%s: failed to write header.", p);
-                goto finish;
-        }
-
-        k = fwrite(items, 1, n * sizeof(CatalogItem), w);
-        if (k != n * sizeof(CatalogItem)) {
-                log_error("%s: failed to write database.", p);
-                goto finish;
-        }
-
-        k = fwrite(sb->buf, 1, sb->len, w);
-        if (k != sb->len) {
-                log_error("%s: failed to write strings.", p);
-                goto finish;
-        }
-
-        fflush(w);
-
-        if (ferror(w)) {
-                log_error("%s: failed to write database.", p);
-                goto finish;
-        }
-
-        fchmod(fileno(w), 0644);
-
-        if (rename(p, CATALOG_DATABASE) < 0) {
-                log_error("rename (%s -> %s) failed: %m", p, CATALOG_DATABASE);
-                r = -errno;
-                goto finish;
-        }
-
-        log_debug("%s: wrote %u items, with %zu bytes of strings, %ld total size.",
-                 CATALOG_DATABASE, n, sb->len, ftell(w));
-
-        free(p);
-        p = NULL;
+        r = write_catalog(database, h, sb, items, n);
+        if (r < 0)
+                log_error("Failed to write %s: %s", database, strerror(-r));
+        else
+                log_debug("%s: wrote %u items, with %zu bytes of strings, %ld total size.",
+                          database, n, sb->len, r);
 
         r = 0;
 
 finish:
-        hashmap_free_free(h);
-
+        if (h)
+                hashmap_free_free(h);
         if (sb)
                 strbuf_cleanup(sb);
 
-        if (p)
-                unlink(p);
-
-        return r;
+        return r < 0 ? r : 0;
 }
 
-static int open_mmap(int *_fd, struct stat *_st, void **_p) {
+static int open_mmap(const char *database, int *_fd, struct stat *_st, void **_p) {
         const CatalogHeader *h;
         int fd;
         void *p;
@@ -417,7 +429,7 @@ static int open_mmap(int *_fd, struct stat *_st, void **_p) {
         assert(_st);
         assert(_p);
 
-        fd = open(CATALOG_DATABASE, O_RDONLY|O_CLOEXEC);
+        fd = open(database, O_RDONLY|O_CLOEXEC);
         if (fd < 0)
                 return -errno;
 
@@ -495,7 +507,7 @@ static const char *find_id(void *p, sd_id128_t id) {
                 le64toh(f->offset);
 }
 
-int catalog_get(sd_id128_t id, char **_text) {
+int catalog_get(const char* database, sd_id128_t id, char **_text) {
         _cleanup_close_ int fd = -1;
         void *p = NULL;
         struct stat st;
@@ -505,7 +517,7 @@ int catalog_get(sd_id128_t id, char **_text) {
 
         assert(_text);
 
-        r = open_mmap(&fd, &st, &p);
+        r = open_mmap(database, &fd, &st, &p);
         if (r < 0)
                 return r;
 
@@ -571,7 +583,7 @@ static void dump_catalog_entry(FILE *f, sd_id128_t id, const char *s, bool oneli
 }
 
 
-int catalog_list(FILE *f, bool oneline) {
+int catalog_list(FILE *f, const char *database, bool oneline) {
         _cleanup_close_ int fd = -1;
         void *p = NULL;
         struct stat st;
@@ -582,7 +594,7 @@ int catalog_list(FILE *f, bool oneline) {
         sd_id128_t last_id;
         bool last_id_set = false;
 
-        r = open_mmap(&fd, &st, &p);
+        r = open_mmap(database, &fd, &st, &p);
         if (r < 0)
                 return r;
 
@@ -608,7 +620,7 @@ int catalog_list(FILE *f, bool oneline) {
         return 0;
 }
 
-int catalog_list_items(FILE *f, bool oneline, char **items) {
+int catalog_list_items(FILE *f, const char *database, bool oneline, char **items) {
         char **item;
         int r = 0;
 
@@ -626,7 +638,7 @@ int catalog_list_items(FILE *f, bool oneline, char **items) {
                         continue;
                 }
 
-                k = catalog_get(id, &msg);
+                k = catalog_get(database, id, &msg);
                 if (k < 0) {
                         log_full(k == -ENOENT ? LOG_NOTICE : LOG_ERR,
                                  "Failed to retrieve catalog entry for '%s': %s",
diff --git a/src/journal/catalog.h b/src/journal/catalog.h
index 8ea2c41..89f8f93 100644
--- a/src/journal/catalog.h
+++ b/src/journal/catalog.h
@@ -24,8 +24,14 @@
 #include <stdbool.h>
 
 #include "sd-id128.h"
-
-int catalog_update(void);
-int catalog_get(sd_id128_t id, char **data);
-int catalog_list(FILE *f, bool oneline);
-int catalog_list_items(FILE *f, bool oneline, char **items);
+#include "hashmap.h"
+#include "strbuf.h"
+
+int catalog_import_file(Hashmap *h, struct strbuf *sb, const char *path);
+unsigned catalog_hash_func(const void *p);
+int catalog_compare_func(const void *a, const void *b);
+int catalog_update(const char* database, const char* root, const char* const* dirs);
+int catalog_get(const char* database, sd_id128_t id, char **data);
+int catalog_list(FILE *f, const char* database, bool oneline);
+int catalog_list_items(FILE *f, const char* database, bool oneline, char **items);
+extern const char * const catalog_file_dirs[];
diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index 0a82a1c..03daafe 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -1020,20 +1020,26 @@ int main(int argc, char *argv[]) {
                 goto finish;
         }
 
-        if (arg_action == ACTION_LIST_CATALOG ||
-            arg_action == ACTION_DUMP_CATALOG)  {
-                bool oneline = arg_action == ACTION_LIST_CATALOG;
-                if (optind < argc)
-                        r = catalog_list_items(stdout, oneline, argv + optind);
-                else
-                        r = catalog_list(stdout, oneline);
-                if (r < 0)
-                        log_error("Failed to list catalog: %s", strerror(-r));
-                goto finish;
-        }
+        if (arg_action == ACTION_UPDATE_CATALOG ||
+            arg_action == ACTION_LIST_CATALOG ||
+            arg_action == ACTION_DUMP_CATALOG) {
+
+                if (arg_action == ACTION_UPDATE_CATALOG) {
+                        r = catalog_update(CATALOG_DATABASE, NULL, catalog_file_dirs);
+                        if (r < 0)
+                                log_error("Failed to list catalog: %s", strerror(-r));
+                } else {
+                        bool oneline = arg_action == ACTION_LIST_CATALOG;
+
+                        if (optind < argc)
+                                r = catalog_list_items(stdout, CATALOG_DATABASE,
+                                                       oneline, argv + optind);
+                        else
+                                r = catalog_list(stdout, CATALOG_DATABASE, oneline);
+                        if (r < 0)
+                                log_error("Failed to list catalog: %s", strerror(-r));
+                }
 
-        if (arg_action == ACTION_UPDATE_CATALOG)  {
-                r = catalog_update();
                 goto finish;
         }
 
diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c
index 82cacf3..bb99671 100644
--- a/src/journal/sd-journal.c
+++ b/src/journal/sd-journal.c
@@ -2439,7 +2439,7 @@ _public_ int sd_journal_get_catalog(sd_journal *j, char **ret) {
         if (r < 0)
                 return r;
 
-        r = catalog_get(id, &text);
+        r = catalog_get(CATALOG_DATABASE, id, &text);
         if (r < 0)
                 return r;
 
@@ -2455,7 +2455,7 @@ _public_ int sd_journal_get_catalog_for_message_id(sd_id128_t id, char **ret) {
         if (!ret)
                 return -EINVAL;
 
-        return catalog_get(id, ret);
+        return catalog_get(CATALOG_DATABASE, id, ret);
 }
 
 _public_ int sd_journal_set_data_threshold(sd_journal *j, size_t sz) {
diff --git a/src/journal/test-catalog.c b/src/journal/test-catalog.c
index c75b146..21149f1 100644
--- a/src/journal/test-catalog.c
+++ b/src/journal/test-catalog.c
@@ -4,6 +4,7 @@
   This file is part of systemd.
 
   Copyright 2012 Lennart Poettering
+  Copyright 2013 Zbigniew Jędrzejewski-Szmek
 
   systemd is free software; you can redistribute it and/or modify it
   under the terms of the GNU Lesser General Public License as published by
@@ -20,31 +21,115 @@
 ***/
 
 #include <locale.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <errno.h>
 
 #include "util.h"
 #include "log.h"
-#include "catalog.h"
+#include "macro.h"
 #include "sd-messages.h"
+#include "catalog.h"
 
-int main(int argc, char *argv[]) {
+static void test_import(Hashmap *h, struct strbuf *sb,
+                        const char* contents, ssize_t size, int code) {
+        int r;
+        char name[] = "/tmp/test-catalog.XXXXXX";
+        int _cleanup_close_ fd = mkstemp(name);
+        assert(fd >= 0);
+        assert_se(write(fd, contents, size) == size);
+
+        r = catalog_import_file(h, sb, name);
+        assert(r == code);
+
+        unlink(name);
+}
+
+static void test_catalog_importing(void) {
+        Hashmap *h;
+        struct strbuf *sb;
+
+        assert_se(h = hashmap_new(catalog_hash_func, catalog_compare_func));
+        assert_se(sb = strbuf_new());
+
+#define BUF "xxx"
+        test_import(h, sb, BUF, sizeof(BUF), -EINVAL);
+#undef BUF
+        assert(hashmap_isempty(h));
+        log_debug("----------------------------------------");
+
+#define BUF \
+"-- 0027229ca0644181a76c4e92458afaff dededededededededededededededede\n" \
+"Subject: message\n" \
+"\n" \
+"payload\n"
+        test_import(h, sb, BUF, sizeof(BUF), -EINVAL);
+#undef BUF
+
+        log_debug("----------------------------------------");
+
+#define BUF \
+"-- 0027229ca0644181a76c4e92458afaff dededededededededededededededed\n" \
+"Subject: message\n" \
+"\n" \
+"payload\n"
+        test_import(h, sb, BUF, sizeof(BUF), 0);
+#undef BUF
+
+        assert(hashmap_size(h) == 1);
 
+        log_debug("----------------------------------------");
+
+        hashmap_free_free(h);
+        strbuf_cleanup(sb);
+}
+
+
+static const char* database = NULL;
+
+static void test_catalog_update(void) {
+        int r;
+        char _cleanup_free_ *path = NULL;
+
+        static char name[] = "/tmp/test-catalog.XXXXXX";
+        r = mkstemp(name);
+        assert(r >= 0);
+
+        database = name;
+
+        /* Test what happens if there are no files. */
+        r = catalog_update(database, NULL, NULL);
+        assert(r >= 0);
+
+        /* Note: this might actually not find anything, if systemd was
+         * not installed before. That should be fine too. */
+        r = catalog_update(database, NULL, catalog_file_dirs);
+        assert(r >= 0);
+}
+
+int main(int argc, char *argv[]) {
         _cleanup_free_ char *text = NULL;
+        int r;
 
         setlocale(LC_ALL, "de_DE.UTF-8");
 
         log_set_max_level(LOG_DEBUG);
 
-        assert_se(catalog_update() >= 0);
+        test_catalog_importing();
 
-        assert_se(catalog_list(stdout, true) >= 0);
+        test_catalog_update();
 
-        assert_se(catalog_list(stdout, false) >= 0);
+        r = catalog_list(stdout, database, true);
+        assert_se(r >= 0);
 
-        assert_se(catalog_get(SD_MESSAGE_COREDUMP, &text) >= 0);
+        r = catalog_list(stdout, database, false);
+        assert_se(r >= 0);
 
+        assert_se(catalog_get(database, SD_MESSAGE_COREDUMP, &text) >= 0);
         printf(">>>%s<<<\n", text);
 
-        fflush(stdout);
+        if (database)
+                unlink(database);
 
         return 0;
 }
diff --git a/src/shared/conf-files.c b/src/shared/conf-files.c
index 296e605..6d99739 100644
--- a/src/shared/conf-files.c
+++ b/src/shared/conf-files.c
@@ -134,7 +134,7 @@ static int conf_files_list_strv_internal(char ***strv, const char *suffix, const
         return 0;
 }
 
-int conf_files_list_strv(char ***strv, const char *suffix, const char *root, const char **dirs) {
+int conf_files_list_strv(char ***strv, const char *suffix, const char *root, const char* const* dirs) {
         _cleanup_strv_free_ char **copy = NULL;
 
         assert(strv);
diff --git a/src/shared/conf-files.h b/src/shared/conf-files.h
index 28588e6..3bd3d2f 100644
--- a/src/shared/conf-files.h
+++ b/src/shared/conf-files.h
@@ -26,7 +26,7 @@
 #include "macro.h"
 
 int conf_files_list(char ***strv, const char *suffix, const char *root, const char *dir, ...);
-int conf_files_list_strv(char ***strv, const char *suffix, const char *root, const char **dirs);
+int conf_files_list_strv(char ***strv, const char *suffix, const char *root, const char* const* dirs);
 int conf_files_list_nulstr(char ***strv, const char *suffix, const char *root, const char *dirs);
 
 #endif
diff --git a/src/shared/util.c b/src/shared/util.c
index 8fa01fa..2241b79 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -59,6 +59,7 @@
 #include <limits.h>
 #include <langinfo.h>
 #include <locale.h>
+#include <libgen.h>
 
 #include "macro.h"
 #include "util.h"
@@ -2356,6 +2357,24 @@ int dir_is_empty(const char *path) {
         }
 }
 
+char* dirname_malloc(const char *path) {
+        char *d, *dir, *dir2;
+
+        d = strdup(path);
+        if (!d)
+                return NULL;
+        dir = dirname(d);
+        assert(dir);
+
+        if (dir != d) {
+                dir2 = strdup(dir);
+                free(d);
+                return dir2;
+        }
+
+        return dir;
+}
+
 unsigned long long random_ull(void) {
         _cleanup_close_ int fd;
         uint64_t ull;
diff --git a/src/shared/util.h b/src/shared/util.h
index 52c3323..aefde5f 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -329,6 +329,7 @@ ssize_t loop_write(int fd, const void *buf, size_t nbytes, bool do_poll);
 bool is_device_path(const char *path);
 
 int dir_is_empty(const char *path);
+char* dirname_malloc(const char *path);
 
 void rename_process(const char name[8]);
 
diff --git a/src/udev/udevadm-hwdb.c b/src/udev/udevadm-hwdb.c
index ca1bf16..a2df291 100644
--- a/src/udev/udevadm-hwdb.c
+++ b/src/udev/udevadm-hwdb.c
@@ -544,7 +544,7 @@ static int adm_hwdb(struct udev *udev, int argc, char *argv[]) {
                 }
                 trie->nodes_count++;
 
-                err = conf_files_list_strv(&files, ".hwdb", root, (const char **)conf_file_dirs);
+                err = conf_files_list_strv(&files, ".hwdb", root, conf_file_dirs);
                 if (err < 0) {
                         log_error("failed to enumerate hwdb files: %s\n", strerror(-err));
                         rc = EXIT_FAILURE;

commit 18cd5fe99f70a55a2d6f2303d6ee0624942695b1
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Wed Mar 27 23:16:32 2013 -0400

    catalog: make sure strings are terminated
    
    Coverity complains: systemd-199/src/journal/catalog.c:126:
    buffer_size_warning: Calling strncpy with a maximum size argument of
    32 bytes on destination array "i->language" of size 32 bytes might
    leave the destination string unterminated.
    
    ...and unfortunately it was right. The string was defined as a
    fixed-size string in some parts of the code, and used a
    null-terminated string in others (e.g. in log statements). There's no
    point in conserving one byte, so just define the max language tag
    length to 31 bytes, and use null terminated strings everywhere.
    
    Also, wrap some lines, zero-fill less bytes, use '\0' instead of just
    0 to be more explicit that this is one byte.

diff --git a/src/journal/catalog.c b/src/journal/catalog.c
index 7ae7b3e..b2c684a 100644
--- a/src/journal/catalog.c
+++ b/src/journal/catalog.c
@@ -34,6 +34,7 @@
 #include "hashmap.h"
 #include "strv.h"
 #include "strbuf.h"
+#include "strxcpyx.h"
 #include "conf-files.h"
 #include "mkdir.h"
 #include "catalog.h"
@@ -96,7 +97,7 @@ static int catalog_compare_func(const void *a, const void *b) {
                         return 1;
         }
 
-        return strncmp(i->language, j->language, sizeof(i->language));
+        return strcmp(i->language, j->language);
 }
 
 static int finish_item(
@@ -123,12 +124,13 @@ static int finish_item(
                 return log_oom();
 
         i->id = id;
-        strncpy(i->language, language, sizeof(i->language));
+        strscpy(i->language, sizeof(i->language), language);
         i->offset = htole64((uint64_t) offset);
 
         r = hashmap_put(h, i, i);
         if (r == EEXIST) {
-                log_warning("Duplicate entry for " SD_ID128_FORMAT_STR ".%s, ignoring.", SD_ID128_FORMAT_VAL(id), language ? language : "C");
+                log_warning("Duplicate entry for " SD_ID128_FORMAT_STR ".%s, ignoring.",
+                            SD_ID128_FORMAT_VAL(id), language ? language : "C");
                 free(i);
                 return 0;
         }
@@ -185,15 +187,15 @@ static int import_file(Hashmap *h, struct strbuf *sb, const char *path) {
                     line[0] == '-' &&
                     line[1] == '-' &&
                     line[2] == ' ' &&
-                    (line[2+1+32] == ' ' || line[2+1+32] == 0)) {
+                    (line[2+1+32] == ' ' || line[2+1+32] == '\0')) {
 
                         bool with_language;
                         sd_id128_t jd;
 
                         /* New entry */
 
-                        with_language = line[2+1+32] != 0;
-                        line[2+1+32] = 0;
+                        with_language = line[2+1+32] != '\0';
+                        line[2+1+32] = '\0';
 
                         if (sd_id128_from_string(line + 2 + 1, &jd) >= 0) {
 
@@ -211,21 +213,21 @@ static int import_file(Hashmap *h, struct strbuf *sb, const char *path) {
                                                 log_error("[%s:%u] Language too short.", path, n);
                                                 return -EINVAL;
                                         }
-                                        if (c > sizeof(language)) {
+                                        if (c > sizeof(language) - 1) {
                                                 log_error("[%s:%u] language too long.", path, n);
                                                 return -EINVAL;
                                         }
 
-                                        strncpy(language, t, sizeof(language));
+                                        strscpy(language, sizeof(language), t);
                                 } else
-                                        zero(language);
+                                        language[0] = '\0';
 
                                 got_id = true;
                                 empty_line = false;
                                 id = jd;
 
                                 if (payload)
-                                        payload[0] = 0;
+                                        payload[0] = '\0';
 
                                 continue;
                         }
@@ -324,7 +326,9 @@ int catalog_update(void) {
 
         n = 0;
         HASHMAP_FOREACH(i, h, j) {
-                log_debug("Found " SD_ID128_FORMAT_STR ", language %s", SD_ID128_FORMAT_VAL(i->id), isempty(i->language) ? "C" : i->language);
+                log_debug("Found " SD_ID128_FORMAT_STR ", language %s",
+                          SD_ID128_FORMAT_VAL(i->id),
+                          isempty(i->language) ? "C" : i->language);
                 items[n++] = *i;
         }
 

commit f45928521249bbaf5dbea84933ae2fcaf5354080
Author: Václav Pavlín <vpavlin at redhat.com>
Date:   Wed Mar 27 15:16:36 2013 +0100

    udev: check return value of uname.

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index 2ad7388..42cf994 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -819,7 +819,11 @@ static void static_dev_create_from_modules(struct udev *udev)
         char buf[4096];
         FILE *f;
 
-        uname(&kernel);
+        if (uname(&kernel) < 0) {
+                log_error("uname failed: %m");
+                return;
+        }
+
         strscpyl(modules, sizeof(modules), ROOTPREFIX "/lib/modules/", kernel.release, "/modules.devname", NULL);
         f = fopen(modules, "re");
         if (f == NULL)

commit c309a7137b06516eafc055eaab31df964599a8b3
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Mar 28 23:41:07 2013 -0400

    bootchart: properly terminate string
    
    systemd-199/src/bootchart/store.c:289: buffer_size_warning: Calling
    strncpy with a maximum size argument of 256 bytes on destination array
    "ps->name" of size 256 bytes might leave the destination string
    unterminated.
    
    ...and indeed, the string was used as NULL-terminated later on.
    
    pid_cmdline_strncpy is renamed to pid_cmdline_strscpy to commemorate
    the fact that it *does* properly terminate the string.

diff --git a/src/bootchart/store.c b/src/bootchart/store.c
index 0253ebb..343365e 100644
--- a/src/bootchart/store.c
+++ b/src/bootchart/store.c
@@ -34,6 +34,7 @@
 #include <time.h>
 
 #include "util.h"
+#include "strxcpyx.h"
 #include "store.h"
 #include "bootchart.h"
 
@@ -89,7 +90,7 @@ static char *bufgetline(char *buf) {
         return c;
 }
 
-static int pid_cmdline_strncpy(char *buffer, int pid, size_t buf_len) {
+static int pid_cmdline_strscpy(char *buffer, size_t buf_len, int pid) {
         char filename[PATH_MAX];
         int _cleanup_close_ fd=-1;
         ssize_t n;
@@ -286,11 +287,11 @@ schedstat_next:
                         if (!sscanf(buf, "%s %*s %*s", key))
                                 continue;
 
-                        strncpy(ps->name, key, 256);
+                        strscpy(ps->name, sizeof(ps->name), key);
 
                         /* cmdline */
                         if (arg_show_cmdline)
-                                pid_cmdline_strncpy(ps->name, pid, 256);
+                                pid_cmdline_strscpy(ps->name, sizeof(ps->name), pid);
 
                         /* discard line 2 */
                         m = bufgetline(buf);
@@ -449,11 +450,11 @@ catch_rename:
                         if (!sscanf(buf, "%s %*s %*s", key))
                                 continue;
 
-                        strncpy(ps->name, key, 256);
+                        strscpy(ps->name, sizeof(ps->name), key);
 
                         /* cmdline */
                         if (arg_show_cmdline)
-                                pid_cmdline_strncpy(ps->name, pid, 256);
+                                pid_cmdline_strscpy(ps->name, sizeof(ps->name), pid);
                 }
         }
 }

commit 268888765352e4dcf07e40917fef6ab41b7deba1
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Mar 28 23:19:19 2013 -0400

    utmp-wtmp: don't try to read past end of string
    
    systemd-199/src/shared/utmp-wtmp.c:228: buffer_size_warning: Calling
    strncpy with a maximum size argument of 32 bytes on destination array
    "store.ut_line" of size 32 bytes might leave the destination string
    unterminated.
    
    The destination string is unterminated on purpose, but we must
    remember that.

diff --git a/src/shared/utmp-wtmp.c b/src/shared/utmp-wtmp.c
index 046fb58..8717dba 100644
--- a/src/shared/utmp-wtmp.c
+++ b/src/shared/utmp-wtmp.c
@@ -403,10 +403,12 @@ int utmp_wall(const char *message, bool (*match_tty)(const char *tty)) {
                 if (u->ut_type != USER_PROCESS || u->ut_user[0] == 0)
                         continue;
 
+                /* this access is fine, because strlen("/dev/") << 32 (UT_LINESIZE) */
                 if (path_startswith(u->ut_line, "/dev/"))
                         path = u->ut_line;
                 else {
-                        if (asprintf(&buf, "/dev/%s", u->ut_line) < 0) {
+                        if (asprintf(&buf, "/dev/%.*s",
+                                     sizeof(u->ut_line), u->ut_line) < 0) {
                                 r = -ENOMEM;
                                 goto finish;
                         }



More information about the systemd-commits mailing list