Merge pull request #31 from libusb/claude/fix-libusb-compat-issue-2wVhM

Fix memory leaks in USB bus and device cleanup

Author: mcuee

.gitignore

@@ -29,5 +29,6 @@ compile
 depcomp
 examples/lsusb
 examples/testlibusb
+examples/hotplug_monitor
 config.h.in
 libtool

examples/Makefile.am

@@ -1,9 +1,12 @@
 AM_CPPFLAGS = -I$(top_srcdir)/libusb
-noinst_PROGRAMS = lsusb testlibusb
+noinst_PROGRAMS = lsusb testlibusb hotplug_monitor
 
 lsusb_SOURCES = lsusb.c
 lsusb_LDADD = ../libusb/libusb.la
 
 testlibusb_SOURCES = testlibusb.c
 testlibusb_LDADD = ../libusb/libusb.la
 
+hotplug_monitor_SOURCES = hotplug-monitor.c
+hotplug_monitor_LDADD = ../libusb/libusb.la
+

examples/README

@@ -0,0 +1,59 @@
+libusb-compat-0.1 examples
+==========================
+
+Small programs that exercise the libusb-0.1 compatibility shim. They are
+built automatically by `make` but are not installed.
+
+lsusb
+-----
+Prints `idVendor:idProduct` for every USB device. The smallest possible
+demonstration of the libusb-0.1 enumeration API.
+
+testlibusb [-v]
+---------------
+Walks every bus and device, opens each one, and prints descriptors.
+With `-v`, also prints each device's full configuration tree.
+
+hotplug_monitor [poll-ms]
+-------------------------
+Polls usb_find_busses() and usb_find_devices() in a loop and prints the
+current bus/device tree whenever either reports a change. Useful for
+exercising the bus-removal path in usb_find_busses() and for verifying
+that exit produces no "device X.Y still referenced" warnings from the
+underlying libusb (issue #25).
+
+Default polling interval is 500 ms; pass an integer argv[1] in
+milliseconds to override. Exit with Ctrl-C; the destructor runs and
+frees all retained device references.
+
+Triggering hot-plug events
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+* Device add/remove: plug or unplug any USB device.
+
+* Bus add/remove (Linux, requires root): unbind and rebind the USB
+  host controller PCI device. This is the only way to make a whole
+  bus disappear from libusb's perspective on most systems.
+
+      lspci -D | grep USB
+      # pick the BDF of the controller you want to drop, e.g.
+      echo 0000:00:14.0 | sudo tee /sys/bus/pci/drivers/xhci_hcd/unbind
+      echo 0000:00:14.0 | sudo tee /sys/bus/pci/drivers/xhci_hcd/bind
+
+  Replace `xhci_hcd` with `ehci-pci`, `ohci-pci`, or `uhci_hcd` as
+  appropriate. Unbinding the controller you booted from will make
+  attached input devices stop working until you rebind it; consider
+  picking a controller that is not in use.
+
+Verifying reference accounting
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Run with the libusb-1.0 debug level cranked up and watch the exit
+output:
+
+      LIBUSB_DEBUG=4 ./hotplug_monitor
+
+After Ctrl-C there should be no `device X.Y still referenced` warnings
+from `libusb_exit`. For an exhaustive memory check, run under valgrind:
+
+      LIBUSB_DEBUG=4 valgrind --leak-check=full ./hotplug_monitor

examples/hotplug-monitor.c

@@ -0,0 +1,76 @@
+/*
+ * hotplug-monitor.c -- exercise libusb-compat-0.1 hot-plug paths.
+ *
+ * Polls usb_find_busses() and usb_find_devices() in a loop and prints the
+ * current bus/device tree whenever either reports a change. Useful for
+ * exercising the bus-removal path in usb_find_busses() and for verifying
+ * that a clean Ctrl-C exit produces no "device X.Y still referenced"
+ * warnings from the underlying libusb (issue #25).
+ *
+ * See examples/README for how to trigger bus add/remove on Linux.
+ *
+ * This file is in the public domain.
+ */
+
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <usb.h>
+
+static volatile sig_atomic_t running = 1;
+
+static void on_signal(int signo)
+{
+	(void) signo;
+	running = 0;
+}
+
+static void print_state(void)
+{
+	struct usb_bus *bus;
+	for (bus = usb_get_busses(); bus; bus = bus->next) {
+		struct usb_device *dev;
+		printf("  bus %s\n", bus->dirname);
+		for (dev = bus->devices; dev; dev = dev->next)
+			printf("    dev %s  %04x:%04x\n",
+				dev->filename,
+				dev->descriptor.idVendor,
+				dev->descriptor.idProduct);
+	}
+}
+
+int main(int argc, char *argv[])
+{
+	unsigned int interval_ms = 500;
+	int iter = 0;
+
+	if (argc > 1) {
+		int v = atoi(argv[1]);
+		if (v > 0)
+			interval_ms = (unsigned int) v;
+	}
+
+	signal(SIGINT, on_signal);
+	signal(SIGTERM, on_signal);
+
+	usb_init();
+
+	while (running) {
+		int bus_changes = usb_find_busses();
+		int dev_changes = usb_find_devices();
+
+		if (iter == 0 || bus_changes || dev_changes) {
+			printf("[iter %d] bus_changes=%d dev_changes=%d\n",
+				iter, bus_changes, dev_changes);
+			print_state();
+			fflush(stdout);
+		}
+
+		iter++;
+		usleep(interval_ms * 1000u);
+	}
+
+	printf("exiting\n");
+	return 0;
+}

libusb/core.c

@@ -81,8 +81,29 @@ static void __attribute__ ((constructor)) _usb_init (void)
 }
 #endif
 
+static void free_device(struct usb_device *dev);
+
+static void free_bus(struct usb_bus *bus)
+{
+	struct usb_device *dev = bus->devices;
+	while (dev) {
+		struct usb_device *tdev = dev->next;
+		free_device(dev);
+		dev = tdev;
+	}
+	free(bus);
+}
+
 static void __attribute__ ((destructor)) _usb_exit (void)
 {
+	struct usb_bus *bus = usb_busses;
+	while (bus) {
+		struct usb_bus *tbus = bus->next;
+		free_bus(bus);
+		bus = tbus;
+	}
+	usb_busses = NULL;
+
 	if (ctx) {
 		libusb_exit (ctx);
 		ctx = NULL;
@@ -261,6 +282,7 @@ static int find_busses(struct usb_bus **ret)
 	return 0;
 
 err:
+	libusb_free_device_list(dev_list, 1);
 	bus = busses;
 	while (bus) {
 		struct usb_bus *tbus = bus->next;
@@ -317,7 +339,7 @@ API_EXPORTED int usb_find_busses(void)
 			usbi_dbg("bus %d removed", bus->location);
 			changes++;
 			LIST_DEL(usb_busses, bus);
-			free(bus);
+			free_bus(bus);
 		}
 
 		bus = tbus;
@@ -591,6 +613,7 @@ static int initialize_device(struct usb_device *dev)
 static void free_device(struct usb_device *dev)
 {
 	clear_device(dev);
+	free(dev->config);
 	libusb_unref_device(dev->dev);
 	free(dev);
 }
@@ -657,10 +680,12 @@ API_EXPORTED int usb_find_devices(void)
 		dev = new_devices;
 		while (dev) {
 			struct usb_device *tdev = dev->next;
-			r = initialize_device(dev);	
+			r = initialize_device(dev);
 			if (r < 0) {
 				usbi_err("couldn't initialize device %d.%d (error %d)",
 					dev->bus->location, dev->devnum, r);
+				LIST_DEL(new_devices, dev);
+				free(dev);
 				dev = tdev;
 				continue;
 			}