diff --git a/src/basic/log.c b/src/basic/log.c index 8b973f9e0a..c713b9ac88 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -73,6 +73,7 @@ static bool prohibit_ipc = false; static char *log_abort_msg = NULL; typedef struct LogContext { + unsigned n_ref; /* Depending on which destructor is used (log_context_free() or log_context_detach()) the memory * referenced by this is freed or not */ char **fields; @@ -1562,7 +1563,7 @@ bool log_context_enabled(void) { return saved_log_context_enabled; } -LogContext* log_context_attach(LogContext *c) { +static LogContext* log_context_attach(LogContext *c) { assert(c); _log_context_num_fields += strv_length(c->fields); @@ -1571,7 +1572,7 @@ LogContext* log_context_attach(LogContext *c) { return LIST_PREPEND(ll, _log_context, c); } -LogContext* log_context_detach(LogContext *c) { +static LogContext* log_context_detach(LogContext *c) { if (!c) return NULL; @@ -1584,11 +1585,21 @@ LogContext* log_context_detach(LogContext *c) { } LogContext* log_context_new(char **fields, bool owned) { + if (!fields) + return NULL; + + LIST_FOREACH(ll, i, _log_context) + if (i->fields == fields) { + assert(!owned); + return log_context_ref(i); + } + LogContext *c = new(LogContext, 1); if (!c) return NULL; *c = (LogContext) { + .n_ref = 1, .fields = fields, .owned = owned, }; @@ -1598,13 +1609,20 @@ LogContext* log_context_new(char **fields, bool owned) { LogContext* log_context_newv(struct iovec *input_iovec, size_t n_input_iovec, bool owned) { if (!input_iovec || n_input_iovec == 0) - return NULL; /* Nothing to do */ + return NULL; + + LIST_FOREACH(ll, i, _log_context) + if (i->input_iovec == input_iovec && i->n_input_iovec == n_input_iovec) { + assert(!owned); + return log_context_ref(i); + } LogContext *c = new(LogContext, 1); if (!c) return NULL; *c = (LogContext) { + .n_ref = 1, .input_iovec = input_iovec, .n_input_iovec = n_input_iovec, .owned = owned, @@ -1613,7 +1631,7 @@ LogContext* log_context_newv(struct iovec *input_iovec, size_t n_input_iovec, bo return log_context_attach(c); } -LogContext* log_context_free(LogContext *c) { +static LogContext* log_context_free(LogContext *c) { if (!c) return NULL; @@ -1627,6 +1645,8 @@ LogContext* log_context_free(LogContext *c) { return mfree(c); } +DEFINE_TRIVIAL_REF_UNREF_FUNC(LogContext, log_context, log_context_free); + LogContext* log_context_new_consume(char **fields) { LogContext *c = log_context_new(fields, /*owned=*/ true); if (!c) diff --git a/src/basic/log.h b/src/basic/log.h index c4ac73e27b..f17a97ee37 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -457,25 +457,23 @@ typedef struct LogContext LogContext; bool log_context_enabled(void); -LogContext* log_context_attach(LogContext *c); -LogContext* log_context_detach(LogContext *c); - LogContext* log_context_new(char **fields, bool owned); LogContext* log_context_newv(struct iovec *input_iovec, size_t n_input_iovec, bool owned); -LogContext* log_context_free(LogContext *c); /* Same as log_context_new(), but frees the given fields strv/iovec on failure. */ LogContext* log_context_new_consume(char **fields); LogContext* log_context_new_consumev(struct iovec *input_iovec, size_t n_input_iovec); +LogContext *log_context_ref(LogContext *c); +LogContext *log_context_unref(LogContext *c); + +DEFINE_TRIVIAL_CLEANUP_FUNC(LogContext*, log_context_unref); + /* Returns the number of attached log context objects. */ size_t log_context_num_contexts(void); /* Returns the number of fields in all attached log contexts. */ size_t log_context_num_fields(void); -DEFINE_TRIVIAL_CLEANUP_FUNC(LogContext*, log_context_detach); -DEFINE_TRIVIAL_CLEANUP_FUNC(LogContext*, log_context_free); - #define LOG_CONTEXT_PUSH(...) \ LOG_CONTEXT_PUSH_STRV(STRV_MAKE(__VA_ARGS__)) @@ -483,13 +481,13 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(LogContext*, log_context_free); LOG_CONTEXT_PUSH(snprintf_ok((char[LINE_MAX]) {}, LINE_MAX, __VA_ARGS__)) #define _LOG_CONTEXT_PUSH_STRV(strv, c) \ - _unused_ _cleanup_(log_context_freep) LogContext *c = log_context_new(strv, /*owned=*/ false); + _unused_ _cleanup_(log_context_unrefp) LogContext *c = log_context_new(strv, /*owned=*/ false); #define LOG_CONTEXT_PUSH_STRV(strv) \ _LOG_CONTEXT_PUSH_STRV(strv, UNIQ_T(c, UNIQ)) #define _LOG_CONTEXT_PUSH_IOV(input_iovec, n_input_iovec, c) \ - _unused_ _cleanup_(log_context_freep) LogContext *c = log_context_newv(input_iovec, n_input_iovec, /*owned=*/ false); + _unused_ _cleanup_(log_context_unrefp) LogContext *c = log_context_newv(input_iovec, n_input_iovec, /*owned=*/ false); #define LOG_CONTEXT_PUSH_IOV(input_iovec, n_input_iovec) \ _LOG_CONTEXT_PUSH_IOV(input_iovec, n_input_iovec, UNIQ_T(c, UNIQ)) @@ -503,19 +501,19 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(LogContext*, log_context_free); _unused_ _cleanup_strv_free_ strv = strv_new(s); \ if (!strv) \ free(s); \ - _unused_ _cleanup_(log_context_freep) LogContext *c = log_context_new_consume(TAKE_PTR(strv)) + _unused_ _cleanup_(log_context_unrefp) LogContext *c = log_context_new_consume(TAKE_PTR(strv)) #define LOG_CONTEXT_CONSUME_STR(s) \ _LOG_CONTEXT_CONSUME_STR(s, UNIQ_T(c, UNIQ), UNIQ_T(sv, UNIQ)) #define _LOG_CONTEXT_CONSUME_STRV(strv, c) \ - _unused_ _cleanup_(log_context_freep) LogContext *c = log_context_new_consume(strv); + _unused_ _cleanup_(log_context_unrefp) LogContext *c = log_context_new_consume(strv); #define LOG_CONTEXT_CONSUME_STRV(strv) \ _LOG_CONTEXT_CONSUME_STRV(strv, UNIQ_T(c, UNIQ)) #define _LOG_CONTEXT_CONSUME_IOV(input_iovec, n_input_iovec, c) \ - _unused_ _cleanup_(log_context_freep) LogContext *c = log_context_new_consumev(input_iovec, n_input_iovec); + _unused_ _cleanup_(log_context_unrefp) LogContext *c = log_context_new_consumev(input_iovec, n_input_iovec); #define LOG_CONTEXT_CONSUME_IOV(input_iovec, n_input_iovec) \ _LOG_CONTEXT_CONSUME_IOV(input_iovec, n_input_iovec, UNIQ_T(c, UNIQ)) diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index e3f71ca302..4387db3841 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -2940,7 +2940,7 @@ static int process_fd_check(sd_bus *bus, sd_bus_message *m) { } static int process_message(sd_bus *bus, sd_bus_message *m) { - _unused_ _cleanup_(log_context_freep) LogContext *c = NULL; + _unused_ _cleanup_(log_context_unrefp) LogContext *c = NULL; int r; assert(bus); diff --git a/src/libsystemd/sd-device/device-monitor.c b/src/libsystemd/sd-device/device-monitor.c index 0d2eea5f59..1d8b7550b6 100644 --- a/src/libsystemd/sd-device/device-monitor.c +++ b/src/libsystemd/sd-device/device-monitor.c @@ -242,7 +242,7 @@ _public_ int sd_device_monitor_stop(sd_device_monitor *m) { static int device_monitor_event_handler(sd_event_source *s, int fd, uint32_t revents, void *userdata) { _cleanup_(sd_device_unrefp) sd_device *device = NULL; - _unused_ _cleanup_(log_context_freep) LogContext *c = NULL; + _unused_ _cleanup_(log_context_unrefp) LogContext *c = NULL; sd_device_monitor *m = ASSERT_PTR(userdata); if (device_monitor_receive_device(m, &device) <= 0) diff --git a/src/test/test-log.c b/src/test/test-log.c index 68b5cb5092..cfbd45c2e3 100644 --- a/src/test/test-log.c +++ b/src/test/test-log.c @@ -80,9 +80,10 @@ static void test_log_context(void) { LOG_CONTEXT_PUSH_STRV(strv); LOG_CONTEXT_PUSH_STRV(strv); - /* Test that the log context was set up correctly. */ - assert_se(log_context_num_contexts() == 4); - assert_se(log_context_num_fields() == 6); + /* Test that the log context was set up correctly. The strv we pushed twice should only + * result in one log context which is reused. */ + assert_se(log_context_num_contexts() == 3); + assert_se(log_context_num_fields() == 4); /* Test that everything still works with modifications to the log context. */ test_log_struct(); @@ -94,8 +95,8 @@ static void test_log_context(void) { LOG_CONTEXT_PUSH_STRV(strv); /* Check that our nested fields got added correctly. */ - assert_se(log_context_num_contexts() == 6); - assert_se(log_context_num_fields() == 9); + assert_se(log_context_num_contexts() == 4); + assert_se(log_context_num_fields() == 5); /* Test that everything still works in a nested block. */ test_log_struct(); @@ -104,15 +105,15 @@ static void test_log_context(void) { } /* Check that only the fields from the nested block got removed. */ - assert_se(log_context_num_contexts() == 4); - assert_se(log_context_num_fields() == 6); + assert_se(log_context_num_contexts() == 3); + assert_se(log_context_num_fields() == 4); } assert_se(log_context_num_contexts() == 0); assert_se(log_context_num_fields() == 0); { - _cleanup_(log_context_freep) LogContext *ctx = NULL; + _cleanup_(log_context_unrefp) LogContext *ctx = NULL; char **strv = STRV_MAKE("SIXTH=ijn", "SEVENTH=PRP"); assert_se(ctx = log_context_new(strv, /*owned=*/ false)); @@ -146,6 +147,7 @@ static void test_log_context(void) { assert_se(iovw); assert_se(iovw_consume(iovw, strdup("MNO=pqr"), STRLEN("MNO=pqr") + 1) == 0); + LOG_CONTEXT_PUSH_IOV(iov, ELEMENTSOF(iov)); LOG_CONTEXT_PUSH_IOV(iov, ELEMENTSOF(iov)); LOG_CONTEXT_CONSUME_IOV(iovw->iovec, iovw->count); LOG_CONTEXT_PUSH("STU=vwx");