libnm-core, etc: drop type-based hwaddr funcs, port to length-based ones

Drop the arptype-based nm_utils_hwaddr funcs, and rename the
length-based ones to no longer have _len in their names. This also
switches nm_utils_hwaddr_atoba() to using a length rather than an
arptype, and adds a length argument to nm_utils_hwaddr_valid() (making
nm_utils_hwaddr_valid() now a replacement for nm_utils_hwaddr_aton()
in some places, where we were only using aton() to do validity
checking).

5 files changed, 66 insertions, 109 deletions

diff --git a/libnm-core/nm-setting-wired.c b/libnm-core/nm-setting-wired.c
index 1147c7faec..2b0e31406a 100644
--- a/libnm-core/nm-setting-wired.c
+++ b/libnm-core/nm-setting-wired.c
@@ -271,12 +271,11 @@ nm_setting_wired_add_mac_blacklist_item (NMSettingWired *setting, const char *ma
 {
 	NMSettingWiredPrivate *priv;
 	GSList *iter;
-	guint8 buf[32];
 
 	g_return_val_if_fail (NM_IS_SETTING_WIRED (setting), FALSE);
 	g_return_val_if_fail (mac != NULL, FALSE);
 
-	if (!nm_utils_hwaddr_aton (mac, ARPHRD_ETHER, buf))
+	if (!nm_utils_hwaddr_valid (mac, ETH_ALEN))
 		return FALSE;
 
 	priv = NM_SETTING_WIRED_GET_PRIVATE (setting);
@@ -330,12 +329,11 @@ nm_setting_wired_remove_mac_blacklist_item_by_value (NMSettingWired *setting, co
 {
 	NMSettingWiredPrivate *priv;
 	GSList *iter;
-	guint8 buf[32];
 
 	g_return_val_if_fail (NM_IS_SETTING_WIRED (setting), FALSE);
 	g_return_val_if_fail (mac != NULL, FALSE);
 
-	if (!nm_utils_hwaddr_aton (mac, ARPHRD_ETHER, buf))
+	if (!nm_utils_hwaddr_valid (mac, ETH_ALEN))
 		return FALSE;
 
 	priv = NM_SETTING_WIRED_GET_PRIVATE (setting);
diff --git a/libnm-core/nm-setting-wireless.c b/libnm-core/nm-setting-wireless.c
index ce788dc800..7a4015e0a6 100644
--- a/libnm-core/nm-setting-wireless.c
+++ b/libnm-core/nm-setting-wireless.c
@@ -507,12 +507,11 @@ nm_setting_wireless_add_mac_blacklist_item (NMSettingWireless *setting, const ch
 {
 	NMSettingWirelessPrivate *priv;
 	GSList *iter;
-	guint8 buf[32];
 
 	g_return_val_if_fail (NM_IS_SETTING_WIRELESS (setting), FALSE);
 	g_return_val_if_fail (mac != NULL, FALSE);
 
-	if (!nm_utils_hwaddr_aton (mac, ARPHRD_ETHER, buf))
+	if (!nm_utils_hwaddr_valid (mac, ETH_ALEN))
 		return FALSE;
 
 	priv = NM_SETTING_WIRELESS_GET_PRIVATE (setting);
@@ -566,12 +565,11 @@ nm_setting_wireless_remove_mac_blacklist_item_by_value (NMSettingWireless *setti
 {
 	NMSettingWirelessPrivate *priv;
 	GSList *iter;
-	guint8 buf[32];
 
 	g_return_val_if_fail (NM_IS_SETTING_WIRELESS (setting), FALSE);
 	g_return_val_if_fail (mac != NULL, FALSE);
 
-	if (!nm_utils_hwaddr_aton (mac, ARPHRD_ETHER, buf))
+	if (!nm_utils_hwaddr_valid (mac, ETH_ALEN))
 		return FALSE;
 
 	priv = NM_SETTING_WIRELESS_GET_PRIVATE (setting);
diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c
index 4c3a89f744..649942ade7 100644
--- a/libnm-core/nm-utils.c
+++ b/libnm-core/nm-utils.c
@@ -1930,52 +1930,30 @@ nm_utils_wifi_is_channel_valid (guint32 channel, const char *band)
  *
  * Returns the length in octets of a hardware address of type @type.
  *
- * Return value: the positive length, or -1 if the type is unknown/unsupported.
+ * It is an error to call this function with any value other than %ARPHRD_ETHER
+ * or %ARPHRD_INFINIBAND.
+ *
+ * Return value: the length.
  */
-int
+gsize
 nm_utils_hwaddr_len (int type)
 {
+	g_return_val_if_fail (type == ARPHRD_ETHER || type == ARPHRD_INFINIBAND, 0);
+
 	if (type == ARPHRD_ETHER)
 		return ETH_ALEN;
 	else if (type == ARPHRD_INFINIBAND)
 		return INFINIBAND_ALEN;
-	else
-		return -1;
+
+	g_assert_not_reached ();
 }
 
 #define HEXVAL(c) ((c) <= '9' ? (c) - '0' : ((c) & 0x4F) - 'A' + 10)
 
 /**
- * nm_utils_hwaddr_aton:
- * @asc: the ASCII representation of a hardware address
- * @type: the type of address; either %ARPHRD_ETHER or %ARPHRD_INFINIBAND
- * @buffer: buffer to store the result into
- *
- * Parses @asc and converts it to binary form in @buffer. See
- * nm_utils_hwaddr_atoba() if you'd rather have the result in a
- * #GByteArray.
- *
- * See also nm_utils_hwaddr_aton_len(), which takes an output length
- * instead of a type.
- *
- * Return value: @buffer, or %NULL if @asc couldn't be parsed
- */
-guint8 *
-nm_utils_hwaddr_aton (const char *asc, int type, gpointer buffer)
-{
-	int len = nm_utils_hwaddr_len (type);
-
-	if (len <= 0) {
-		g_return_val_if_reached (NULL);
-		return NULL;
-	}
-	return nm_utils_hwaddr_aton_len (asc, buffer, len);
-}
-
-/**
  * nm_utils_hwaddr_atoba:
  * @asc: the ASCII representation of a hardware address
- * @type: the type of address; either %ARPHRD_ETHER or %ARPHRD_INFINIBAND
+ * @length: the expected length in bytes of the result
  *
  * Parses @asc and converts it to binary form in a #GByteArray. See
  * nm_utils_hwaddr_aton() if you don't want a #GByteArray.
@@ -1984,19 +1962,16 @@ nm_utils_hwaddr_aton (const char *asc, int type, gpointer buffer)
  * be parsed
  */
 GByteArray *
-nm_utils_hwaddr_atoba (const char *asc, int type)
+nm_utils_hwaddr_atoba (const char *asc, gsize length)
 {
 	GByteArray *ba;
-	int len = nm_utils_hwaddr_len (type);
 
-	if (len <= 0) {
-		g_return_val_if_reached (NULL);
-		return NULL;
-	}
+	g_return_val_if_fail (asc != NULL, NULL);
+	g_return_val_if_fail (length > 0 && length <= NM_UTILS_HWADDR_LEN_MAX, NULL);
 
-	ba = g_byte_array_sized_new (len);
-	g_byte_array_set_size (ba, len);
-	if (!nm_utils_hwaddr_aton_len (asc, ba->data, len)) {
+	ba = g_byte_array_sized_new (length);
+	g_byte_array_set_size (ba, length);
+	if (!nm_utils_hwaddr_aton (asc, ba->data, length)) {
 		g_byte_array_unref (ba);
 		return NULL;
 	}
@@ -2005,32 +1980,7 @@ nm_utils_hwaddr_atoba (const char *asc, int type)
 }
 
 /**
- * nm_utils_hwaddr_ntoa:
- * @addr: a binary hardware address
- * @type: the type of address; either %ARPHRD_ETHER or %ARPHRD_INFINIBAND
- *
- * Converts @addr to textual form.
- *
- * See also nm_utils_hwaddr_ntoa_len(), which takes a length instead of
- * a type.
- *
- * Return value: (transfer full): the textual form of @addr
- */
-char *
-nm_utils_hwaddr_ntoa (gconstpointer addr, int type)
-{
-	int len = nm_utils_hwaddr_len (type);
-
-	if (len <= 0) {
-		g_return_val_if_reached (NULL);
-		return NULL;
-	}
-
-	return nm_utils_hwaddr_ntoa_len (addr, len);
-}
-
-/**
- * nm_utils_hwaddr_aton_len:
+ * nm_utils_hwaddr_aton:
  * @asc: the ASCII representation of a hardware address
  * @buffer: buffer to store the result into
  * @length: the expected length in bytes of the result and
@@ -2043,18 +1993,15 @@ nm_utils_hwaddr_ntoa (gconstpointer addr, int type)
  *   or would be shorter or longer than @length.
  */
 guint8 *
-nm_utils_hwaddr_aton_len (const char *asc, gpointer buffer, gsize length)
+nm_utils_hwaddr_aton (const char *asc, gpointer buffer, gsize length)
 {
 	const char *in = asc;
 	guint8 *out = (guint8 *)buffer;
 	char delimiter = '\0';
 
-	if (!asc) {
-		g_return_val_if_reached (NULL);
-		return NULL;
-	}
-	g_return_val_if_fail (buffer, NULL);
-	g_return_val_if_fail (length, NULL);
+	g_return_val_if_fail (asc != NULL, NULL);
+	g_return_val_if_fail (buffer != NULL, NULL);
+	g_return_val_if_fail (length > 0 && length <= NM_UTILS_HWADDR_LEN_MAX, NULL);
 
 	while (length && *in) {
 		guint8 d1 = in[0], d2 = in[1];
@@ -2094,7 +2041,7 @@ nm_utils_hwaddr_aton_len (const char *asc, gpointer buffer, gsize length)
 }
 
 /**
- * nm_utils_hwaddr_ntoa_len:
+ * nm_utils_hwaddr_ntoa:
  * @addr: a binary hardware address
  * @length: the length of @addr
  *
@@ -2103,14 +2050,14 @@ nm_utils_hwaddr_aton_len (const char *asc, gpointer buffer, gsize length)
  * Return value: (transfer full): the textual form of @addr
  */
 char *
-nm_utils_hwaddr_ntoa_len (gconstpointer addr, gsize length)
+nm_utils_hwaddr_ntoa (gconstpointer addr, gsize length)
 {
 	const guint8 *in = addr;
 	char *out, *result;
 	const char *LOOKUP = "0123456789ABCDEF";
 
 	g_return_val_if_fail (addr != NULL, g_strdup (""));
-	g_return_val_if_fail (length != 0, g_strdup (""));
+	g_return_val_if_fail (length > 0 && length <= NM_UTILS_HWADDR_LEN_MAX, g_strdup (""));
 
 	result = out = g_malloc (length * 3);
 	for (;;) {
@@ -2126,30 +2073,45 @@ nm_utils_hwaddr_ntoa_len (gconstpointer addr, gsize length)
 	}
 }
 
+static int
+hwaddr_binary_len (const char *asc)
+{
+	int octets = 1;
+
+	for (; *asc; asc++) {
+		if (*asc == ':' || *asc == '-')
+			octets++;
+	}
+	return octets;
+}
+
 /**
  * nm_utils_hwaddr_valid:
  * @asc: the ASCII representation of a hardware address
+ * @length: the length of address that @asc is expected to convert to
+ *   (or -1 to accept any length up to %NM_UTILS_HWADDR_LEN_MAX)
  *
- * Parses @asc to see if it is a valid hardware address of some type.
+ * Parses @asc to see if it is a valid hardware address of the given
+ * length.
  *
  * Return value: %TRUE if @asc appears to be a valid hardware address
- *   of some type, %FALSE if not.
+ *   of the indicated length, %FALSE if not.
  */
 gboolean
-nm_utils_hwaddr_valid (const char *asc)
+nm_utils_hwaddr_valid (const char *asc, gssize length)
 {
 	guint8 buf[NM_UTILS_HWADDR_LEN_MAX];
-	gsize in_len, out_len;
 
-	if (!asc || !*asc)
-		return FALSE;
-	in_len = strlen (asc);
-	if ((in_len + 1) % 3 != 0)
-		return FALSE;
-	out_len = (in_len + 1) / 3;
-	if (out_len > NM_UTILS_HWADDR_LEN_MAX)
-		return FALSE;
-	return nm_utils_hwaddr_aton_len (asc, buf, out_len) != NULL;
+	g_return_val_if_fail (asc != NULL, FALSE);
+	g_return_val_if_fail (length == -1 || (length > 0 && length <= NM_UTILS_HWADDR_LEN_MAX), FALSE);
+
+	if (length == -1) {
+		length = hwaddr_binary_len (asc);
+		if (length == 0 || length > NM_UTILS_HWADDR_LEN_MAX)
+			return FALSE;
+	}
+
+	return nm_utils_hwaddr_aton (asc, buf, length) != NULL;
 }
 
 /**
diff --git a/libnm-core/nm-utils.h b/libnm-core/nm-utils.h
index 4064b9c3f2..ea007cb0d1 100644
--- a/libnm-core/nm-utils.h
+++ b/libnm-core/nm-utils.h
@@ -135,15 +135,13 @@ gboolean nm_utils_wifi_is_channel_valid (guint32 channel, const char *band);
  */
 #define NM_UTILS_HWADDR_LEN_MAX 20 /* INFINIBAND_ALEN */
 
-int         nm_utils_hwaddr_len   (int type) G_GNUC_PURE;
-char       *nm_utils_hwaddr_ntoa  (gconstpointer addr, int type);
-GByteArray *nm_utils_hwaddr_atoba (const char *asc, int type);
-guint8     *nm_utils_hwaddr_aton  (const char *asc, int type, gpointer buffer);
+gsize       nm_utils_hwaddr_len   (int type) G_GNUC_PURE;
 
-char       *nm_utils_hwaddr_ntoa_len  (gconstpointer addr, gsize length);
-guint8     *nm_utils_hwaddr_aton_len  (const char *asc, gpointer buffer, gsize length);
+char       *nm_utils_hwaddr_ntoa  (gconstpointer addr, gsize length);
+GByteArray *nm_utils_hwaddr_atoba (const char *asc, gsize length);
+guint8     *nm_utils_hwaddr_aton  (const char *asc, gpointer buffer, gsize length);
 
-gboolean    nm_utils_hwaddr_valid (const char *asc);
+gboolean    nm_utils_hwaddr_valid (const char *asc, gssize length);
 
 char *nm_utils_bin2hexstr (const char *bytes, int len, int final_len);
 int   nm_utils_hex2byte   (const char *hex);
diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c
index 0ec1488cfa..f281815818 100644
--- a/libnm-core/tests/test-general.c
+++ b/libnm-core/tests/test-general.c
@@ -1910,7 +1910,7 @@ test_hwaddr_aton_ether_normal (void)
 	guint8 buf[100];
 	guint8 expected[ETH_ALEN] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55 };
 
-	g_assert (nm_utils_hwaddr_aton ("00:11:22:33:44:55", ARPHRD_ETHER, buf) != NULL);
+	g_assert (nm_utils_hwaddr_aton ("00:11:22:33:44:55", buf, ETH_ALEN) != NULL);
 	g_assert (memcmp (buf, expected, sizeof (expected)) == 0);
 }
 
@@ -1923,7 +1923,7 @@ test_hwaddr_aton_ib_normal (void)
 		0x77, 0x88, 0x99, 0x01, 0x12, 0x23, 0x34, 0x45, 0x56, 0x67, 0x78, 0x89,
 		0x90 };
 
-	g_assert (nm_utils_hwaddr_aton (source, ARPHRD_INFINIBAND, buf) != NULL);
+	g_assert (nm_utils_hwaddr_aton (source, buf, INFINIBAND_ALEN) != NULL);
 	g_assert (memcmp (buf, expected, sizeof (expected)) == 0);
 }
 
@@ -1933,7 +1933,7 @@ test_hwaddr_aton_no_leading_zeros (void)
 	guint8 buf[100];
 	guint8 expected[ETH_ALEN] = { 0x00, 0x1A, 0x2B, 0x03, 0x44, 0x05 };
 
-	g_assert (nm_utils_hwaddr_aton ("0:1a:2B:3:44:5", ARPHRD_ETHER, buf) != NULL);
+	g_assert (nm_utils_hwaddr_aton ("0:1a:2B:3:44:5", buf, ETH_ALEN) != NULL);
 	g_assert (memcmp (buf, expected, sizeof (expected)) == 0);
 }
 
@@ -1942,7 +1942,7 @@ test_hwaddr_aton_malformed (void)
 {
 	guint8 buf[100];
 
-	g_assert (nm_utils_hwaddr_aton ("0:1a:2B:3:a@%%", ARPHRD_ETHER, buf) == NULL);
+	g_assert (nm_utils_hwaddr_aton ("0:1a:2B:3:a@%%", buf, ETH_ALEN) == NULL);
 }
 
 static void
@@ -2658,6 +2658,7 @@ 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 ();