netdev: use new ni_netdev_index_to_name utility

Avoid direct if_nametoindex and also if_nametoindex calls
using the ni_netdev_name_to_index utility where possible
to prepare for (net/if*.h) header file inclusion cleanup.

client/ethtool.c

@@ -1271,7 +1271,7 @@ ni_do_ethtool(const char *caller, int argc, char **argv)
 	}
 
 	status = NI_WICKED_RC_ERROR;
-	ni_netdev_ref_init(&ref, argv[optind], if_nametoindex(argv[optind]));
+	ni_netdev_ref_init(&ref, argv[optind], ni_netdev_name_to_index(argv[optind]));
 	if (!ref.index) {
 		fprintf(stderr, "%s: cannot find interface with name '%s'\n", argv[0], argv[optind]);
 		goto cleanup;

client/read-config.c

@@ -295,7 +295,7 @@ ni_ifconfig_read(xml_document_array_t *array, const char *root, const char *path
 }
 
 static ni_config_origin_prio_t
-__ni_ifconfig_origin_get_prio(const char *origin)
+ni_ifconfig_origin_get_prio(const char *origin)
 {
 	ni_config_origin_prio_t prio;
 
@@ -316,66 +316,61 @@ __ni_ifconfig_origin_get_prio(const char *origin)
 
 /*
  * Parse name node and its namespace from xml config.
- * Set ifname if available - it should be at least IF_NAMESIZE bytes long.
  *
- * Return ifindex value or 0 if not available.
+ * Return true when inteface name is available/can be resolved.
  */
-static char *
-__ifconfig_read_get_ifname(xml_node_t *ifnode, unsigned int *ifindex)
+static ni_bool_t
+ni_ifconfig_read_get_ifname(xml_node_t *ifnode, char **ifname)
 {
 	xml_node_t *nnode = NULL;
 	const char *namespace;
-	char *ifname = NULL;
+
+	if (!ifnode || !ifname)
+		return FALSE;
 
 	/* Check for   <name> node */
 	nnode = xml_node_get_child(ifnode, "name");
 	if (!nnode || ni_string_empty(nnode->cdata)) {
-		ni_debug_ifconfig("cannot get interface name - "
+		ni_error("cannot get interface name - "
 			"config has no valid <name> node");
-		goto error;
+		return FALSE;
 	}
 
-	ifname = nnode->cdata;
-
 	/* Resolve a namespace if specified */
 	namespace = xml_node_get_attr(nnode, "namespace");
 	if (ni_string_empty(namespace)) {
-		if (ifindex)
-			*ifindex = if_nametoindex(ifname);
+		if (!ni_netdev_name_is_valid(nnode->cdata)) {
+			ni_error("invalid interface name node data: '%s'",
+					nnode->cdata);
+			return FALSE;
+		}
+		if (!ni_string_dup(ifname, nnode->cdata)) {
+			ni_error("unable to allocate interface name: %m");
+			return FALSE;
+		}
 	}
 	else if (ni_string_eq(namespace, "ifindex")) {
 		unsigned int value;
-		char name_buf[IF_NAMESIZE+1];
 
-		if (ni_parse_uint(ifname, &value, 10) < 0) {
-			ni_debug_ifconfig("unable to parse ifindex value "
+		if (ni_parse_uint(nnode->cdata, &value, 10) < 0) {
+			ni_error("unable to parse ifindex value"
 				" specified via <name namespace=\"ifindex\">");
-			goto error;
+			return FALSE;
 		}
-
-		/* Get ifname based on ifindex */
-		if (ni_string_empty(if_indextoname(value, name_buf))) {
-			ni_debug_ifconfig("unable to obtain interface name "
-				"using ifindex value");
-			goto error;
+		if (!ni_netdev_index_to_name(ifname, value)) {
+			ni_error("unable to obtain interface name"
+				" using ifindex value %u", value);
+			return FALSE;
 		}
-
-		ifname = NULL;
-		ni_string_dup(&ifname, name_buf);
-
-		if (ifindex)
-			*ifindex = value;
 	}
 	else {
 		/* TODO: Implement other namespaces */;
+		ni_error("unable to resolve interace name from namespace '%s'",
+			namespace);
+		return FALSE;
 	}
 
-	return ifname;
-
-error:
-	if (ifindex)
-		*ifindex = 0;
-	return NULL;
+	return TRUE;
 }
 
 ni_bool_t
@@ -384,7 +379,7 @@ ni_ifconfig_validate_adding_doc(xml_document_t *config_doc, ni_bool_t check_prio
 	static ni_var_array_t validated_cfgs; /* Array of already processed configs */
 	ni_config_origin_prio_t src_prio, dst_prio;
 	xml_node_t *src_root, *src_child;
-	char *ifname;
+	char *ifname = NULL;
 
 	if (!config_doc)
 		return FALSE;
@@ -393,7 +388,7 @@ ni_ifconfig_validate_adding_doc(xml_document_t *config_doc, ni_bool_t check_prio
 		return TRUE;
 
 	src_root = xml_document_root(config_doc);
-	src_prio = __ni_ifconfig_origin_get_prio(xml_node_location_filename(src_root));
+	src_prio = ni_ifconfig_origin_get_prio(xml_node_location_filename(src_root));
 
 	/* Go through all config_doc's <interfaces> */
 	for (src_child = src_root->children; src_child; src_child = src_child->next) {
@@ -404,24 +399,27 @@ ni_ifconfig_validate_adding_doc(xml_document_t *config_doc, ni_bool_t check_prio
 			continue;
 		}
 
-		ifname = __ifconfig_read_get_ifname(src_child, NULL);
-		if (ni_string_empty(ifname))
-			return FALSE;
+		if (!ni_ifconfig_read_get_ifname(src_child, &ifname))
+			goto cleanup;
 
 		rv = ni_var_array_get_uint(&validated_cfgs, ifname, &dst_prio);
 		if (rv < 0)
-			return FALSE;
+			goto cleanup;
 
 		if (rv && dst_prio < src_prio) {
 			ni_warn("Ignoring %s config %s because of higher prio config",
 				ifname, xml_node_location_filename(src_root));
-			return FALSE;
+			goto cleanup;
 		}
 
 		ni_var_array_set_uint(&validated_cfgs, ifname, src_prio);
 	}
 
+	ni_string_free(&ifname);
 	return TRUE;
+cleanup:
+	ni_string_free(&ifname);
+	return FALSE;
 }
 
 /*

include/wicked/netinfo.h

@@ -178,6 +178,7 @@ extern ni_netdev_t *	ni_netdev_by_hwaddr(ni_netconfig_t *nic, const ni_hwaddr_t
 extern ni_netdev_t *	ni_netdev_by_vlan_name_and_tag(ni_netconfig_t *nc,
 				const char *physdev, uint16_t tag);
 extern unsigned int	ni_netdev_name_to_index(const char *);
+extern const char *	ni_netdev_index_to_name(char **, unsigned int);
 extern const char *	ni_netdev_make_name(ni_netconfig_t *, const char *, unsigned int);
 
 extern ni_netdev_t *	ni_netdev_new(const char *name, unsigned int ifindex);

src/ifconfig.c

@@ -5884,7 +5884,7 @@ __ni_system_netdev_create(ni_netconfig_t *nc,
 		return -1;
 	}
 
-	if (!ifindex && !(ifindex = if_nametoindex(ifname))) {
+	if (!ifindex && !(ifindex = ni_netdev_name_to_index(ifname))) {
 		ni_error("%s: created %s interface, but can't find it's index",
 				ifname, type);
 		return -1;

src/ifevent.c

@@ -266,7 +266,6 @@ __ni_netdev_process_events(ni_netconfig_t *nc, ni_netdev_t *dev, unsigned int ol
 int
 __ni_rtevent_newlink(ni_netconfig_t *nc, const struct sockaddr_nl *nladdr, struct nlmsghdr *h)
 {
-	char namebuf[IF_NAMESIZE+1] = {'\0'};
 	ni_netdev_t *dev, *old;
 	struct ifinfomsg *ifi;
 	struct nlattr *nla;
@@ -280,7 +279,7 @@ __ni_rtevent_newlink(ni_netconfig_t *nc, const struct sockaddr_nl *nladdr, struc
 		return 0;
 
 	old = ni_netdev_by_index(nc, ifi->ifi_index);
-	ifname = if_indextoname(ifi->ifi_index, namebuf);
+	ni_netdev_index_to_name(&ifname, ifi->ifi_index);
 	if (!ifname) {
 		/*
 		 * device (index) does not exists any more;
@@ -307,25 +306,28 @@ __ni_rtevent_newlink(ni_netconfig_t *nc, const struct sockaddr_nl *nladdr, struc
 		}
 		dev = old;
 		old_flags = old->link.ifflags;
+		ni_string_free(&ifname);
 	} else {
 		if (!(dev = ni_netdev_new(ifname, ifi->ifi_index))) {
 			ni_warn("%s[%u]: unable to allocate memory for device",
 					ifname, ifi->ifi_index);
+			ni_string_free(&ifname);
 			return -1;
 		}
 		dev->created = 1;
 		ni_netconfig_device_append(nc, dev);
+		ni_string_free(&ifname);
 	}
 
 	if (__ni_netdev_process_newlink(dev, h, ifi, nc) < 0) {
-		ni_error("Problem parsing RTM_NEWLINK message for %s", ifname);
+		ni_error("Problem parsing RTM_NEWLINK message for %s", dev->name);
 		return -1;
 	}
 
-	if ((ifname = dev->name)) {
+	if (dev->name) {
 		ni_netdev_t *conflict;
 
-		conflict = ni_netdev_by_name(nc, ifname);
+		conflict = ni_netdev_by_name(nc, dev->name);
 		if (conflict && conflict->link.ifindex != (unsigned int)ifi->ifi_index) {
 			/*
 			 * As the events often provide an already obsolete name [2 events,
@@ -340,9 +342,7 @@ __ni_rtevent_newlink(ni_netconfig_t *nc, const struct sockaddr_nl *nladdr, struc
 			 * Just update the name of the conflicting device in advance too
 			 * and when the interface does not exist any more, emit events.
 			 */
-			char *current = if_indextoname(conflict->link.ifindex, namebuf);
-			if (current) {
-				ni_string_dup(&conflict->name, current);
+			if (ni_netdev_index_to_name(&conflict->name, conflict->link.ifindex)) {
 				__ni_netdev_event(nc, conflict, NI_EVENT_DEVICE_RENAME);
 			} else {
 				unsigned int ifflags = conflict->link.ifflags;

src/iflist.c

@@ -1102,22 +1102,26 @@ __ni_process_ifinfomsg_ovs_type(ni_iftype_t *type, const char *ifname, ni_netcon
 }
 
 static ni_bool_t
-ni_is_wireless(const char *ifname, unsigned int ifindex)
+ni_netdev_is_wireless(const char *ifname, unsigned int ifindex)
 {
-	char ifname_tmp[IF_NAMESIZE];
+	char *name = NULL;
+	ni_bool_t ret;
 
 	/* rtnetlink does not tell us if the device has a
 	 * wireless extensions or not, but sysfs does. */
-	if ( ni_sysfs_netif_exists(ifname, "wireless"))
+	if (ni_sysfs_netif_exists(ifname, "wireless"))
 		return TRUE;
 
-	/* There might be a race condition, where the iterface was renamed and the
-	 * directory in sysfs doesn't exists anymore */
-	if (if_indextoname(ifindex, ifname_tmp))
-		if (ni_sysfs_netif_exists(ifname_tmp, "wireless"))
-			return TRUE;
+	/* There might be a race condition, where the iterface was renamed
+	 * and the directory in sysfs doesn't exists anymore... we have to
+	 * either keep type at NI_IFTYPE_UNKNOWN until next (rename) event
+	 * arrives or try again with the new name here and now */
+	if (!ni_netdev_index_to_name(&name, ifindex))
+		return FALSE;
 
-	return FALSE;
+	ret = ni_sysfs_netif_exists(name, "wireless");
+	ni_string_free(&name);
+	return ret;
 }
 
 static void
@@ -1157,7 +1161,7 @@ __ni_process_ifinfomsg_linktype(ni_linkinfo_t *link, const char *ifname, ni_netc
 			/* We're at the very least an ethernet. */
 			tmp_link_type = NI_IFTYPE_ETHERNET;
 
-			if (ni_is_wireless(ifname, link->ifindex))
+			if (ni_netdev_is_wireless(ifname, link->ifindex))
 				tmp_link_type = NI_IFTYPE_WIRELESS;
 
 			memset(&drv_info, 0, sizeof(drv_info));
@@ -3511,7 +3515,7 @@ __ni_discover_bridge(ni_netdev_t *dev)
 		unsigned int index;
 		ni_bridge_port_t *port;
 
-		if ((index = if_nametoindex(ifname)) == 0) {
+		if ((index = ni_netdev_name_to_index(ifname)) == 0) {
 			/* Looks like someone is renaming interfaces while we're
 			 * trying to discover them :-( */
 			ni_error("%s: port interface %s has index 0?!", __func__, ifname);

src/netinfo.c

@@ -885,6 +885,23 @@ ni_netdev_name_to_index(const char *name)
 	return if_nametoindex(name);
 }
 
+const char *
+ni_netdev_index_to_name(char **ifname, unsigned int ifindex)
+{
+	char ifnamebuf[IFNAMSIZ] = {'\0'};
+
+	if (!ifname || !ifindex)
+		return NULL;
+
+	if (!if_indextoname(ifindex, ifnamebuf))
+		return NULL;
+
+	if (!ni_string_dup(ifname, ifnamebuf))
+		return NULL;
+
+	return *ifname;
+}
+
 /*
  * Create a unique interface name
  */

src/udev-utils.c

@@ -302,25 +302,6 @@ netdev_uinfo_ready(const ni_var_array_t *vars, const char *ifname, unsigned int
 	return -1;
 }
 
-static int
-ni_udev_netdev_update_name(ni_netdev_t *dev)
-{
-	char ifnamebuf[IF_NAMESIZE+1] = {'\0'};
-	const char *ifname;
-
-	if (!dev || !dev->link.ifindex)
-		return -1;
-
-	ifname = if_indextoname(dev->link.ifindex, ifnamebuf);
-	if (ni_string_empty(ifname))
-		return -1; /* device seems to be gone */
-
-	if (!ni_string_eq(dev->name, ifname))
-		ni_string_dup(&dev->name, ifname);
-
-	return 0;
-}
-
 ni_bool_t
 ni_udev_netdev_is_ready(ni_netdev_t *dev)
 {
@@ -334,7 +315,7 @@ ni_udev_netdev_is_ready(ni_netdev_t *dev)
 		 * start to receive, that is the device ifname may
 		 * be obsolete in the meantime due to udev renames.
 		 */
-		if (ni_udev_netdev_update_name(dev) < 0)
+		if (!ni_netdev_index_to_name(&dev->name, dev->link.ifindex))
 			return FALSE;
 
 		snprintf(pathbuf, sizeof(pathbuf), "%s/%s",

src/uevent.c

@@ -659,21 +659,16 @@ __ni_uevent_ifevent_forwarder(const ni_var_array_t *vars, void *user_data)
 
 	if (dev && !(dev->link.ifflags & NI_IFF_DEVICE_READY)) {
 		unsigned int old_flags = dev->link.ifflags;
-		char namebuf[IF_NAMESIZE+1] = {'\0'};
-		const char *ifname;
 
 		if (!ni_string_empty(uinfo.interface_old))
 			return;
 
 		if (!uinfo.tags || !strstr(uinfo.tags, ":systemd:"))
 			return;
 
-		if (!(ifname = if_indextoname(dev->link.ifindex, namebuf)))
+		if (!ni_netdev_index_to_name(&dev->name, dev->link.ifindex))
 			return; /* device gone in the meantime */
 
-		if (!ni_string_eq(dev->name, ifname))
-			ni_string_dup(&dev->name, ifname);
-
 		dev->link.ifflags |= NI_IFF_DEVICE_READY;
 		__ni_netdev_process_events(nc, dev, old_flags);
 	}