platform: fix wrapper nm_platform_addr_flags2str() for rtnl_addr_flags2str()th/nm_platform_addr_flags2str

The compatibily wrapper for rtnl_addr_flags2str() did not
behave identical because libnl adds a trailing ',' if it
encounters unknown attributes.

Try to fix that and add test cases.

Signed-off-by: Thomas Haller <thaller@redhat.com>

4 files changed, 136 insertions, 22 deletions

diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
index ad39084c69..bd5e2e64e4 100644
--- a/src/platform/nm-platform.c
+++ b/src/platform/nm-platform.c
@@ -1955,20 +1955,39 @@ nm_platform_ip4_address_to_string (const NMPlatformIP4Address *address)
 void
 nm_platform_addr_flags2str (int flags, char *buf, size_t size)
 {
-	rtnl_addr_flags2str(flags, buf, size);
-
-	/* There are two recent flags IFA_F_MANAGETEMPADDR and IFA_F_NOPREFIXROUTE.
-	 * If libnl does not yet support them, add them by hand.
-	 * These two flags were introduced together with the extended ifa_flags,
-	 * so, check for that.
-	 */
-	if ((flags & IFA_F_MANAGETEMPADDR) && !nm_platform_check_support_libnl_extended_ifa_flags ()) {
-		strncat (buf, buf[0] ? "," IFA_F_MANAGETEMPADDR_STR : IFA_F_MANAGETEMPADDR_STR,
-		         size - strlen (buf) - 1);
-	}
-	if ((flags & IFA_F_NOPREFIXROUTE) && !nm_platform_check_support_libnl_extended_ifa_flags ()) {
-		strncat (buf, buf[0] ? "," IFA_F_NOPREFIXROUTE_STR : IFA_F_NOPREFIXROUTE_STR,
-		         size - strlen (buf) - 1);
+	if (!(flags & (IFA_F_MANAGETEMPADDR | IFA_F_NOPREFIXROUTE)) ||
+	    nm_platform_check_support_libnl_extended_ifa_flags ())
+		rtnl_addr_flags2str (flags, buf, size);
+	else {
+		/* There are two recent flags IFA_F_MANAGETEMPADDR and IFA_F_NOPREFIXROUTE.
+		 * If libnl does not yet support them, add them by hand.
+		 * These two flags were introduced together with the extended ifa_flags
+		 * so check for nm_platform_check_support_libnl_extended_ifa_flags (). */
+		gboolean has_other_unknown_flags = FALSE;
+		size_t len;
+
+		/* if there are unknown flags to rtnl_addr_flags2str(), libnl appends ','
+		 * to indicate them. We want to keep this behavior, if there are other
+		 * unknown flags present. */
+
+		rtnl_addr_flags2str (flags & ~(IFA_F_MANAGETEMPADDR | IFA_F_NOPREFIXROUTE), buf, size);
+
+		len = strlen (buf);
+		if (len > 0) {
+			has_other_unknown_flags = (buf[len - 1] == ',');
+			if (!has_other_unknown_flags)
+				g_strlcat (buf, ",", size);
+		}
+
+		if ((flags & (IFA_F_MANAGETEMPADDR | IFA_F_NOPREFIXROUTE)) == (IFA_F_MANAGETEMPADDR | IFA_F_NOPREFIXROUTE))
+			g_strlcat (buf, IFA_F_MANAGETEMPADDR_STR","IFA_F_NOPREFIXROUTE_STR, size);
+		else if (flags & IFA_F_MANAGETEMPADDR)
+			g_strlcat (buf, IFA_F_MANAGETEMPADDR_STR, size);
+		else
+			g_strlcat (buf, IFA_F_NOPREFIXROUTE_STR, size);
+
+		if (has_other_unknown_flags)
+			g_strlcat (buf, ",", size);
 	}
 }
 
@@ -1987,10 +2006,10 @@ nm_platform_addr_flags2str (int flags, char *buf, size_t size)
 const char *
 nm_platform_ip6_address_to_string (const NMPlatformIP6Address *address)
 {
-	char s_flags[256];
+#define S_FLAGS_PREFIX " flags "
+	char s_flags[128];
 	char s_address[INET6_ADDRSTRLEN];
 	char s_peer[INET6_ADDRSTRLEN];
-	char *str_flags;
 	char str_dev[TO_STRING_DEV_BUF_SIZE];
 	char *str_peer = NULL;
 
@@ -2005,18 +2024,19 @@ nm_platform_ip6_address_to_string (const NMPlatformIP6Address *address)
 
 	_to_string_dev (address->ifindex, str_dev, sizeof (str_dev));
 
-	nm_platform_addr_flags2str (address->flags, s_flags, sizeof (s_flags));
-
-	str_flags = s_flags[0] ? g_strconcat (" flags ", s_flags, NULL) : NULL;
+	nm_platform_addr_flags2str (address->flags, &s_flags[STRLEN (S_FLAGS_PREFIX)], sizeof (s_flags) - STRLEN (S_FLAGS_PREFIX));
+	if (s_flags[STRLEN (S_FLAGS_PREFIX)] == '\0')
+		s_flags[0] = '\0';
+	else
+		memcpy (s_flags, S_FLAGS_PREFIX, STRLEN (S_FLAGS_PREFIX));
 
 	g_snprintf (to_string_buffer, sizeof (to_string_buffer), "%s/%d lft %u pref %u time %u%s%s%s src %s",
 	            s_address, address->plen, (guint)address->lifetime, (guint)address->preferred,
 	            (guint)address->timestamp,
 	            str_peer ? str_peer : "",
 	            str_dev,
-	            str_flags ? str_flags : "",
+	            s_flags,
 	            source_to_string (address->source));
-	g_free (str_flags);
 	g_free (str_peer);
 	return to_string_buffer;
 }
diff --git a/src/platform/tests/.gitignore b/src/platform/tests/.gitignore
index 5457fbca6c..5633ec4795 100644
--- a/src/platform/tests/.gitignore
+++ b/src/platform/tests/.gitignore
@@ -1,6 +1,7 @@
 /dump
 /monitor
 /platform
+/test-general
 /test-link-fake
 /test-link-linux
 /test-address-fake
diff --git a/src/platform/tests/Makefile.am b/src/platform/tests/Makefile.am
index b94e12e606..4500435be1 100644
--- a/src/platform/tests/Makefile.am
+++ b/src/platform/tests/Makefile.am
@@ -29,6 +29,7 @@ noinst_PROGRAMS = \
 	dump \
 	monitor \
 	platform \
+	test-general \
 	test-link-fake \
 	test-link-linux \
 	test-address-fake \
@@ -105,6 +106,13 @@ test_cleanup_linux_CPPFLAGS = \
 	-DKERNEL_HACKS=1
 test_cleanup_linux_LDADD = $(PLATFORM_LDADD)
 
+test_general_SOURCES = \
+	test-general.c
+
+test_general_LDADD = \
+	$(top_builddir)/src/libNetworkManager.la
+
+
 # Unfortunately, we cannot run nm-linux-platform-test as an automatic test
 # program by default, as it requires root access and modifies kernel
 # configuration.
@@ -113,7 +121,7 @@ test_cleanup_linux_LDADD = $(PLATFORM_LDADD)
 # correctly.
 
 @VALGRIND_RULES@
-USERTESTS = test-link-fake test-address-fake test-route-fake test-cleanup-fake
+USERTESTS = test-link-fake test-address-fake test-route-fake test-cleanup-fake test-general
 ROOTTESTS = test-link-linux test-address-linux test-route-linux test-cleanup-linux
 
 # If explicitly enabled, we can run the root tests
diff --git a/src/platform/tests/test-general.c b/src/platform/tests/test-general.c
new file mode 100644
index 0000000000..d7c1f9863b
--- /dev/null
+++ b/src/platform/tests/test-general.c
@@ -0,0 +1,85 @@
+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Copyright (C) 2014 Red Hat, Inc.
+ *
+ */
+
+#include <glib.h>
+#include <string.h>
+#include <errno.h>
+
+#include "NetworkManagerUtils.h"
+#include "nm-utils.h"
+#include "nm-logging.h"
+
+
+static void
+ASSERT_CONTAINS_SUBSTR (const char *str, const char *substr)
+{
+	g_assert (str);
+	g_assert (substr);
+	if (strstr (str, substr) == NULL) {
+		nm_log_dbg (LOGD_PLATFORM, "Expects \"%s\" but got \"%s\"", substr, str ? str : "(null)");
+		g_assert_cmpstr (str, ==, substr);
+	}
+}
+
+static void
+test_nm_platform_ip6_address_to_string_flags (void)
+{
+	NMPlatformIP6Address addr = { 0 };
+
+	g_assert_cmpstr (strstr (nm_platform_ip6_address_to_string (&addr), " flags "), ==, NULL);
+
+	addr.flags = IFA_F_MANAGETEMPADDR;
+	ASSERT_CONTAINS_SUBSTR (nm_platform_ip6_address_to_string (&addr), " flags mngtmpaddr ");
+
+	addr.flags = IFA_F_NOPREFIXROUTE;
+	ASSERT_CONTAINS_SUBSTR (nm_platform_ip6_address_to_string (&addr), " flags noprefixroute ");
+
+	addr.flags = IFA_F_MANAGETEMPADDR | IFA_F_NOPREFIXROUTE;
+	ASSERT_CONTAINS_SUBSTR (nm_platform_ip6_address_to_string (&addr), " flags mngtmpaddr,noprefixroute ");
+
+	addr.flags = IFA_F_TENTATIVE | IFA_F_NOPREFIXROUTE;
+	ASSERT_CONTAINS_SUBSTR (nm_platform_ip6_address_to_string (&addr), " flags tentative,noprefixroute ");
+
+	addr.flags = IFA_F_TENTATIVE | IFA_F_PERMANENT | IFA_F_MANAGETEMPADDR| IFA_F_NOPREFIXROUTE;
+	ASSERT_CONTAINS_SUBSTR (nm_platform_ip6_address_to_string (&addr), " flags tentative,permanent,mngtmpaddr,noprefixroute ");
+
+	addr.flags = IFA_F_TENTATIVE | IFA_F_PERMANENT | IFA_F_MANAGETEMPADDR| IFA_F_NOPREFIXROUTE | 0x8000;
+	ASSERT_CONTAINS_SUBSTR (nm_platform_ip6_address_to_string (&addr), " flags tentative,permanent,mngtmpaddr,noprefixroute, ");
+
+	addr.flags = IFA_F_TENTATIVE | IFA_F_PERMANENT | IFA_F_MANAGETEMPADDR| IFA_F_NOPREFIXROUTE | ((G_MAXUINT - (G_MAXUINT >> 1)) >> 1);
+	ASSERT_CONTAINS_SUBSTR (nm_platform_ip6_address_to_string (&addr), " flags tentative,permanent,mngtmpaddr,noprefixroute, ");
+}
+
+/*******************************************/
+
+int
+main (int argc, char **argv)
+{
+	g_test_init (&argc, &argv, NULL);
+
+	g_type_init ();
+
+	nm_logging_setup ("DEBUG", "ALL", NULL, NULL);
+
+	g_test_add_func ("/general/nm_platform_ip6_address_to_string/flags", test_nm_platform_ip6_address_to_string_flags);
+
+	return g_test_run ();
+}
+