[systemd-commits] 4 commits - man/journalctl.xml src/core src/journal src/network

Zbigniew Jędrzejewski-Szmek zbyszek at kemper.freedesktop.org
Sat Jan 11 07:09:57 PST 2014


 man/journalctl.xml            |   28 ++++--
 src/core/manager.c            |    4 
 src/journal/journal-file.c    |    5 -
 src/journal/journal-file.h    |   24 +++++
 src/journal/journal-verify.c  |    4 
 src/journal/journalctl.c      |  175 ++++++++++++++++++++++++++++++++++++++----
 src/journal/mmap-cache.c      |   57 +++++++++++--
 src/journal/mmap-cache.h      |   18 ++++
 src/journal/sd-journal.c      |   18 +++-
 src/network/networkd-bridge.c |    4 
 src/network/networkd-link.c   |   44 +++++-----
 src/network/networkd.h        |   38 ++-------
 12 files changed, 321 insertions(+), 98 deletions(-)

New commits:
commit 5bf348d77ec35260e2c021ad11d08dca7b92498d
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sat Jan 11 09:52:48 2014 -0500

    core: fix unused variable

diff --git a/src/core/manager.c b/src/core/manager.c
index 1eda2b1..a80d6a1 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -530,11 +530,13 @@ static int manager_setup_notify(Manager *m) {
 }
 
 static int manager_setup_kdbus(Manager *m) {
+#ifdef ENABLE_KDBUS
         _cleanup_free_ char *p = NULL;
+#endif
 
+#ifdef ENABLE_KDBUS
         assert(m);
 
-#ifdef ENABLE_KDBUS
         if (m->kdbus_fd >= 0)
                 return 0;
 

commit 39032b87779323a244dd89f4832949d462b2ac68
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sat Jan 11 09:42:55 2014 -0500

    network: use GNU-ism to simplify macros
    
    Thanks David!

diff --git a/src/network/networkd-bridge.c b/src/network/networkd-bridge.c
index d644bd5..fa87d3e 100644
--- a/src/network/networkd-bridge.c
+++ b/src/network/networkd-bridge.c
@@ -116,7 +116,7 @@ static int bridge_enter_ready(Bridge *bridge) {
 
         bridge->state = BRIDGE_STATE_READY;
 
-        log_bridge_info(bridge, "bridge ready");
+        log_info_bridge(bridge, "bridge ready");
 
         LIST_FOREACH(callbacks, callback, bridge->callbacks) {
                 /* join the links that were attempted to be joined befor the
@@ -206,7 +206,7 @@ static int bridge_create(Bridge *bridge) {
                 return r;
         }
 
-        log_bridge_debug(bridge, "creating bridge");
+        log_debug_bridge(bridge, "creating bridge");
 
         bridge->state = BRIDGE_STATE_CREATING;
 
diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index 3c683a0..52b023c 100644
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -134,7 +134,7 @@ static int link_enter_configured(Link *link) {
         assert(link);
         assert(link->state == LINK_STATE_SETTING_ROUTES);
 
-        log_link_info(link, "link configured");
+        log_info_link(link, "link configured");
 
         link->state = LINK_STATE_CONFIGURED;
 
@@ -144,7 +144,7 @@ static int link_enter_configured(Link *link) {
 static void link_enter_failed(Link *link) {
         assert(link);
 
-        log_link_warning(link, "failed");
+        log_warning_link(link, "failed");
 
         link->state = LINK_STATE_FAILED;
 }
@@ -170,7 +170,7 @@ static int route_handler(sd_rtnl *rtnl, sd_rtnl_message *m, void *userdata) {
         /* we might have received an old reply after moving back to SETTING_ADDRESSES,
          * ignore it */
         if (link->route_messages == 0 && link->state == LINK_STATE_SETTING_ROUTES) {
-                log_link_debug(link, "routes set");
+                log_debug_link(link, "routes set");
                 link_enter_configured(link);
         }
 
@@ -190,7 +190,7 @@ static int link_enter_set_routes(Link *link) {
         if (!link->network->static_routes && !link->dhcp_route)
                 return link_enter_configured(link);
 
-        log_link_debug(link, "setting routes");
+        log_debug_link(link, "setting routes");
 
         LIST_FOREACH(static_routes, route, link->network->static_routes) {
                 r = route_configure(route, link, &route_handler);
@@ -243,7 +243,7 @@ static int address_handler(sd_rtnl *rtnl, sd_rtnl_message *m, void *userdata) {
                                 NULL);
 
         if (link->addr_messages == 0) {
-                log_link_debug(link, "addresses set");
+                log_debug_link(link, "addresses set");
                 link_enter_set_routes(link);
         }
 
@@ -263,7 +263,7 @@ static int link_enter_set_addresses(Link *link) {
         if (!link->network->static_addresses && !link->dhcp_address)
                 return link_enter_set_routes(link);
 
-        log_link_debug(link, "setting addresses");
+        log_debug_link(link, "setting addresses");
 
         LIST_FOREACH(static_addresses, address, link->network->static_addresses) {
                 r = address_configure(address, link, &address_handler);
@@ -330,7 +330,7 @@ static void dhcp_handler(sd_dhcp_client *client, int event, void *userdata) {
         }
 
         if (event == DHCP_EVENT_NO_LEASE)
-                log_link_debug(link, "IP address in use.");
+                log_debug_link(link, "IP address in use.");
 
         if (event == DHCP_EVENT_IP_CHANGE || event == DHCP_EVENT_EXPIRED ||
             event == DHCP_EVENT_STOP) {
@@ -349,28 +349,28 @@ static void dhcp_handler(sd_dhcp_client *client, int event, void *userdata) {
 
         r = sd_dhcp_client_get_address(client, &address);
         if (r < 0) {
-                log_link_warning(link, "DHCP error: no address");
+                log_warning_link(link, "DHCP error: no address");
                 link_enter_failed(link);
                 return;
         }
 
         r = sd_dhcp_client_get_netmask(client, &netmask);
         if (r < 0) {
-                log_link_warning(link, "DHCP error: no netmask");
+                log_warning_link(link, "DHCP error: no netmask");
                 link_enter_failed(link);
                 return;
         }
 
         prefixlen = sd_dhcp_client_prefixlen(&netmask);
         if (prefixlen < 0) {
-                log_link_warning(link, "DHCP error: no prefixlen");
+                log_warning_link(link, "DHCP error: no prefixlen");
                 link_enter_failed(link);
                 return;
         }
 
         r = sd_dhcp_client_get_router(client, &gateway);
         if (r < 0) {
-                log_link_warning(link, "DHCP error: no router");
+                log_warning_link(link, "DHCP error: no router");
                 link_enter_failed(link);
                 return;
         }
@@ -395,7 +395,7 @@ static void dhcp_handler(sd_dhcp_client *client, int event, void *userdata) {
 
                 r = address_new_dynamic(&addr);
                 if (r < 0) {
-                        log_link_error(link, "Could not allocate address");
+                        log_error_link(link, "Could not allocate address");
                         link_enter_failed(link);
                         return;
                 }
@@ -407,7 +407,7 @@ static void dhcp_handler(sd_dhcp_client *client, int event, void *userdata) {
 
                 r = route_new_dynamic(&rt);
                 if (r < 0) {
-                        log_link_error(link, "Could not allocate route");
+                        log_error_link(link, "Could not allocate route");
                         link_enter_failed(link);
                         return;
                 }
@@ -480,7 +480,7 @@ static int link_update_flags(Link *link, unsigned flags) {
 
         if ((link->flags & IFF_LOWER_UP) != (flags & IFF_LOWER_UP)) {
                 if (flags & IFF_LOWER_UP) {
-                        log_link_info(link, "carrier on");
+                        log_info_link(link, "carrier on");
 
                         if (link->network->dhcp) {
                                 r = link_acquire_conf(link);
@@ -490,7 +490,7 @@ static int link_update_flags(Link *link, unsigned flags) {
                                 }
                         }
                 } else {
-                        log_link_info(link, "carrier off");
+                        log_info_link(link, "carrier off");
 
                         if (link->network->dhcp) {
                                 r = sd_dhcp_client_stop(link->dhcp);
@@ -539,11 +539,11 @@ static int link_up(Link *link) {
         assert(link->manager);
         assert(link->manager->rtnl);
 
-        log_link_debug(link, "bringing link up");
+        log_debug_link(link, "bringing link up");
 
         r = sd_rtnl_message_link_new(RTM_SETLINK, link->ifindex, &req);
         if (r < 0) {
-                log_link_error(link, "Could not allocate RTM_SETLINK message");
+                log_error_link(link, "Could not allocate RTM_SETLINK message");
                 return r;
         }
 
@@ -631,7 +631,7 @@ static int link_enter_join_bridge(Link *link) {
                         link->network->bridge->name,
                         BRIDGE(link->network->bridge),
                         NULL);
-        log_link_debug(link, "joining bridge");
+        log_debug_link(link, "joining bridge");
 
         r = bridge_join(link->network->bridge, link, &bridge_handler);
         if (r < 0) {
@@ -662,7 +662,7 @@ static int link_get_handler(sd_rtnl *rtnl, sd_rtnl_message *m, void *userdata) {
                 link_enter_failed(link);
         }
 
-        log_link_debug(link, "got link state");
+        log_debug_link(link, "got link state");
 
         link_update(link, m);
 
@@ -677,11 +677,11 @@ static int link_get(Link *link) {
         assert(link->manager);
         assert(link->manager->rtnl);
 
-        log_link_debug(link, "requesting link status");
+        log_debug_link(link, "requesting link status");
 
         r = sd_rtnl_message_link_new(RTM_GETLINK, link->ifindex, &req);
         if (r < 0) {
-                log_link_error(link, "Could not allocate RTM_GETLINK message");
+                log_error_link(link, "Could not allocate RTM_GETLINK message");
                 return r;
         }
 
@@ -723,7 +723,7 @@ int link_update(Link *link, sd_rtnl_message *m) {
 
         r = sd_rtnl_message_link_get_flags(m, &flags);
         if (r < 0) {
-                log_link_warning(link, "could not get link flags");
+                log_warning_link(link, "could not get link flags");
                 return r;
         }
 
diff --git a/src/network/networkd.h b/src/network/networkd.h
index 2da852f..4f44f78 100644
--- a/src/network/networkd.h
+++ b/src/network/networkd.h
@@ -281,37 +281,23 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Link*, link_free);
 
 /* Macros which append INTERFACE= to the message */
 
-#define log_full_link(level, link, fmt, ...) log_meta_object(level, __FILE__, __LINE__, __func__, "INTERFACE=", link->ifname, "%s: " fmt, link->ifname, __VA_ARGS__)
-#define log_debug_link(link, ...)       log_full_link(LOG_DEBUG, link, __VA_ARGS__)
-#define log_info_link(link, ...)        log_full_link(LOG_INFO, link, __VA_ARGS__)
-#define log_notice_link(link, ...)      log_full_link(LOG_NOTICE, link, __VA_ARGS__)
-#define log_warning_link(link, ...)     log_full_link(LOG_WARNING, link, __VA_ARGS__)
-#define log_error_link(link, ...)       log_full_link(LOG_ERR, link, __VA_ARGS__)
-
-#define log_link_full(level, link, message) log_meta_object(level, __FILE__, __LINE__, __func__, "INTERFACE=", link->ifname, "%s: " message, link->ifname)
-#define log_link_debug(link, ...)       log_link_full(LOG_DEBUG, link, __VA_ARGS__)
-#define log_link_info(link, ...)        log_link_full(LOG_INFO, link, __VA_ARGS__)
-#define log_link_notice(link, ...)      log_link_full(LOG_NOTICE, link, __VA_ARGS__)
-#define log_link_warning(link, ...)     log_link_full(LOG_WARNING, link, __VA_ARGS__)
-#define log_link_error(link, ...)       log_link_full(LOG_ERR, link, __VA_ARGS__)
+#define log_full_link(level, link, fmt, ...) log_meta_object(level, __FILE__, __LINE__, __func__, "INTERFACE=", link->ifname, "%s: " fmt, link->ifname, ##__VA_ARGS__)
+#define log_debug_link(link, ...)       log_full_link(LOG_DEBUG, link, ##__VA_ARGS__)
+#define log_info_link(link, ...)        log_full_link(LOG_INFO, link, ##__VA_ARGS__)
+#define log_notice_link(link, ...)      log_full_link(LOG_NOTICE, link, ##__VA_ARGS__)
+#define log_warning_link(link, ...)     log_full_link(LOG_WARNING, link, ##__VA_ARGS__)
+#define log_error_link(link, ...)       log_full_link(LOG_ERR, link, ##__VA_ARGS__)
 
 #define log_struct_link(level, link, ...) log_struct(level, "INTERFACE=%s", link->ifname, __VA_ARGS__)
 
 /* More macros which append INTERFACE= to the message */
 
-#define log_full_bridge(level, bridge, fmt, ...) log_meta_object(level, __FILE__, __LINE__, __func__, "INTERFACE=", bridge->name, "%s: " fmt, bridge->name, __VA_ARGS__)
-#define log_debug_bridge(bridge, ...)       log_full_bridge(LOG_DEBUG, bridge, __VA_ARGS__)
-#define log_info_bridge(bridge, ...)        log_full_bridge(LOG_INFO, bridge, __VA_ARGS__)
-#define log_notice_bridge(bridge, ...)      log_full_bridge(LOG_NOTICE, bridge, __VA_ARGS__)
-#define log_warning_bridge(bridge, ...)     log_full_bridge(LOG_WARNING, bridge, __VA_ARGS__)
-#define log_error_bridge(bridge, ...)       log_full_bridge(LOG_ERR, bridge, __VA_ARGS__)
-
-#define log_bridge_full(level, bridge, message) log_meta_object(level, __FILE__, __LINE__, __func__, "INTERFACE=", bridge->name, "%s: " message, bridge->name)
-#define log_bridge_debug(bridge, ...)       log_bridge_full(LOG_DEBUG, bridge, __VA_ARGS__)
-#define log_bridge_info(bridge, ...)        log_bridge_full(LOG_INFO, bridge, __VA_ARGS__)
-#define log_bridge_notice(bridge, ...)      log_bridge_full(LOG_NOTICE, bridge, __VA_ARGS__)
-#define log_bridge_warning(bridge, ...)     log_bridge_full(LOG_WARNING, bridge, __VA_ARGS__)
-#define log_bridge_error(bridge, ...)       log_bridge_full(LOG_ERR, bridge, __VA_ARGS__)
+#define log_full_bridge(level, bridge, fmt, ...) log_meta_object(level, __FILE__, __LINE__, __func__, "INTERFACE=", bridge->name, "%s: " fmt, bridge->name, ##__VA_ARGS__)
+#define log_debug_bridge(bridge, ...)       log_full_bridge(LOG_DEBUG, bridge, ##__VA_ARGS__)
+#define log_info_bridge(bridge, ...)        log_full_bridge(LOG_INFO, bridge, ##__VA_ARGS__)
+#define log_notice_bridge(bridge, ...)      log_full_bridge(LOG_NOTICE, bridge, ##__VA_ARGS__)
+#define log_warning_bridge(bridge, ...)     log_full_bridge(LOG_WARNING, bridge,## __VA_ARGS__)
+#define log_error_bridge(bridge, ...)       log_full_bridge(LOG_ERR, bridge, ##__VA_ARGS__)
 
 #define log_struct_bridge(level, bridge, ...) log_struct(level, "INTERFACE=%s", bridge->name, __VA_ARGS__)
 

commit ea18a4b57e2bb94af7b3ecb7abdaec40e9f485f0
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sat Dec 28 19:47:36 2013 -0500

    journalctl: allow globbing in --unit and --user-unit
    
    This is a continuation of e3e0314b systemctl: allow globbing in commands
    which take multiple unit names.
    
    Multiple patterns can be specified, as separate arguments, or as one argument
    with patterns seperated by commas.
    
    If patterns are given, at least one unit must be matched (by any of the patterns).
    This is different behaviour than systemctl, but here it is necessary because
    otherwise anything would be matched, which is unlikely to be the intended
    behaviour.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=59336

diff --git a/man/journalctl.xml b/man/journalctl.xml
index 55474c5..3b05e80 100644
--- a/man/journalctl.xml
+++ b/man/journalctl.xml
@@ -509,17 +509,27 @@
 
                         <varlistentry>
                                 <term><option>-u</option></term>
-                                <term><option>--unit=</option></term>
+                                <term><option>--unit=<replaceable>UNIT</replaceable>|<replaceable>PATTERN</replaceable></option></term>
 
                                 <listitem><para>Show messages for the
-                                specified systemd unit. This will add
-                                a match for messages from the unit
-                                (<literal>_SYSTEMD_UNIT=</literal>)
-                                and additional matches for messages
-                                from systemd and messages about
-                                coredumps for the specified unit.</para>
-                                <para>This parameter can be specified multiple times.
-                                </para></listitem>
+                                specified systemd unit
+                                <replaceable>UNIT</replaceable>, or
+                                for any of the units matched by
+                                <replaceable>PATTERN</replaceable>.
+                                If a pattern is specified, a list of
+                                unit names found in the journal is
+                                compared with the specified pattern
+                                and all that match are used. For each
+                                unit name a match is added for
+                                messages from the unit
+                                (<literal>_SYSTEMD_UNIT=<replaceable>UNIT</replaceable></literal>)
+                                along with additional matches for
+                                messages from systemd and messages
+                                about coredumps for the specified
+                                unit.</para>
+
+                                <para>This parameter can be specified
+                                multiple times. </para></listitem>
                         </varlistentry>
 
                         <varlistentry>
diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index 42c16f6..2d99ade 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -21,6 +21,7 @@
 
 #include <locale.h>
 #include <fcntl.h>
+#include <fnmatch.h>
 #include <errno.h>
 #include <stddef.h>
 #include <string.h>
@@ -49,6 +50,7 @@
 #include "build.h"
 #include "pager.h"
 #include "strv.h"
+#include "set.h"
 #include "journal-internal.h"
 #include "journal-def.h"
 #include "journal-verify.h"
@@ -983,40 +985,177 @@ static int add_dmesg(sd_journal *j) {
         return 0;
 }
 
-static int add_units(sd_journal *j) {
-        _cleanup_free_ char *u = NULL;
+static int get_possible_units(sd_journal *j,
+                              const char *fields,
+                              char **patterns,
+                              Set **units) {
+        _cleanup_set_free_free_ Set *found;
+        const char *field;
         int r;
+
+        found = set_new(string_hash_func, string_compare_func);
+        if (!found)
+                return log_oom();
+
+        NULSTR_FOREACH(field, fields) {
+                const void *data;
+                size_t size;
+
+                r = sd_journal_query_unique(j, field);
+                if (r < 0)
+                        return r;
+
+                SD_JOURNAL_FOREACH_UNIQUE(j, data, size) {
+                        char **pattern, *eq;
+                        size_t prefix;
+                        _cleanup_free_ char *u = NULL;
+
+                        eq = memchr(data, '=', size);
+                        if (eq)
+                                prefix = eq - (char*) data + 1;
+                        else
+                                prefix = 0;
+
+                        u = strndup((char*) data + prefix, size - prefix);
+                        if (!u)
+                                return log_oom();
+
+                        STRV_FOREACH(pattern, patterns)
+                                if (fnmatch(*pattern, u, FNM_NOESCAPE) == 0) {
+                                        log_debug("Matched %s with pattern %s=%s", u, field, *pattern);
+
+                                        r = set_consume(found, u);
+                                        u = NULL;
+                                        if (r < 0 && r != -EEXIST)
+                                                return r;
+
+                                        break;
+                                }
+                }
+        }
+
+        *units = found;
+        found = NULL;
+        return 0;
+}
+
+/* This list is supposed to return the superset of unit names
+ * possibly matched by rules added with add_matches_for_unit... */
+#define SYSTEM_UNITS                 \
+        "_SYSTEMD_UNIT\0"            \
+        "COREDUMP_UNIT\0"            \
+        "UNIT\0"                     \
+        "OBJECT_SYSTEMD_UNIT\0"      \
+        "_SYSTEMD_SLICE\0"
+
+/* ... and add_matches_for_user_unit */
+#define USER_UNITS                   \
+        "_SYSTEMD_USER_UNIT\0"       \
+        "USER_UNIT\0"                \
+        "COREDUMP_USER_UNIT\0"       \
+        "OBJECT_SYSTEMD_USER_UNIT\0"
+
+static int add_units(sd_journal *j) {
+        _cleanup_strv_free_ char **patterns = NULL;
+        int r, count = 0;
         char **i;
 
         assert(j);
 
         STRV_FOREACH(i, arg_system_units) {
-                u = unit_name_mangle(*i, MANGLE_NOGLOB);
+                _cleanup_free_ char *u = NULL;
+
+                u = unit_name_mangle(*i, MANGLE_GLOB);
                 if (!u)
                         return log_oom();
-                r = add_matches_for_unit(j, u);
-                if (r < 0)
-                        return r;
-                r = sd_journal_add_disjunction(j);
+
+                if (string_is_glob(u)) {
+                        r = strv_push(&patterns, u);
+                        if (r < 0)
+                                return r;
+                        u = NULL;
+                } else {
+                        r = add_matches_for_unit(j, u);
+                        if (r < 0)
+                                return r;
+                        r = sd_journal_add_disjunction(j);
+                        if (r < 0)
+                                return r;
+                        count ++;
+                }
+        }
+
+        if (!strv_isempty(patterns)) {
+                _cleanup_set_free_free_ Set *units = NULL;
+                Iterator it;
+                char *u;
+
+                r = get_possible_units(j, SYSTEM_UNITS, patterns, &units);
                 if (r < 0)
                         return r;
+
+                SET_FOREACH(u, units, it) {
+                        r = add_matches_for_unit(j, u);
+                        if (r < 0)
+                                return r;
+                        r = sd_journal_add_disjunction(j);
+                        if (r < 0)
+                                return r;
+                        count ++;
+                }
         }
 
+        strv_free(patterns);
+        patterns = NULL;
+
         STRV_FOREACH(i, arg_user_units) {
-                u = unit_name_mangle(*i, MANGLE_NOGLOB);
+                _cleanup_free_ char *u = NULL;
+
+                u = unit_name_mangle(*i, MANGLE_GLOB);
                 if (!u)
                         return log_oom();
 
-                r = add_matches_for_user_unit(j, u, getuid());
-                if (r < 0)
-                        return r;
+                if (string_is_glob(u)) {
+                        r = strv_push(&patterns, u);
+                        if (r < 0)
+                                return r;
+                        u = NULL;
+                } else {
+                        r = add_matches_for_user_unit(j, u, getuid());
+                        if (r < 0)
+                                return r;
+                        r = sd_journal_add_disjunction(j);
+                        if (r < 0)
+                                return r;
+                        count ++;
+                }
+        }
+
+        if (!strv_isempty(patterns)) {
+                _cleanup_set_free_free_ Set *units = NULL;
+                Iterator it;
+                char *u;
 
-                r = sd_journal_add_disjunction(j);
+                r = get_possible_units(j, USER_UNITS, patterns, &units);
                 if (r < 0)
                         return r;
 
+                SET_FOREACH(u, units, it) {
+                        r = add_matches_for_user_unit(j, u, getuid());
+                        if (r < 0)
+                                return r;
+                        r = sd_journal_add_disjunction(j);
+                        if (r < 0)
+                                return r;
+                        count ++;
+                }
         }
 
+        /* Complain if the user request matches but nothing whatsoever was
+         * found, since otherwise everything would be matched. */
+        if (!(strv_isempty(arg_system_units) && strv_isempty(arg_user_units)) && count == 0)
+                return -ENODATA;
+
         r = sd_journal_add_conjunction(j);
         if (r < 0)
                 return r;
@@ -1543,16 +1682,22 @@ int main(int argc, char *argv[]) {
         strv_free(arg_system_units);
         strv_free(arg_user_units);
 
-        if (r < 0)
+        if (r < 0) {
+                log_error("Failed to add filter for units: %s", strerror(-r));
                 return EXIT_FAILURE;
+        }
 
         r = add_priorities(j);
-        if (r < 0)
+        if (r < 0) {
+                log_error("Failed to add filter for priorities: %s", strerror(-r));
                 return EXIT_FAILURE;
+        }
 
         r = add_matches(j, argv + optind);
-        if (r < 0)
+        if (r < 0) {
+                log_error("Failed to add filters: %s", strerror(-r));
                 return EXIT_FAILURE;
+        }
 
         if (_unlikely_(log_get_max_level() >= LOG_PRI(LOG_DEBUG))) {
                 _cleanup_free_ char *filter;

commit ae97089d49d1795a35a443b7b830ee666028e733
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Sat Dec 28 19:33:23 2013 -0500

    journal: fix access to munmapped memory in sd_journal_enumerate_unique
    
    sd_j_e_u needs to keep a reference to an object while comparing it
    with possibly duplicate objects in other files. Because the size of
    mmap cache is limited, with enough files and object to compare to,
    at some point the object being compared would be munmapped, resulting
    in a segmentation fault.
    
    Fix this issue by turning keep_always into a reference count that can
    be increased and decreased. Other callers which set keep_always=true
    are unmodified: their references are never released but are ignored
    when the whole file is closed, which happens at some point. keep_always
    is increased in sd_j_e_u and later on released.

diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c
index 275324b..b3747e3 100644
--- a/src/journal/journal-file.c
+++ b/src/journal/journal-file.c
@@ -419,7 +419,6 @@ int journal_file_move_to_object(JournalFile *f, int type, uint64_t offset, Objec
         void *t;
         Object *o;
         uint64_t s;
-        unsigned context;
 
         assert(f);
         assert(ret);
@@ -428,10 +427,8 @@ int journal_file_move_to_object(JournalFile *f, int type, uint64_t offset, Objec
         if (!VALID64(offset))
                 return -EFAULT;
 
-        /* One context for each type, plus one catch-all for the rest */
-        context = type > 0 && type < _OBJECT_TYPE_MAX ? type : 0;
 
-        r = journal_file_move_to(f, context, false, offset, sizeof(ObjectHeader), &t);
+        r = journal_file_move_to(f, type_to_context(type), false, offset, sizeof(ObjectHeader), &t);
         if (r < 0)
                 return r;
 
diff --git a/src/journal/journal-file.h b/src/journal/journal-file.h
index 21b0821..885b3b1 100644
--- a/src/journal/journal-file.h
+++ b/src/journal/journal-file.h
@@ -127,6 +127,10 @@ int journal_file_open_reliably(
 #define ALIGN64(x) (((x) + 7ULL) & ~7ULL)
 #define VALID64(x) (((x) & 7ULL) == 0ULL)
 
+/* Use six characters to cover the offsets common in smallish journal
+ * files without adding too many zeros. */
+#define OFSfmt "%06"PRIx64
+
 static inline bool VALID_REALTIME(uint64_t u) {
         /* This considers timestamps until the year 3112 valid. That should be plenty room... */
         return u > 0 && u < (1ULL << 55);
@@ -196,3 +200,23 @@ int journal_file_get_cutoff_realtime_usec(JournalFile *f, usec_t *from, usec_t *
 int journal_file_get_cutoff_monotonic_usec(JournalFile *f, sd_id128_t boot, usec_t *from, usec_t *to);
 
 bool journal_file_rotate_suggested(JournalFile *f, usec_t max_file_usec);
+
+
+static unsigned type_to_context(int type) {
+        /* One context for each type, plus one catch-all for the rest */
+        return type > 0 && type < _OBJECT_TYPE_MAX ? type : 0;
+}
+
+static inline int journal_file_object_keep(JournalFile *f, Object *o, uint64_t offset) {
+        unsigned context = type_to_context(o->object.type);
+
+        return mmap_cache_get(f->mmap, f->fd, f->prot, context, true,
+                              offset, o->object.size, &f->last_stat, NULL);
+}
+
+static inline int journal_file_object_release(JournalFile *f, Object *o, uint64_t offset) {
+        unsigned context = type_to_context(o->object.type);
+
+        return mmap_cache_release(f->mmap, f->fd, f->prot, context,
+                                  offset, o->object.size);
+}
diff --git a/src/journal/journal-verify.c b/src/journal/journal-verify.c
index 82b0f0a..f2422ff 100644
--- a/src/journal/journal-verify.c
+++ b/src/journal/journal-verify.c
@@ -34,10 +34,6 @@
 #include "compress.h"
 #include "fsprg.h"
 
-/* Use six characters to cover the offsets common in smallish journal
- * files without adding to many zeros. */
-#define OFSfmt "%06"PRIx64
-
 static int journal_file_object_verify(JournalFile *f, uint64_t offset, Object *o) {
         uint64_t i;
 
diff --git a/src/journal/mmap-cache.c b/src/journal/mmap-cache.c
index 117dc2f..7dbbb5e 100644
--- a/src/journal/mmap-cache.c
+++ b/src/journal/mmap-cache.c
@@ -38,7 +38,7 @@ typedef struct FileDescriptor FileDescriptor;
 struct Window {
         MMapCache *cache;
 
-        bool keep_always;
+        unsigned keep_always;
         bool in_unused;
 
         int prot;
@@ -185,7 +185,7 @@ static void context_detach_window(Context *c) {
         c->window = NULL;
         LIST_REMOVE(by_window, w->contexts, c);
 
-        if (!w->contexts && !w->keep_always) {
+        if (!w->contexts && w->keep_always == 0) {
                 /* Not used anymore? */
                 LIST_PREPEND(unused, c->cache->unused, w);
                 if (!c->cache->last_unused)
@@ -360,7 +360,6 @@ static int try_context(
         assert(m->n_ref > 0);
         assert(fd >= 0);
         assert(size > 0);
-        assert(ret);
 
         c = hashmap_get(m->contexts, UINT_TO_PTR(context+1));
         if (!c)
@@ -378,9 +377,10 @@ static int try_context(
                 return 0;
         }
 
-        c->window->keep_always = c->window->keep_always || keep_always;
+        c->window->keep_always += keep_always;
 
-        *ret = (uint8_t*) c->window->ptr + (offset - c->window->offset);
+        if (ret)
+                *ret = (uint8_t*) c->window->ptr + (offset - c->window->offset);
         return 1;
 }
 
@@ -402,7 +402,6 @@ static int find_mmap(
         assert(m->n_ref > 0);
         assert(fd >= 0);
         assert(size > 0);
-        assert(ret);
 
         f = hashmap_get(m->fds, INT_TO_PTR(fd + 1));
         if (!f)
@@ -422,9 +421,10 @@ static int find_mmap(
                 return -ENOMEM;
 
         context_attach_window(c, w);
-        w->keep_always = w->keep_always || keep_always;
+        w->keep_always += keep_always;
 
-        *ret = (uint8_t*) w->ptr + (offset - w->offset);
+        if (ret)
+                *ret = (uint8_t*) w->ptr + (offset - w->offset);
         return 1;
 }
 
@@ -450,7 +450,6 @@ static int add_mmap(
         assert(m->n_ref > 0);
         assert(fd >= 0);
         assert(size > 0);
-        assert(ret);
 
         woffset = offset & ~((uint64_t) page_size() - 1ULL);
         wsize = size + (offset - woffset);
@@ -520,7 +519,8 @@ static int add_mmap(
         c->window = w;
         LIST_PREPEND(by_window, w->contexts, c);
 
-        *ret = (uint8_t*) w->ptr + (offset - w->offset);
+        if (ret)
+                *ret = (uint8_t*) w->ptr + (offset - w->offset);
         return 1;
 }
 
@@ -541,7 +541,6 @@ int mmap_cache_get(
         assert(m->n_ref > 0);
         assert(fd >= 0);
         assert(size > 0);
-        assert(ret);
 
         /* Check whether the current context is the right one already */
         r = try_context(m, fd, prot, context, keep_always, offset, size, ret);
@@ -563,6 +562,42 @@ int mmap_cache_get(
         return add_mmap(m, fd, prot, context, keep_always, offset, size, st, ret);
 }
 
+int mmap_cache_release(
+                MMapCache *m,
+                int fd,
+                int prot,
+                unsigned context,
+                uint64_t offset,
+                size_t size) {
+
+        FileDescriptor *f;
+        Window *w;
+
+        assert(m);
+        assert(m->n_ref > 0);
+        assert(fd >= 0);
+        assert(size > 0);
+
+        f = hashmap_get(m->fds, INT_TO_PTR(fd + 1));
+        if (!f)
+                return -EBADF;
+
+        assert(f->fd == fd);
+
+        LIST_FOREACH(by_fd, w, f->windows)
+                if (window_matches(w, fd, prot, offset, size))
+                        break;
+
+        if (!w)
+                return -ENOENT;
+
+        if (w->keep_always == 0)
+                return -ENOLCK;
+
+        w->keep_always -= 1;
+        return 0;
+}
+
 void mmap_cache_close_fd(MMapCache *m, int fd) {
         FileDescriptor *f;
 
diff --git a/src/journal/mmap-cache.h b/src/journal/mmap-cache.h
index 912336d..647555a 100644
--- a/src/journal/mmap-cache.h
+++ b/src/journal/mmap-cache.h
@@ -31,7 +31,23 @@ MMapCache* mmap_cache_new(void);
 MMapCache* mmap_cache_ref(MMapCache *m);
 MMapCache* mmap_cache_unref(MMapCache *m);
 
-int mmap_cache_get(MMapCache *m, int fd, int prot, unsigned context, bool keep_always, uint64_t offset, size_t size, struct stat *st, void **ret);
+int mmap_cache_get(
+        MMapCache *m,
+        int fd,
+        int prot,
+        unsigned context,
+        bool keep_always,
+        uint64_t offset,
+        size_t size,
+        struct stat *st,
+        void **ret);
+int mmap_cache_release(
+        MMapCache *m,
+        int fd,
+        int prot,
+        unsigned context,
+        uint64_t offset,
+        size_t size);
 void mmap_cache_close_fd(MMapCache *m, int fd);
 void mmap_cache_close_context(MMapCache *m, unsigned context);
 
diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c
index 283d593..7466006 100644
--- a/src/journal/sd-journal.c
+++ b/src/journal/sd-journal.c
@@ -2481,9 +2481,7 @@ _public_ int sd_journal_query_unique(sd_journal *j, const char *field) {
 }
 
 _public_ int sd_journal_enumerate_unique(sd_journal *j, const void **data, size_t *l) {
-        Object *o;
         size_t k;
-        int r;
 
         assert_return(j, -EINVAL);
         assert_return(!journal_pid_changed(j), -ECHILD);
@@ -2503,9 +2501,11 @@ _public_ int sd_journal_enumerate_unique(sd_journal *j, const void **data, size_
         for (;;) {
                 JournalFile *of;
                 Iterator i;
+                Object *o;
                 const void *odata;
                 size_t ol;
                 bool found;
+                int r;
 
                 /* Proceed to next data object in the field's linked list */
                 if (j->unique_offset == 0) {
@@ -2542,8 +2542,16 @@ _public_ int sd_journal_enumerate_unique(sd_journal *j, const void **data, size_
                         return r;
 
                 /* Let's do the type check by hand, since we used 0 context above. */
-                if (o->object.type != OBJECT_DATA)
+                if (o->object.type != OBJECT_DATA) {
+                        log_error("%s:offset " OFSfmt ": object has type %d, expected %d",
+                                  j->unique_file->path, j->unique_offset,
+                                  o->object.type, OBJECT_DATA);
                         return -EBADMSG;
+                }
+
+                r = journal_file_object_keep(j->unique_file, o, j->unique_offset);
+                if (r < 0)
+                        return r;
 
                 r = return_data(j, j->unique_file, o, &odata, &ol);
                 if (r < 0)
@@ -2577,6 +2585,10 @@ _public_ int sd_journal_enumerate_unique(sd_journal *j, const void **data, size_
                 if (found)
                         continue;
 
+                r = journal_file_object_release(j->unique_file, o, j->unique_offset);
+                if (r < 0)
+                        return r;
+
                 r = return_data(j, j->unique_file, o, data, l);
                 if (r < 0)
                         return r;



More information about the systemd-commits mailing list