From c66ee23f73a0b8e4056abf4a2a463ae48346f249 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 12 Feb 2016 15:43:30 +0100 Subject: [PATCH 1/8] macros: add nm_auto_free macro Similar to gs_free to cleanup pointers with free(). Note that g_free() and free() cannot be used interchangably. --- shared/nm-default.h | 2 ++ shared/nm-macros-internal.h | 10 +++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/shared/nm-default.h b/shared/nm-default.h index f1fcb845e..72b74a549 100644 --- a/shared/nm-default.h +++ b/shared/nm-default.h @@ -41,6 +41,8 @@ /* always include these headers for our internal source files. */ +#include + #include "nm-glib.h" #include "nm-version.h" #include "gsystem-local-alloc.h" diff --git a/shared/nm-macros-internal.h b/shared/nm-macros-internal.h index 63f513847..4d802746b 100644 --- a/shared/nm-macros-internal.h +++ b/shared/nm-macros-internal.h @@ -22,7 +22,15 @@ #ifndef __NM_MACROS_INTERNAL_H__ #define __NM_MACROS_INTERNAL_H__ -#include "nm-default.h" +/********************************************************/ + +/** + * nm_auto_free: + * + * Call free() on a variable location when it goes out of scope. + */ +#define nm_auto_free __attribute__ ((cleanup(_nm_auto_free_impl))) +GS_DEFINE_CLEANUP_FUNCTION(void*, _nm_auto_free_impl, free) /********************************************************/ From 04805f659f7b6b9c0782d70b809900c57d6f9de4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 12 Feb 2016 15:46:20 +0100 Subject: [PATCH 2/8] platform: simplify event_handler_recvmsgs() by using cleanup attribute --- src/platform/nm-linux-platform.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 4c3ba361c..734f0ffac 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -5557,7 +5557,6 @@ event_handler_recvmsgs (NMPlatform *platform, gboolean handle_events) NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); struct nl_sock *sk = priv->nlh; int n, err = 0, multipart = 0, interrupted = 0, nrecv = 0; - unsigned char *buf = NULL; struct nlmsghdr *hdr; WaitForNlResponseResult seq_result; @@ -5567,10 +5566,12 @@ event_handler_recvmsgs (NMPlatform *platform, gboolean handle_events) initialize the variable. Thomas Graf. */ struct sockaddr_nl nla = {0}; - struct nl_msg *msg = NULL; - struct ucred *creds = NULL; + nm_auto_free struct ucred *creds = NULL; + nm_auto_free unsigned char *buf = NULL; continue_reading: + g_clear_pointer (&buf, free); + g_clear_pointer (&creds, free); errno = 0; n = nl_recv (sk, &nla, &buf, &creds); @@ -5600,9 +5601,9 @@ continue_reading: hdr = (struct nlmsghdr *) buf; while (nlmsg_ok (hdr, n)) { + nm_auto_nlmsg struct nl_msg *msg = NULL; gboolean abort_parsing = FALSE; - nlmsg_free (msg); msg = nlmsg_convert (hdr); if (!msg) { err = -NLE_NOMEM; @@ -5704,13 +5705,6 @@ continue_reading: goto out; } - nlmsg_free (msg); - free (buf); - free (creds); - buf = NULL; - msg = NULL; - creds = NULL; - if (multipart) { /* Multipart message not yet complete, continue reading */ goto continue_reading; @@ -5724,10 +5718,6 @@ stop: } err = 0; out: - nlmsg_free (msg); - free (buf); - free (creds); - if (interrupted) err = -NLE_DUMP_INTR; From 9254cbe875f6868c8428ed36caae0e6666bd2f49 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 12 Feb 2016 16:25:33 +0100 Subject: [PATCH 3/8] platform: don't return number of messages from event_handler_recvmsgs() The value is not used by the callers. Also, with @handle_events set to false, it is not clear what the value really means because we skip over errors. --- src/platform/nm-linux-platform.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 734f0ffac..9ce4422ab 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -5556,7 +5556,7 @@ event_handler_recvmsgs (NMPlatform *platform, gboolean handle_events) { NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); struct nl_sock *sk = priv->nlh; - int n, err = 0, multipart = 0, interrupted = 0, nrecv = 0; + int n, err = 0, multipart = 0, interrupted = 0; struct nlmsghdr *hdr; WaitForNlResponseResult seq_result; @@ -5612,7 +5612,6 @@ continue_reading: nlmsg_set_proto (msg, NETLINK_ROUTE); nlmsg_set_src (msg, &nla); - nrecv++; if (!creds || creds->pid) { if (creds) @@ -5720,10 +5719,6 @@ stop: out: if (interrupted) err = -NLE_DUMP_INTR; - - if (!err) - err = nrecv; - return err; } From 43381f9b854d76c0cf4cb7b7413eeeee09cc885c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 12 Feb 2016 16:36:25 +0100 Subject: [PATCH 4/8] platform: downgrade logging message to TRACE level in event_handler_recvmsgs() Doesn't seem important and might be triggered by other processes. --- src/platform/nm-linux-platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 9ce4422ab..3469a8289 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -5615,9 +5615,9 @@ continue_reading: if (!creds || creds->pid) { if (creds) - _LOGD ("netlink: recvmsg: received non-kernel message (pid %d)", creds->pid); + _LOGT ("netlink: recvmsg: received non-kernel message (pid %d)", creds->pid); else - _LOGD ("netlink: recvmsg: received message without credentials"); + _LOGT ("netlink: recvmsg: received message without credentials"); goto stop; } From dc97a3b39b75228c9b436d416167db30bc48163a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 12 Feb 2016 16:20:52 +0100 Subject: [PATCH 5/8] platform: fix error handling in event_handler_recvmsgs() @abort_parsing is set TRUE at two places, which also explicitly set @err to something. We don't want to reset @err and got to the next @hdr. Instead error out first. --- src/platform/nm-linux-platform.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 3469a8289..76ca16f30 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -5697,11 +5697,12 @@ continue_reading: } event_seq_check (platform, msg, seq_result); - err = 0; - hdr = nlmsg_next (hdr, &n); if (abort_parsing) goto out; + + err = 0; + hdr = nlmsg_next (hdr, &n); } if (multipart) { From 3f00899bdedaee8fcd0168a267785565de20e794 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 12 Feb 2016 16:29:46 +0100 Subject: [PATCH 6/8] platform: continue reading in event_handler_recvmsgs() when not handling events If @handle_events is FALSE, we want to drain the socket. In that case even when encountering an error error we don't want to abort, but instead continue reading the next message. --- src/platform/nm-linux-platform.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 76ca16f30..0723bf309 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -5698,8 +5698,11 @@ continue_reading: event_seq_check (platform, msg, seq_result); - if (abort_parsing) + if (abort_parsing) { + if (!handle_events) + goto continue_reading; goto out; + } err = 0; hdr = nlmsg_next (hdr, &n); From 329ac02d386f1b3bfd63a5dbc0025d612110c8af Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 12 Feb 2016 16:39:05 +0100 Subject: [PATCH 7/8] platform: don't set @err during stop: in event_handler_recvmsgs() If we break the loop normally, @err must be already set to zero. The only other way this can happen is when the credentials are invalid. Move setting @err to there. --- src/platform/nm-linux-platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 0723bf309..a5b277d6a 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -5618,6 +5618,7 @@ continue_reading: _LOGT ("netlink: recvmsg: received non-kernel message (pid %d)", creds->pid); else _LOGT ("netlink: recvmsg: received message without credentials"); + err = 0; goto stop; } @@ -5719,7 +5720,6 @@ stop: * Repeat reading. */ goto continue_reading; } - err = 0; out: if (interrupted) err = -NLE_DUMP_INTR; From 3d759b1f11110216749fbd4893e3e63594a8d70d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 12 Feb 2016 16:41:51 +0100 Subject: [PATCH 8/8] platform: during @abort_parsing goto stop in event_handler_recvmsgs() Now, that we no longer overwrite @err, we can jump to stop: instead of out:. --- src/platform/nm-linux-platform.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index a5b277d6a..f442f88b6 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -5699,11 +5699,8 @@ continue_reading: event_seq_check (platform, msg, seq_result); - if (abort_parsing) { - if (!handle_events) - goto continue_reading; - goto out; - } + if (abort_parsing) + goto stop; err = 0; hdr = nlmsg_next (hdr, &n);