core: Eliminate hotplug pipe, using list and event pipe instead

To further consolidate libusb internal events, this change removes
the hotplug pipe. Hotplug messages are now kept in a list within the
context, and the event pipe is signalled when a new hotplug message
is added. When handling events, the hotplug messages will be processed
from the list instead of from a separate pipe.

This change is greatly beneficial for the Windows/WinCE backends which
do not allow pipes to be used in the WaitFor* functions.

Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>

6 files changed, 81 insertions, 94 deletions

diff --git a/libusb/core.c b/libusb/core.c
index 13100da..626fabc 100644
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -684,12 +684,8 @@ struct libusb_device *usbi_alloc_device(struct libusb_context *ctx,
 
 void usbi_connect_device(struct libusb_device *dev)
 {
-	libusb_hotplug_message message;
-	ssize_t ret;
+	struct libusb_context *ctx = DEVICE_CTX(dev);
 
-	memset(&message, 0, sizeof(message));
-	message.event = LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED;
-	message.device = dev;
 	dev->attached = 1;
 
 	usbi_mutex_lock(&dev->ctx->usb_devs_lock);
@@ -697,25 +693,17 @@ void usbi_connect_device(struct libusb_device *dev)
 	usbi_mutex_unlock(&dev->ctx->usb_devs_lock);
 
 	/* Signal that an event has occurred for this device if we support hotplug AND
-	 * the hotplug pipe is ready. This prevents an event from getting raised during
-	 * initial enumeration. */
-	if (libusb_has_capability(LIBUSB_CAP_HAS_HOTPLUG) && dev->ctx->hotplug_pipe[1] > 0) {
-		ret = usbi_write(dev->ctx->hotplug_pipe[1], &message, sizeof(message));
-		if (sizeof (message) != ret) {
-			usbi_err(DEVICE_CTX(dev), "error writing hotplug message");
-		}
+	 * the hotplug message list is ready. This prevents an event from getting raised
+	 * during initial enumeration. */
+	if (libusb_has_capability(LIBUSB_CAP_HAS_HOTPLUG) && dev->ctx->hotplug_msgs.next) {
+		usbi_hotplug_notification(ctx, dev, LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED);
 	}
 }
 
 void usbi_disconnect_device(struct libusb_device *dev)
 {
-	libusb_hotplug_message message;
-	struct libusb_context *ctx = dev->ctx;
-	ssize_t ret;
+	struct libusb_context *ctx = DEVICE_CTX(dev);
 
-	memset(&message, 0, sizeof(message));
-	message.event = LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT;
-	message.device = dev;
 	usbi_mutex_lock(&dev->lock);
 	dev->attached = 0;
 	usbi_mutex_unlock(&dev->lock);
@@ -725,14 +713,11 @@ void usbi_disconnect_device(struct libusb_device *dev)
 	usbi_mutex_unlock(&ctx->usb_devs_lock);
 
 	/* Signal that an event has occurred for this device if we support hotplug AND
-	 * the hotplug pipe is ready. This prevents an event from getting raised during
-	 * initial enumeration. libusb_handle_events will take care of dereferencing the
-	 * device. */
-	if (libusb_has_capability(LIBUSB_CAP_HAS_HOTPLUG) && dev->ctx->hotplug_pipe[1] > 0) {
-		ret = usbi_write(dev->ctx->hotplug_pipe[1], &message, sizeof(message));
-		if (sizeof(message) != ret) {
-			usbi_err(DEVICE_CTX(dev), "error writing hotplug message");
-		}
+	 * the hotplug message list is ready. This prevents an event from getting raised
+	 * during initial enumeration. libusb_handle_events will take care of dereferencing
+	 * the device. */
+	if (libusb_has_capability(LIBUSB_CAP_HAS_HOTPLUG) && dev->ctx->hotplug_msgs.next) {
+		usbi_hotplug_notification(ctx, dev, LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT);
 	}
 }
 
diff --git a/libusb/hotplug.c b/libusb/hotplug.c
index 9ecb2c7..9171f2c 100644
--- a/libusb/hotplug.c
+++ b/libusb/hotplug.c
@@ -203,6 +203,27 @@ void usbi_hotplug_match(struct libusb_context *ctx, struct libusb_device *dev,
 	/* the backend is expected to call the callback for each active transfer */
 }
 
+void usbi_hotplug_notification(struct libusb_context *ctx, struct libusb_device *dev,
+	libusb_hotplug_event event)
+{
+	libusb_hotplug_message *message = calloc(1, sizeof(*message));
+
+	if (!message) {
+		usbi_err(ctx, "error allocating hotplug message");
+		return;
+	}
+
+	message->event = event;
+	message->device = dev;
+
+	/* Take the event data lock and add this message to the list. */
+	usbi_mutex_lock(&ctx->event_data_lock);
+	list_add_tail(&message->list, &ctx->hotplug_msgs);
+	usbi_mutex_unlock(&ctx->event_data_lock);
+
+	usbi_signal_event(ctx);
+}
+
 int API_EXPORTED libusb_hotplug_register_callback(libusb_context *ctx,
 	libusb_hotplug_event events, libusb_hotplug_flag flags,
 	int vendor_id, int product_id, int dev_class,
@@ -285,8 +306,6 @@ void API_EXPORTED libusb_hotplug_deregister_callback (struct libusb_context *ctx
 	libusb_hotplug_callback_handle handle)
 {
 	struct libusb_hotplug_callback *hotplug_cb;
-	libusb_hotplug_message message;
-	ssize_t ret;
 
 	/* check for hotplug support */
 	if (!libusb_has_capability(LIBUSB_CAP_HAS_HOTPLUG)) {
@@ -305,12 +324,7 @@ void API_EXPORTED libusb_hotplug_deregister_callback (struct libusb_context *ctx
 	}
 	usbi_mutex_unlock(&ctx->hotplug_cbs_lock);
 
-	/* wakeup handle_events to do the actual free */
-	memset(&message, 0, sizeof(message));
-	ret = usbi_write(ctx->hotplug_pipe[1], &message, sizeof(message));
-	if (sizeof(message) != ret) {
-		usbi_err(ctx, "error writing hotplug message");
-	}
+	usbi_hotplug_notification(ctx, NULL, 0);
 }
 
 void usbi_hotplug_deregister_all(struct libusb_context *ctx) {
diff --git a/libusb/hotplug.h b/libusb/hotplug.h
index 321a0a8..2bec81b 100644
--- a/libusb/hotplug.h
+++ b/libusb/hotplug.h
@@ -69,8 +69,14 @@ struct libusb_hotplug_callback {
 typedef struct libusb_hotplug_callback libusb_hotplug_callback;
 
 struct libusb_hotplug_message {
+	/** The hotplug event that occurred */
 	libusb_hotplug_event event;
+
+	/** The device for which this hotplug event occurred */
 	struct libusb_device *device;
+
+	/** List this message is contained in (ctx->hotplug_msgs) */
+	struct list_head list;
 };
 
 typedef struct libusb_hotplug_message libusb_hotplug_message;
@@ -78,5 +84,7 @@ typedef struct libusb_hotplug_message libusb_hotplug_message;
 void usbi_hotplug_deregister_all(struct libusb_context *ctx);
 void usbi_hotplug_match(struct libusb_context *ctx, struct libusb_device *dev,
 			libusb_hotplug_event event);
+void usbi_hotplug_notification(struct libusb_context *ctx, struct libusb_device *dev,
+			libusb_hotplug_event event);
 
 #endif
diff --git a/libusb/io.c b/libusb/io.c
index f00af14..6ae4f8d 100644
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1118,6 +1118,7 @@ int usbi_io_init(struct libusb_context *ctx)
 	usbi_cond_init(&ctx->event_waiters_cond, NULL);
 	list_init(&ctx->flying_transfers);
 	list_init(&ctx->ipollfds);
+	list_init(&ctx->hotplug_msgs);
 
 	/* FIXME should use an eventfd on kernels that support it */
 	r = usbi_pipe(ctx->event_pipe);
@@ -1130,17 +1131,6 @@ int usbi_io_init(struct libusb_context *ctx)
 	if (r < 0)
 		goto err_close_pipe;
 
-	/* create hotplug pipe */
-	r = usbi_pipe(ctx->hotplug_pipe);
-	if (r < 0) {
-		r = LIBUSB_ERROR_OTHER;
-		goto err_remove_pipe;
-	}
-
-	r = usbi_add_pollfd(ctx, ctx->hotplug_pipe[0], POLLIN);
-	if (r < 0)
-		goto err_close_hp_pipe;
-
 #ifdef USBI_TIMERFD_AVAILABLE
 	ctx->timerfd = timerfd_create(usbi_backend->get_timerfd_clockid(),
 		TFD_NONBLOCK);
@@ -1160,13 +1150,8 @@ int usbi_io_init(struct libusb_context *ctx)
 #ifdef USBI_TIMERFD_AVAILABLE
 err_close_timerfd:
 	close(ctx->timerfd);
-	usbi_remove_pollfd(ctx, ctx->hotplug_pipe[0]);
-#endif
-err_close_hp_pipe:
-	usbi_close(ctx->hotplug_pipe[0]);
-	usbi_close(ctx->hotplug_pipe[1]);
-err_remove_pipe:
 	usbi_remove_pollfd(ctx, ctx->event_pipe[0]);
+#endif
 err_close_pipe:
 	usbi_close(ctx->event_pipe[0]);
 	usbi_close(ctx->event_pipe[1]);
@@ -1185,9 +1170,6 @@ void usbi_io_exit(struct libusb_context *ctx)
 	usbi_remove_pollfd(ctx, ctx->event_pipe[0]);
 	usbi_close(ctx->event_pipe[0]);
 	usbi_close(ctx->event_pipe[1]);
-	usbi_remove_pollfd(ctx, ctx->hotplug_pipe[0]);
-	usbi_close(ctx->hotplug_pipe[0]);
-	usbi_close(ctx->hotplug_pipe[1]);
 #ifdef USBI_TIMERFD_AVAILABLE
 	if (usbi_using_timerfd(ctx)) {
 		usbi_remove_pollfd(ctx, ctx->timerfd);
@@ -1978,17 +1960,16 @@ static int handle_events(struct libusb_context *ctx, struct timeval *tv)
 	/* there are certain fds that libusb uses internally, currently:
 	 *
 	 *   1) event pipe
-	 *   2) hotplug pipe
-	 *   3) timerfd
+	 *   2) timerfd
 	 *
 	 * the backend will never need to attempt to handle events on these fds, so
 	 * we determine how many fds are in use internally for this context and when
 	 * handle_events() is called in the backend, the pollfd list and count will
 	 * be adjusted to skip over these internal fds */
 	if (usbi_using_timerfd(ctx))
-		internal_nfds = 3;
-	else
 		internal_nfds = 2;
+	else
+		internal_nfds = 1;
 
 	/* only reallocate the poll fds when the list of poll fds has been modified
 	 * since the last poll, otherwise reuse them to save the additional overhead */
@@ -2048,58 +2029,52 @@ redo_poll:
 
 	/* fds[0] is always the event pipe */
 	if (fds[0].revents) {
-		/* another thread wanted to interrupt event handling, and it succeeded!
-		 * handle any other events that cropped up at the same time, and
-		 * simply return */
 		unsigned int ru;
+		libusb_hotplug_message *message = NULL;
 
 		usbi_dbg("caught a fish on the event pipe");
 
-		/* read the dummy data from the event pipe unless someone is closing
-		 * a device */
+		/* take the the event data lock while processing events */
 		usbi_mutex_lock(&ctx->event_data_lock);
+
+		/* check if someone is closing a device */
 		ru = ctx->device_close;
-		usbi_mutex_unlock(&ctx->event_data_lock);
-		if (!ru) {
-			r = usbi_clear_event(ctx);
-			if (r)
-				goto handled;
+
+		/* check for any pending hotplug messages */
+		if (!list_empty(&ctx->hotplug_msgs)) {
+			usbi_dbg("hotplug message received");
+			special_event = 1;
+			message = list_first_entry(&ctx->hotplug_msgs, libusb_hotplug_message, list);
+			list_del(&message->list);
 		}
 
-		if (0 == --r)
-			goto handled;
-	}
+		usbi_mutex_unlock(&ctx->event_data_lock);
 
-	/* fd[1] is always the hotplug pipe */
-	if (libusb_has_capability(LIBUSB_CAP_HAS_HOTPLUG) && fds[1].revents) {
-		libusb_hotplug_message message;
-		ssize_t ret;
+		/* process the hotplug message, if any */
+		if (message) {
+			usbi_hotplug_match(ctx, message->device, message->event);
 
-		usbi_dbg("caught a fish on the hotplug pipe");
-		special_event = 1;
+			/* the device left, dereference the device */
+			if (LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT == message->event)
+				libusb_unref_device(message->device);
 
-		/* read the message from the hotplug thread */
-		ret = usbi_read(ctx->hotplug_pipe[0], &message, sizeof (message));
-		if (ret != sizeof(message)) {
-			usbi_err(ctx, "hotplug pipe read error %d != %u",
-				 ret, sizeof(message));
-			r = LIBUSB_ERROR_OTHER;
-			goto handled;
+			free(message);
 		}
 
-		usbi_hotplug_match(ctx, message.device, message.event);
-
-		/* the device left. dereference the device */
-		if (LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT == message.event)
-			libusb_unref_device(message.device);
+		/* clear the event pipe if this was an fd or hotplug notification */
+		if (!ru || message) {
+			r = usbi_clear_event(ctx);
+			if (r)
+				goto handled;
+		}
 
 		if (0 == --r)
 			goto handled;
-	} /* else there shouldn't be anything on this pipe */
+	}
 
 #ifdef USBI_TIMERFD_AVAILABLE
-	/* on timerfd configurations, fds[2] is the timerfd */
-	if (usbi_using_timerfd(ctx) && fds[2].revents) {
+	/* on timerfd configurations, fds[1] is the timerfd */
+	if (usbi_using_timerfd(ctx) && fds[1].revents) {
 		/* timerfd indicates that a timeout has expired */
 		int ret;
 		usbi_dbg("timerfd triggered");
diff --git a/libusb/libusbi.h b/libusb/libusbi.h
index cb24cf9..c94c5a2 100644
--- a/libusb/libusbi.h
+++ b/libusb/libusbi.h
@@ -87,6 +87,9 @@ struct list_head {
 #define list_entry(ptr, type, member) \
 	((type *)((uintptr_t)(ptr) - (uintptr_t)offsetof(type, member)))
 
+#define list_first_entry(ptr, type, member) \
+	list_entry((ptr)->next, type, member)
+
 /* Get each entry from a list
  *  pos - A structure pointer has a "member" element
  *  head - list head
@@ -258,7 +261,6 @@ struct libusb_context {
 	/* A list of registered hotplug callbacks */
 	struct list_head hotplug_cbs;
 	usbi_mutex_t hotplug_cbs_lock;
-	int hotplug_pipe[2];
 
 	/* this is a list of in-flight transfer handles, sorted by timeout
 	 * expiration. URBs to timeout the soonest are placed at the beginning of
@@ -298,6 +300,9 @@ struct libusb_context {
 	 * pick up a new fd for polling. Protected by event_data_lock. */
 	unsigned int fd_notify;
 
+	/* A list of pending hotplug messages. Protected by event_data_lock. */
+	struct list_head hotplug_msgs;
+
 	/* used to wait for event completion in threads other than the one that is
 	 * event handling */
 	usbi_mutex_t event_waiters_lock;
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index 9624091..90d77c5 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 10937
+#define LIBUSB_NANO 10938