Merge pull request #468 from mtomaschewski/leaks

fixes for memory and file descriptor leaks

include/wicked/bridge.h

@@ -95,6 +95,7 @@ extern void		ni_bridge_get_port_names(const ni_bridge_t *, ni_string_array_t *);
 extern ni_bridge_port_t *ni_bridge_port_new(ni_bridge_t *br, const char *ifname, unsigned int ifindex);
 extern ni_bridge_port_t *ni_bridge_port_by_index(const ni_bridge_t *br, unsigned int ifindex);
 extern ni_bridge_port_t *ni_bridge_port_by_name(const ni_bridge_t *br, const char *ifname);
+extern ni_bridge_port_t *ni_bridge_port_clone(const ni_bridge_port_t *port);
 extern void		ni_bridge_port_free(ni_bridge_port_t *port);
 
 

include/wicked/system.h

@@ -49,7 +49,7 @@ extern int		ni_system_bridge_create(ni_netconfig_t *, const char *,
 extern int		ni_system_bridge_setup(ni_netconfig_t *, ni_netdev_t *,
 				const ni_bridge_t *);
 extern int		ni_system_bridge_add_port(ni_netconfig_t *, ni_netdev_t *,
-				ni_bridge_port_t *);
+				const ni_bridge_port_t *);
 extern int		ni_system_bridge_remove_port(ni_netdev_t *, unsigned int);
 extern int		ni_system_bridge_shutdown(ni_netdev_t *);
 extern int		ni_system_bridge_delete(ni_netconfig_t *, ni_netdev_t *);

src/bridge.c

@@ -41,6 +41,20 @@ ni_bridge_port_new(ni_bridge_t *bridge, const char *ifname, unsigned int ifindex
 	return port;
 }
 
+ni_bridge_port_t *
+ni_bridge_port_clone(const ni_bridge_port_t *src)
+{
+	ni_bridge_port_t *dst;
+
+	if (src) {
+		dst = ni_bridge_port_new(NULL, src->ifname, src->ifindex);
+		dst->priority = src->priority;
+		dst->path_cost = src->path_cost;
+		return dst;
+	}
+	return NULL;
+}
+
 void
 ni_bridge_port_free(ni_bridge_port_t *port)
 {

src/client/client_state.c

@@ -368,6 +368,7 @@ ni_client_state_save(const ni_client_state_t *client_state, unsigned int ifindex
 		xml_node_free(node);
 		goto failure;
 	}
+	xml_node_free(node);
 
 	if (rename(temp, path) < 0) {
 		ni_error("Cannot move temp file to state file %s", path);

src/dbus-objects/bridge.c

@@ -294,7 +294,9 @@ __ni_objectmodel_bridge_set_ports(ni_dbus_object_t *object, const ni_dbus_proper
 			return FALSE;
 		}
 
-		ni_bridge_add_port(bridge, port);
+		if (ni_bridge_add_port(bridge, port) < 0) {
+			ni_bridge_port_free(port); /* duplicate port */
+		}
 	}
 
 	return TRUE;
@@ -315,8 +317,10 @@ __ni_objectmodel_bridge_port_to_dict(const ni_bridge_port_t *port, ni_dbus_varia
 	 *
 	 * FIXME: should we resolve the object path here?
 	 */
-	if (port->ifname)
-		ni_dbus_dict_add_string(dict, "device", port->ifname);
+	if (ni_string_empty(port->ifname))
+		return FALSE;
+
+	ni_dbus_dict_add_string(dict, "device", port->ifname);
 	ni_dbus_dict_add_uint32(dict, "priority", port->priority);
 	ni_dbus_dict_add_uint32(dict, "path-cost", port->path_cost);
 
@@ -342,8 +346,11 @@ __ni_objectmodel_bridge_port_from_dict(ni_bridge_port_t *port, const ni_dbus_var
 		return TRUE;
 
 	/* FIXME: should expect object path here and map that to an ifindex */
-	if (ni_dbus_dict_get_string(dict, "device", &string))
+	if (ni_dbus_dict_get_string(dict, "device", &string) && !ni_string_empty(string))
 		ni_string_dup(&port->ifname, string);
+	else
+		return FALSE;
+
 	if (ni_dbus_dict_get_uint32(dict, "priority", &value))
 		port->priority = value;
 	if (ni_dbus_dict_get_uint32(dict, "path-cost", &value))

src/ifconfig.c

@@ -1130,13 +1130,13 @@ ni_system_bridge_delete(ni_netconfig_t *nc, ni_netdev_t *dev)
 
 /*
  * Add a port to a bridge interface
- * Note, in case of success, the bridge will have taken ownership of the port object.
  */
 int
-ni_system_bridge_add_port(ni_netconfig_t *nc, ni_netdev_t *brdev, ni_bridge_port_t *port)
+ni_system_bridge_add_port(ni_netconfig_t *nc, ni_netdev_t *brdev, const ni_bridge_port_t *port)
 {
 	ni_bridge_t *bridge = ni_netdev_get_bridge(brdev);
 	ni_netdev_t *pif = NULL;
+	ni_bridge_port_t *new_port;
 	int rv;
 
 	if (port->ifindex)
@@ -1210,7 +1210,14 @@ ni_system_bridge_add_port(ni_netconfig_t *nc, ni_netdev_t *brdev, ni_bridge_port
 		return rv;
 	}
 
-	ni_bridge_add_port(bridge, port);
+	/* when this fails, next event will update/add it... */
+	new_port = ni_bridge_port_clone(port);
+	new_port->ifindex = pif->link.ifindex;
+	if (!ni_string_eq(new_port->ifname, pif->name))
+		ni_string_dup(&new_port->ifname, pif->name);
+
+	if (!ni_bridge_add_port(bridge, new_port))
+		ni_bridge_port_free(new_port);
 	return 0;
 }
 

src/ifevent.c

@@ -441,6 +441,7 @@ __ni_rtevent_deladdr(ni_netconfig_t *nc, const struct sockaddr_nl *nladdr, struc
 
 		__ni_address_list_remove(&dev->addrs, ap);
 	}
+	ni_string_free(&tmp.label);
 
 	return 0;
 }

src/iflist.c

@@ -1831,8 +1831,10 @@ __ni_netdev_process_newaddr_event(ni_netdev_t *dev, struct nlmsghdr *h, struct i
 	ap = ni_address_list_find(dev->addrs, &tmp.local_addr);
 	if (!ap) {
 		ap = ni_netdev_add_address(dev, tmp.family, tmp.prefixlen, &tmp.local_addr);
-		if (!ap)
+		if (!ap) {
+			ni_string_free(&tmp.label);
 			return -1;
+		}
 	}
 	ap->seq = dev->seq;
 	ap->scope = tmp.scope;

src/process.c

@@ -423,9 +423,11 @@ ni_process_run_and_capture_output(ni_process_t *pi, ni_buffer_t *out_buffer)
 			out_buffer->tail += cnt;
 		} else if (errno != EINTR) {
 			ni_error("read error on subprocess pipe: %m");
-			return -1;
+			rv = -1;
+			break;
 		}
 	}
+	close(pfd[0]);
 
 	while (waitpid(pi->pid, &pi->status, 0) < 0) {
 		if (errno == EINTR)