core: refactor nm_ethernet_address_is_valid()th/wifi-bssid-cleanup

The caller *always* needs to know whether the argument
is an address in binary or text from. At that point,
it's only inconvenient to require the user to either
pass "-1" or ETH_ALEN as size (nothing else was supported
anyway).

Split the function and rename.

5 files changed, 40 insertions, 49 deletions

diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c
index ac6b0f8551..d3bebef216 100644
--- a/src/devices/wifi/nm-device-iwd.c
+++ b/src/devices/wifi/nm-device-iwd.c
@@ -445,8 +445,7 @@ periodic_update(NMDeviceIwd *self)
         _notify(self, PROP_BITRATE);
     }
 
-    if (nm_ethernet_address_is_valid(&bssid, ETH_ALEN)
-        && !nm_ether_addr_equal(&bssid, &priv->current_ap_bssid)) {
+    if (nm_ether_addr_is_valid(&bssid) && !nm_ether_addr_equal(&bssid, &priv->current_ap_bssid)) {
         priv->current_ap_bssid = bssid;
         ap_changed |= nm_wifi_ap_set_address_bin(priv->current_ap, &bssid);
         ap_changed |= nm_wifi_ap_set_freq(priv->current_ap,
diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c
index 3fe434bb2d..d21f379be8 100644
--- a/src/devices/wifi/nm-device-wifi.c
+++ b/src/devices/wifi/nm-device-wifi.c
@@ -3391,7 +3391,7 @@ activation_success_handler(NMDevice *device)
                                                 update_bssid ? &bssid : NULL,
                                                 NULL,
                                                 update_rate ? &rate : NULL)) {
-                if (update_bssid && nm_ethernet_address_is_valid(&bssid, ETH_ALEN))
+                if (update_bssid && nm_ether_addr_is_valid(&bssid))
                     ap_changed |= nm_wifi_ap_set_address_bin(priv->current_ap, &bssid);
                 if (update_rate)
                     ap_changed |= nm_wifi_ap_set_max_bitrate(priv->current_ap, rate);
diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c
index 7adc011e30..1f816f84e3 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -187,51 +187,46 @@ nm_utils_exp10(gint16 ex)
 
 /*****************************************************************************/
 
-/*
- * nm_ethernet_address_is_valid:
- * @addr: pointer to a binary or ASCII Ethernet address
- * @len: length of @addr, or -1 if @addr is ASCII
- *
- * Compares an Ethernet address against known invalid addresses.
-
- * Returns: %TRUE if @addr is a valid Ethernet address, %FALSE if it is not.
- */
 gboolean
-nm_ethernet_address_is_valid(gconstpointer addr, gssize len)
+nm_ether_addr_is_valid(const NMEtherAddr *addr)
 {
-    guint8 invalid_addr[4][ETH_ALEN] = {
+    static const guint8 invalid_addr[][ETH_ALEN] = {
         {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF},
         {0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
         {0x44, 0x44, 0x44, 0x44, 0x44, 0x44},
         {0x00, 0x30, 0xb4, 0x00, 0x00, 0x00}, /* prism54 dummy MAC */
     };
-    guint8 addr_bin[ETH_ALEN];
-    guint  i;
+    int i;
 
-    if (!addr) {
-        g_return_val_if_fail(len == -1 || len == ETH_ALEN, FALSE);
+    if (!addr)
         return FALSE;
-    }
-
-    if (len == -1) {
-        if (!nm_utils_hwaddr_aton(addr, addr_bin, ETH_ALEN))
-            return FALSE;
-        addr = addr_bin;
-    } else if (len != ETH_ALEN)
-        g_return_val_if_reached(FALSE);
 
     /* Check for multicast address */
-    if ((((guint8 *) addr)[0]) & 0x01)
+    if (addr->ether_addr_octet[0] & 0x01u)
         return FALSE;
 
-    for (i = 0; i < G_N_ELEMENTS(invalid_addr); i++) {
-        if (nm_utils_hwaddr_matches(addr, ETH_ALEN, invalid_addr[i], ETH_ALEN))
+    for (i = 0; i < (int) G_N_ELEMENTS(invalid_addr); i++) {
+        if (memcmp(addr, invalid_addr[i], ETH_ALEN) == 0)
             return FALSE;
     }
 
     return TRUE;
 }
 
+gboolean
+nm_ether_addr_is_valid_str(const char *str)
+{
+    NMEtherAddr addr_bin;
+
+    if (!str)
+        return FALSE;
+
+    if (!nm_utils_hwaddr_aton(str, &addr_bin, ETH_ALEN))
+        return FALSE;
+
+    return nm_ether_addr_is_valid(&addr_bin);
+}
+
 /*****************************************************************************/
 
 void
diff --git a/src/nm-core-utils.h b/src/nm-core-utils.h
index ecbf1ebbff..dcc354c753 100644
--- a/src/nm-core-utils.h
+++ b/src/nm-core-utils.h
@@ -95,7 +95,8 @@ void _nm_singleton_instance_register_destruction(GObject *instance);
 
 /*****************************************************************************/
 
-gboolean nm_ethernet_address_is_valid(gconstpointer addr, gssize len);
+gboolean nm_ether_addr_is_valid(const NMEtherAddr *addr);
+gboolean nm_ether_addr_is_valid_str(const char *str);
 
 gconstpointer
 nm_utils_ipx_address_clear_host_address(int family, gpointer dst, gconstpointer src, guint8 plen);
diff --git a/src/tests/test-core-with-expect.c b/src/tests/test-core-with-expect.c
index bc24fb009b..9c77a85d47 100644
--- a/src/tests/test-core-with-expect.c
+++ b/src/tests/test-core-with-expect.c
@@ -562,30 +562,26 @@ test_nm_utils_array_remove_at_indexes(void)
 static void
 test_nm_ethernet_address_is_valid(void)
 {
-    g_assert(!nm_ethernet_address_is_valid(NULL, -1));
-    g_assert(!nm_ethernet_address_is_valid(NULL, ETH_ALEN));
+    g_assert(!nm_ether_addr_is_valid_str(NULL));
+    g_assert(!nm_ether_addr_is_valid(NULL));
 
-    g_assert(!nm_ethernet_address_is_valid("FF:FF:FF:FF:FF:FF", -1));
-    g_assert(!nm_ethernet_address_is_valid("00:00:00:00:00:00", -1));
-    g_assert(!nm_ethernet_address_is_valid("44:44:44:44:44:44", -1));
-    g_assert(!nm_ethernet_address_is_valid("00:30:b4:00:00:00", -1));
+    g_assert(!nm_ether_addr_is_valid_str("FF:FF:FF:FF:FF:FF"));
+    g_assert(!nm_ether_addr_is_valid_str("00:00:00:00:00:00"));
+    g_assert(!nm_ether_addr_is_valid_str("44:44:44:44:44:44"));
+    g_assert(!nm_ether_addr_is_valid_str("00:30:b4:00:00:00"));
 
-    g_assert(!nm_ethernet_address_is_valid("", -1));
-    g_assert(!nm_ethernet_address_is_valid("1", -1));
-    g_assert(!nm_ethernet_address_is_valid("2", -1));
+    g_assert(!nm_ether_addr_is_valid_str(""));
+    g_assert(!nm_ether_addr_is_valid_str("1"));
+    g_assert(!nm_ether_addr_is_valid_str("2"));
 
-    g_assert(
-        !nm_ethernet_address_is_valid(((guint8[8]){0x00, 0x30, 0xb4, 0x00, 0x00, 0x00}), ETH_ALEN));
-    g_assert(
-        nm_ethernet_address_is_valid(((guint8[8]){0x00, 0x30, 0xb4, 0x00, 0x00, 0x01}), ETH_ALEN));
+    g_assert(!nm_ether_addr_is_valid(&NM_ETHER_ADDR_INIT(0x00, 0x30, 0xb4, 0x00, 0x00, 0x00)));
+    g_assert(nm_ether_addr_is_valid(&NM_ETHER_ADDR_INIT(0x00, 0x30, 0xb4, 0x00, 0x00, 0x01)));
 
     /* some Broad cast addresses (with MSB of first octet set). */
-    g_assert(!nm_ethernet_address_is_valid("57:44:44:44:44:44", -1));
-    g_assert(nm_ethernet_address_is_valid("56:44:44:44:44:44", -1));
-    g_assert(
-        !nm_ethernet_address_is_valid(((guint8[8]){0x03, 0x30, 0xb4, 0x00, 0x00, 0x00}), ETH_ALEN));
-    g_assert(
-        nm_ethernet_address_is_valid(((guint8[8]){0x02, 0x30, 0xb4, 0x00, 0x00, 0x01}), ETH_ALEN));
+    g_assert(!nm_ether_addr_is_valid_str("57:44:44:44:44:44"));
+    g_assert(nm_ether_addr_is_valid_str("56:44:44:44:44:44"));
+    g_assert(!nm_ether_addr_is_valid(&NM_ETHER_ADDR_INIT(0x03, 0x30, 0xb4, 0x00, 0x00, 0x00)));
+    g_assert(nm_ether_addr_is_valid(&NM_ETHER_ADDR_INIT(0x02, 0x30, 0xb4, 0x00, 0x00, 0x01)));
 }
 
 /*****************************************************************************/