From 844ec79b3c2f246114ea316ebe1f36044bdb688e Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 28 Mar 2013 20:17:24 -0400 Subject: [PATCH 1/1] 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. --- Makefile.am | 8 +- src/journal/catalog.c | 188 ++++++++++++++++++++----------------- src/journal/catalog.h | 16 +++- src/journal/journalctl.c | 32 ++++--- 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/util.c | 19 ++++ src/shared/util.h | 1 + src/udev/udevadm-hwdb.c | 2 +- 11 files changed, 251 insertions(+), 122 deletions(-) diff --git a/Makefile.am b/Makefile.am index 923f81bb2..2eae8773f 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 b2c684ac2..ebf062260 100644 --- a/src/journal/catalog.c +++ b/src/journal/catalog.c @@ -26,6 +26,7 @@ #include #include #include +#include #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 8ea2c41c2..89f8f93d0 100644 --- a/src/journal/catalog.h +++ b/src/journal/catalog.h @@ -24,8 +24,14 @@ #include #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 0a82a1cf1..03daafe9d 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 82cacf367..bb9967193 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 c75b1464f..21149f18a 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 +#include +#include +#include #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 296e60576..6d9973935 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 28588e6f0..3bd3d2f3d 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 8fa01fa8a..2241b7985 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -59,6 +59,7 @@ #include #include #include +#include #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 52c33238b..aefde5f85 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 ca1bf165a..a2df291a4 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; -- 2.30.2