platform: refactor nmp_utils_sysctl_open_netdir()

- use nm_auto_close cleanup attribute
- optionally, return the found ifname
- don't stat "phy80211". If such an entity can be opened,
  just assume it's a directory.

src/platform/nm-platform-utils.c

@@ -38,9 +38,12 @@
 
 #include "nm-core-utils.h"
 
+extern char *if_indextoname (unsigned int __ifindex, char *__ifname);
+
 /******************************************************************
  * ethtool
  ******************************************************************/
+
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,27)
 #define ethtool_cmd_speed(pedata) ((pedata)->speed)
 
@@ -48,8 +51,6 @@
 	G_STMT_START { (pedata)->speed = (guint16) (speed); } G_STMT_END
 #endif
 
-extern char *if_indextoname (unsigned int __ifindex, char *__ifname);
-
 static gboolean
 ethtool_get (const char *name, gpointer edata)
 {
@@ -625,51 +626,83 @@ nmp_utils_ip_config_source_to_string (NMIPConfigSource source, char *buf, gsize
 	return buf;
 }
 
+/**
+ * nmp_utils_sysctl_open_netdir:
+ * @ifindex: the ifindex for which to open "/sys/class/net/%s"
+ * @ifname_guess: (allow-none): optional argument, if present used as initial
+ *   guess as the current name for @ifindex. If guessed right,
+ *   it saves an addtional if_indextoname() call.
+ * @out_ifname: (allow-none): if present, must be at least IFNAMSIZ
+ *   characters. On success, this will contain the actual ifname
+ *   found while opening the directory.
+ *
+ * Returns: a negative value on failure, on success returns the open fd
+ *   to the "/sys/class/net/%s" directory for @ifindex.
+ */
 int
-nmp_utils_open_sysctl(int ifindex, const char *ifname)
+nmp_utils_sysctl_open_netdir (int ifindex,
+                              const char *ifname_guess,
+                              char *out_ifname)
 {
 	#define SYS_CLASS_NET "/sys/class/net/"
+	const char *ifname = ifname_guess;
+	char ifname_buf_last_try[IFNAMSIZ];
 	char ifname_buf[IFNAMSIZ];
 	guint try_count = 0;
-	char sysdir[NM_STRLEN (SYS_CLASS_NET) + IFNAMSIZ + 1] = SYS_CLASS_NET;
+	char sysdir[NM_STRLEN (SYS_CLASS_NET) + IFNAMSIZ] = SYS_CLASS_NET;
 	char fd_buf[256];
-	int fd;
-	int fd_ifindex;
 	ssize_t nn;
 
-	while (++try_count < 4) {
+	g_return_val_if_fail (ifindex >= 0, -1);
+
+	ifname_buf_last_try[0] = '\0';
+
+	for (try_count = 0; try_count < 10; try_count++, ifname = NULL) {
+		nm_auto_close int fd_dir = -1;
+		nm_auto_close int fd_ifindex = -1;
+		int fd;
+
 		if (!ifname) {
 			ifname = if_indextoname (ifindex, ifname_buf);
 			if (!ifname)
 				return -1;
 		}
 
-		nm_utils_ifname_cpy (&sysdir[NM_STRLEN (SYS_CLASS_NET)], ifname);
+		nm_assert (nm_utils_iface_valid_name (ifname));
-		fd = open (sysdir, O_DIRECTORY);
-		if (fd < 0)
-			goto next;
-		fd_ifindex = openat (fd, "ifindex", 0);
-		if (fd_ifindex < 0) {
-			close (fd);
-			goto next;
-		}
-		/* read ifindex file, and compare it to @ifindex. If match, return fd. */
-		nn = nm_utils_fd_read_loop (fd_ifindex, fd_buf, sizeof (fd_buf) - 1, FALSE);
-		if (nn < 0) {
-			close (fd);
-			close (fd_ifindex);
-			goto next;
-		}
-		fd_buf[sizeof (fd_buf) - 1] = '\0';
 
-		if (ifindex != _nm_utils_ascii_str_to_int64 (fd_buf, 10, 1, G_MAXINT, -1)) {
+		if (g_strlcpy (&sysdir[NM_STRLEN (SYS_CLASS_NET)], ifname, IFNAMSIZ) >= IFNAMSIZ)
-			close (fd);
+			g_return_val_if_reached (-1);
-			close (fd_ifindex);
+
-			goto next;
+		/* we only retry, if the name changed since previous attempt.
-		}
+		 * Hence, it is extremely unlikely that this loop runes until the
+		 * end of the @try_count. */
+		if (nm_streq (ifname, ifname_buf_last_try))
+			return -1;
+		strcpy (ifname_buf_last_try, ifname);
+
+		fd_dir = open (sysdir, O_DIRECTORY | O_CLOEXEC);
+		if (fd_dir < 0)
+			continue;
+
+		fd_ifindex = openat (fd_dir, "ifindex", O_CLOEXEC);
+		if (fd_ifindex < 0)
+			continue;
+
+		nn = nm_utils_fd_read_loop (fd_ifindex, fd_buf, sizeof (fd_buf) - 2, FALSE);
+		if (nn <= 0)
+			continue;
+		fd_buf[nn] = '\0';
+
+		if (ifindex != _nm_utils_ascii_str_to_int64 (fd_buf, 10, 1, G_MAXINT, -1))
+			continue;
+
+		if (out_ifname)
+			strcpy (out_ifname, ifname);
+
+		fd = fd_dir;
+		fd_dir = -1;
 		return fd;
-next:
-		ifname = NULL;
 	}
+
 	return -1;
 }

src/platform/nm-platform-utils.h

@@ -61,6 +61,8 @@ NMIPConfigSource nmp_utils_ip_config_source_coerce_from_rtprot (NMIPConfigSource
 NMIPConfigSource nmp_utils_ip_config_source_round_trip_rtprot  (NMIPConfigSource source) _nm_const;
 const char *     nmp_utils_ip_config_source_to_string (NMIPConfigSource source, char *buf, gsize len);
 
-int nmp_utils_open_sysctl(int ifindex, const char *ifname);
+int nmp_utils_sysctl_open_netdir (int ifindex,
+                                  const char *ifname_guess,
+                                  char *out_ifname);
 
 #endif /* __NM_PLATFORM_UTILS_H__ */

src/platform/wifi/wifi-utils.c

@@ -187,29 +187,26 @@ wifi_utils_is_wifi (int ifindex, const char *ifname)
 {
 	int fd_sysnet;
 	int fd_phy80211;
-	struct stat s;
+	char ifname_verified[IFNAMSIZ];
 
-	g_return_val_if_fail (ifname != NULL, FALSE);
+	g_return_val_if_fail (ifindex > 0, FALSE);
 
-	fd_sysnet = nmp_utils_open_sysctl (ifindex, ifname);
+	fd_sysnet = nmp_utils_sysctl_open_netdir (ifindex, ifname, ifname_verified);
 	if (fd_sysnet < 0)
 		return FALSE;
 
-	fd_phy80211 = openat (fd_sysnet, "phy80211", 0);
+	/* there might have been a race and ifname might be wrong. Below for checking
-	if (fd_phy80211 < 0) {
+	 * wext, use the possibly improved name that we just verified. */
-		close (fd_sysnet);
+	ifname = ifname_verified;
-		return FALSE;
-	}
 
-	if ((fstat (fd_phy80211, &s) == 0 && (s.st_mode & S_IFDIR))) {
+	fd_phy80211 = openat (fd_sysnet, "phy80211", O_CLOEXEC);
-		close (fd_sysnet);
+	close (fd_sysnet);
+
+	if (fd_phy80211 >= 0) {
 		close (fd_phy80211);
 		return TRUE;
 	}
 
-	close (fd_sysnet);
-	close (fd_phy80211);
-
 #if HAVE_WEXT
 	if (wifi_wext_is_wifi (ifname))
 		return TRUE;