summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>2016-07-25 12:15:57 -0400
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>2016-07-25 12:15:57 -0400
commit662bea6729d4147dfdd1501f81335adf5a2a6012 (patch)
treec6bf1a6690dfb1a511a1fdd76b172b11917d3f1f /src
parenta733551846bcb2c7f46cc68bbc2e5741e653fbef (diff)
systemctl: avoid "leaking" some strings in UnitStatusInfo
% valgrind --leak-check=full systemctl status multipathd.service --no-pager -n0 ... ==431== 16 bytes in 2 blocks are definitely lost in loss record 1 of 2 ==431== at 0x4C2BBAD: malloc (vg_replace_malloc.c:299) ==431== by 0x534AF19: strdup (in /usr/lib64/libc-2.23.so) ==431== by 0x4E81AEE: free_and_strdup (string-util.c:794) ==431== by 0x4EF66C1: map_basic (bus-util.c:1030) ==431== by 0x4EF6A8E: bus_message_map_all_properties (bus-util.c:1153) ==431== by 0x120487: show_one (systemctl.c:4672) ==431== by 0x1218F3: show (systemctl.c:4990) ==431== by 0x4EC359E: dispatch_verb (verbs.c:92) ==431== by 0x12A3AE: systemctl_main (systemctl.c:7742) ==431== by 0x12B1A8: main (systemctl.c:8011) ==431== ==431== LEAK SUMMARY: ==431== definitely lost: 16 bytes in 2 blocks This happens because map_basic() strdups the strings. Other code in systemctl assigns strings to UnitStatusInfo without copying them, relying on the fact that the message is longer lived than UnitStatusInfo. Add a helper function that is similar to map_basic, but only accepts strings and does not copy them. The alternative of continuing to use map_basic() but adding proper cleanup to free fields in UnitStatusInfo seems less attractive because it'd require changing a lot of code and doing a lot of more allocations for little gain. (I put "leaking" in quotes, because systemctl is short lived anyway.)
Diffstat (limited to 'src')
-rw-r--r--src/systemctl/systemctl.c23
1 files changed, 19 insertions, 4 deletions
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index e1bcef2d95..d4ec1da290 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -224,6 +224,21 @@ static void release_busses(void) {
busses[w] = sd_bus_flush_close_unref(busses[w]);
}
+static int map_string_no_copy(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) {
+ char *s;
+ const char **p = userdata;
+ int r;
+
+ r = sd_bus_message_read_basic(m, SD_BUS_TYPE_STRING, &s);
+ if (r < 0)
+ return r;
+
+ if (!isempty(s))
+ *p = s;
+
+ return 0;
+}
+
static void ask_password_agent_open_if_enabled(void) {
/* Open the password agent as a child process if necessary */
@@ -4632,8 +4647,8 @@ static int show_one(
bool *ellipsized) {
static const struct bus_properties_map property_map[] = {
- { "LoadState", "s", NULL, offsetof(UnitStatusInfo, load_state) },
- { "ActiveState", "s", NULL, offsetof(UnitStatusInfo, active_state) },
+ { "LoadState", "s", map_string_no_copy, offsetof(UnitStatusInfo, load_state) },
+ { "ActiveState", "s", map_string_no_copy, offsetof(UnitStatusInfo, active_state) },
{}
};
@@ -5664,8 +5679,8 @@ static int unit_exists(const char *unit) {
_cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
_cleanup_free_ char *path = NULL;
static const struct bus_properties_map property_map[] = {
- { "LoadState", "s", NULL, offsetof(UnitStatusInfo, load_state) },
- { "ActiveState", "s", NULL, offsetof(UnitStatusInfo, active_state)},
+ { "LoadState", "s", map_string_no_copy, offsetof(UnitStatusInfo, load_state) },
+ { "ActiveState", "s", map_string_no_copy, offsetof(UnitStatusInfo, active_state)},
{},
};
UnitStatusInfo info = {};