libnl-util: refactor nm_utils_ip4_prefix_to_netmask/netmask_to_prefix

- use a more efficient implementation for prefix_to_netmask
- fix netmask_to_prefix to behave consistently in case of
  invalid netmask
- remove unused duplicated functions from NetworkManagerUtils.c
- add test functions

Based-on-patch-by: Pavel Šimerda <psimerda@redhat.com>
Signed-off-by: Thomas Haller <thaller@redhat.com>
Related: https://bugzilla.gnome.org/show_bug.cgi?id=721771

libnm-util/nm-utils.c

@@ -1060,25 +1060,28 @@ nm_utils_ip4_routes_to_gvalue (GSList *list, GValue *value)
 guint32
 nm_utils_ip4_netmask_to_prefix (guint32 netmask)
 {
-	guchar *p, *end;
-	guint32 prefix = 0;
+	guint32 prefix;
+	guint8 v;
+	const guint8 *p = (guint8 *) &netmask;
 
-	p = (guchar *) &netmask;
-	end = p + sizeof (guint32);
-
-	while ((*p == 0xFF) && p < end) {
-		prefix += 8;
-		p++;
+	if (p[3]) {
+		prefix = 24;
+		v = p[3];
+	} else if (p[2]) {
+		prefix = 16;
+		v = p[2];
+	} else if (p[1]) {
+		prefix = 8;
+		v = p[1];
+	} else {
+		prefix = 0;
+		v = p[0];
 	}
 
-	if (p < end) {
-		guchar v = *p;
-
 	while (v) {
 		prefix++;
 		v <<= 1;
 	}
-	}
 
 	return prefix;
 }
@@ -1092,16 +1095,7 @@ nm_utils_ip4_netmask_to_prefix (guint32 netmask)
 guint32
 nm_utils_ip4_prefix_to_netmask (guint32 prefix)
 {
-	guint32 msk = 0x80000000;
-	guint32 netmask = 0;
-
-	while (prefix > 0) {
-		netmask |= msk;
-		msk >>= 1;
-		prefix--;
-	}
-
-	return (guint32) htonl (netmask);
+	return prefix < 32 ? ~htonl(0xFFFFFFFF >> prefix) : 0xFFFFFFFF;
 }
 
 

libnm-util/tests/test-general.c

@@ -1731,6 +1731,60 @@ test_connection_changed_cb (NMConnection *connection, gboolean *data)
 	*data = TRUE;
 }
 
+static void
+test_ip4_prefix_to_netmask (void)
+{
+	int i;
+
+	for (i = 0; i<=32; i++) {
+		guint32 netmask = nm_utils_ip4_prefix_to_netmask (i);
+		int plen = nm_utils_ip4_netmask_to_prefix (netmask);
+
+		g_assert_cmpint (i, ==, plen);
+		{
+			guint32 msk = 0x80000000;
+			guint32 netmask2 = 0;
+			guint32 prefix = i;
+			while (prefix > 0) {
+				netmask2 |= msk;
+				msk >>= 1;
+				prefix--;
+			}
+			g_assert_cmpint (netmask, ==, (guint32) htonl (netmask2));
+		}
+	}
+}
+
+static void
+test_ip4_netmask_to_prefix (void)
+{
+	int i, j;
+
+	GRand *rand = g_rand_new ();
+
+	g_rand_set_seed (rand, 1);
+
+	for (i = 2; i<=32; i++) {
+		guint32 netmask = nm_utils_ip4_prefix_to_netmask (i);
+		guint32 netmask_lowest_bit = netmask & ~nm_utils_ip4_prefix_to_netmask (i-1);
+
+		g_assert_cmpint (i, ==, nm_utils_ip4_netmask_to_prefix (netmask));
+
+		for (j = 0; j < 20; j++) {
+			guint32 r = g_rand_int (rand);
+			guint32 netmask_holey;
+
+			netmask_holey = (netmask & r) | netmask_lowest_bit;
+
+			/* create an invalid netmask with holes and check that
+			 * the function does something resonable. */
+			g_assert_cmpint (i, ==, nm_utils_ip4_netmask_to_prefix (netmask_holey));
+		}
+	}
+
+	g_rand_free (rand);
+}
+
 #define ASSERT_CHANGED(statement) \
 { \
 	changed = FALSE; \
@@ -2218,6 +2272,8 @@ int main (int argc, char **argv)
 	test_hwaddr_aton_ib_normal ();
 	test_hwaddr_aton_no_leading_zeros ();
 	test_hwaddr_aton_malformed ();
+	test_ip4_prefix_to_netmask ();
+	test_ip4_netmask_to_prefix ();
 
 	test_connection_changed_signal ();
 	test_setting_connection_changed_signal ();

src/NetworkManagerUtils.c

@@ -100,60 +100,6 @@ nm_spawn_process (const char *args)
 	return status;
 }
 
-/*
- * nm_utils_ip4_netmask_to_prefix
- *
- * Figure out the network prefix from a netmask.  Netmask
- * MUST be in network byte order.
- *
- */
-guint32
-nm_utils_ip4_netmask_to_prefix (guint32 netmask)
-{
-	guchar *p, *end;
-	guint32 prefix = 0;
-
-	p = (guchar *) &netmask;
-	end = p + sizeof (guint32);
-
-	while ((*p == 0xFF) && p < end) {
-		prefix += 8;
-		p++;
-	}
-
-	if (p < end) {
-		guchar v = *p;
-
-		while (v) {
-			prefix++;
-			v <<= 1;
-		}
-	}
-
-	return prefix;
-}
-
-/*
- * nm_utils_ip4_prefix_to_netmask
- *
- * Figure out the netmask from a prefix.
- *
- */
-guint32
-nm_utils_ip4_prefix_to_netmask (guint32 prefix)
-{
-	guint32 msk = 0x80000000;
-	guint32 netmask = 0;
-
-	while (prefix > 0) {
-		netmask |= msk;
-		msk >>= 1;
-		prefix--;
-	}
-
-	return (guint32) htonl (netmask);
-}
-
 gboolean
 nm_match_spec_string (const GSList *specs, const char *match)
 {