USB: Handle live changes to the set of FDs to poll

Introduce new internal session API for changing the set of polled
file descriptors for an already installed event source. Use the
new API to apply changes to the USB poll FDs when requested to do
so by libusb. Doing so is necessary to make the generic USB code
work on Windows.

src/libsigrok-internal.h

@@ -735,11 +735,14 @@ struct sr_session {
 };
 
 SR_PRIV int sr_session_source_add_internal(struct sr_session *session,
-		const GPollFD *pollfds, int num_fds, int timeout,
+		int timeout, sr_receive_data_callback cb, void *cb_data,
-		sr_receive_data_callback cb, void *cb_data,
 		gintptr poll_object);
+SR_PRIV int sr_session_source_poll_add(struct sr_session *session,
+		gintptr poll_object, gintptr fd, int events);
 SR_PRIV int sr_session_source_remove_internal(struct sr_session *session,
 		gintptr poll_object);
+SR_PRIV int sr_session_source_poll_remove(struct sr_session *session,
+		gintptr poll_object, gintptr fd);
 SR_PRIV int sr_session_send(const struct sr_dev_inst *sdi,
 		const struct sr_datafeed_packet *packet);
 SR_PRIV int sr_session_stop_sync(struct sr_session *session);

src/session.c

@@ -526,8 +526,16 @@ static int sr_session_iteration(struct sr_session *session)
 		 */
 		sr_spew("callback for event source %" G_GINTPTR_FORMAT " with"
 			" event mask 0x%.2X", poll_object, (unsigned)revents);
-		if (!source->cb(fd, revents, source->cb_data))
+		if (!source->cb(fd, revents, source->cb_data)) {
-			sr_session_source_remove_internal(session, poll_object);
+			/* Hackish, to be cleaned up when porting to
+			 * the GLib main loop.
+			 */
+			if (poll_object == (gintptr)session->ctx->libusb_ctx)
+				usb_source_remove(session, session->ctx);
+			else
+				sr_session_source_remove_internal(session,
+						poll_object);
+		}
 		/*
 		 * We want to take as little time as possible to stop
 		 * the session if we have been told to do so. Therefore,
@@ -902,8 +910,6 @@ SR_PRIV int sr_session_send(const struct sr_dev_inst *sdi,
  * Add an event source for a file descriptor.
  *
  * @param session The session to use. Must not be NULL.
- * @param[in] pollfds The FDs to poll, or NULL if @a num_fds is 0.
- * @param[in] num_fds Number of FDs in the array.
  * @param[in] timeout Max time in ms to wait before the callback is called,
  *                    or -1 to wait indefinitely.
  * @param cb Callback function to add. Must not be NULL.
@@ -915,23 +921,17 @@ SR_PRIV int sr_session_send(const struct sr_dev_inst *sdi,
  * @retval SR_ERR An event source for @a poll_object is already installed.
  */
 SR_PRIV int sr_session_source_add_internal(struct sr_session *session,
-		const GPollFD *pollfds, int num_fds, int timeout,
+		int timeout, sr_receive_data_callback cb, void *cb_data,
-		sr_receive_data_callback cb, void *cb_data,
 		gintptr poll_object)
 {
 	struct source src;
 	unsigned int i;
-	int k;
 
 	/* Note: cb_data can be NULL, that's not a bug. */
 	if (!cb) {
 		sr_err("%s: cb was NULL", __func__);
 		return SR_ERR_ARG;
 	}
-	if (!pollfds && num_fds != 0) {
-		sr_err("%s: pollfds was NULL", __func__);
-		return SR_ERR_ARG;
-	}
 	/* Make sure that poll_object is unique.
 	 */
 	for (i = 0; i < session->sources->len; ++i) {
@@ -942,12 +942,12 @@ SR_PRIV int sr_session_source_add_internal(struct sr_session *session,
 			return SR_ERR;
 		}
 	}
-	sr_dbg("Installing event source %" G_GINTPTR_FORMAT " with %d FDs"
+	sr_dbg("Installing event source %" G_GINTPTR_FORMAT
-		" and %d ms timeout.", poll_object, num_fds, timeout);
+		" with %d ms timeout.", poll_object, timeout);
 	src.cb = cb;
 	src.cb_data = cb_data;
 	src.poll_object = poll_object;
-	src.num_fds = num_fds;
+	src.num_fds = 0;
 	src.triggered = FALSE;
 
 	if (timeout >= 0) {
@@ -959,12 +959,56 @@ SR_PRIV int sr_session_source_add_internal(struct sr_session *session,
 	}
 	g_array_append_val(session->sources, src);
 
-	for (k = 0; k < num_fds; ++k) {
+	return SR_OK;
-		sr_dbg("Registering poll FD %" G_GINTPTR_FORMAT
-			" with event mask 0x%.2X.",
-			(gintptr)pollfds[k].fd, (unsigned)pollfds[k].events);
 }
-	g_array_append_vals(session->pollfds, pollfds, num_fds);
+
+SR_PRIV int sr_session_source_poll_add(struct sr_session *session,
+		gintptr poll_object, gintptr fd, int events)
+{
+	struct source *source;
+	GPollFD pollfd;
+	unsigned int i;
+	int fd_index, k;
+
+	source = NULL;
+	fd_index = 0;
+
+	/* Look up existing event source.
+	 */
+	for (i = 0; i < session->sources->len; ++i) {
+		source = &g_array_index(session->sources, struct source, i);
+		if (source->poll_object == poll_object)
+			break;
+		fd_index += source->num_fds;
+	}
+	if (!source) {
+		sr_err("Cannot add poll FD %" G_GINTPTR_FORMAT
+			" to non-existing event source %" G_GINTPTR_FORMAT
+			".",  fd, poll_object);
+		return SR_ERR;
+	}
+	/* Make sure the FD is unique.
+	 */
+	for (k = 0; k < source->num_fds; ++k)
+		if (g_array_index(session->pollfds, GPollFD, fd_index + k)
+				.fd == fd) {
+			sr_err("Cannot add poll FD %" G_GINTPTR_FORMAT
+				" twice to event source %" G_GINTPTR_FORMAT
+				".", fd, poll_object);
+			return SR_ERR;
+		}
+
+	pollfd.fd = fd;
+	pollfd.events = events;
+	pollfd.revents = 0;
+
+	g_array_insert_val(session->pollfds,
+		fd_index + source->num_fds, pollfd);
+	++source->num_fds;
+
+	sr_dbg("Added poll FD %" G_GINTPTR_FORMAT " with event mask 0x%.2X"
+		" to event source %" G_GINTPTR_FORMAT ".",
+		fd, (unsigned)events, poll_object);
 
 	return SR_OK;
 }
@@ -973,7 +1017,7 @@ SR_PRIV int sr_session_source_add_internal(struct sr_session *session,
  * Add an event source for a file descriptor.
  *
  * @param session The session to use. Must not be NULL.
- * @param fd The file descriptor.
+ * @param fd The file descriptor, or a negative value to create a timer source.
  * @param events Events to check for.
  * @param timeout Max time in ms to wait before the callback is called,
  *                or -1 to wait indefinitely.
@@ -988,18 +1032,17 @@ SR_PRIV int sr_session_source_add_internal(struct sr_session *session,
 SR_API int sr_session_source_add(struct sr_session *session, int fd,
 		int events, int timeout, sr_receive_data_callback cb, void *cb_data)
 {
-	GPollFD p;
+	int ret;
 
 	if (fd < 0 && timeout < 0) {
 		sr_err("Timer source without timeout would block indefinitely");
 		return SR_ERR_ARG;
 	}
-	p.fd = fd;
+	ret = sr_session_source_add_internal(session, timeout, cb, cb_data, fd);
-	p.events = events;
+	if (ret != SR_OK || fd < 0)
-	p.revents = 0;
+		return ret;
 
-	return sr_session_source_add_internal(session,
+	return sr_session_source_poll_add(session, fd, fd, events);
-		&p, (fd < 0) ? 0 : 1, timeout, cb, cb_data, fd);
 }
 
 /**
@@ -1021,12 +1064,19 @@ SR_API int sr_session_source_add_pollfd(struct sr_session *session,
 		GPollFD *pollfd, int timeout, sr_receive_data_callback cb,
 		void *cb_data)
 {
+	int ret;
+
 	if (!pollfd) {
 		sr_err("%s: pollfd was NULL", __func__);
 		return SR_ERR_ARG;
 	}
-	return sr_session_source_add_internal(session, pollfd, 1,
+	ret = sr_session_source_add_internal(session,
 			timeout, cb, cb_data, (gintptr)pollfd);
+	if (ret != SR_OK)
+		return ret;
+
+	return sr_session_source_poll_add(session,
+			(gintptr)pollfd, pollfd->fd, pollfd->events);
 }
 
 /**
@@ -1049,17 +1099,22 @@ SR_API int sr_session_source_add_channel(struct sr_session *session,
 		GIOChannel *channel, int events, int timeout,
 		sr_receive_data_callback cb, void *cb_data)
 {
-	GPollFD p;
+	int ret;
 
-#ifdef G_OS_WIN32
+	ret = sr_session_source_add_internal(session,
-	g_io_channel_win32_make_pollfd(channel, events, &p);
-#else
-	p.fd = g_io_channel_unix_get_fd(channel);
-	p.events = events;
-	p.revents = 0;
-#endif
-	return sr_session_source_add_internal(session, &p, 1,
 			timeout, cb, cb_data, (gintptr)channel);
+	if (ret != SR_OK)
+		return ret;
+#ifdef G_OS_WIN32
+	GPollFD p;
+	g_io_channel_win32_make_pollfd(channel, events, &p);
+
+	return sr_session_source_poll_add(session,
+			(gintptr)channel, p.fd, p.events);
+#else
+	return sr_session_source_poll_add(session, (gintptr)channel,
+			g_io_channel_unix_get_fd(channel), events);
+#endif
 }
 
 /**
@@ -1108,6 +1163,52 @@ SR_PRIV int sr_session_source_remove_internal(struct sr_session *session,
 	return SR_ERR_BUG;
 }
 
+SR_PRIV int sr_session_source_poll_remove(struct sr_session *session,
+		gintptr poll_object, gintptr fd)
+{
+	struct source *source;
+	unsigned int i;
+	int fd_index, k;
+
+	source = NULL;
+	fd_index = 0;
+
+	/* Look up existing event source.
+	 */
+	for (i = 0; i < session->sources->len; ++i) {
+		source = &g_array_index(session->sources, struct source, i);
+		if (source->poll_object == poll_object)
+			break;
+		fd_index += source->num_fds;
+	}
+	if (!source) {
+		sr_err("Cannot remove poll FD %" G_GINTPTR_FORMAT
+			" from non-existing event source %" G_GINTPTR_FORMAT
+			".", fd, poll_object);
+		return SR_ERR;
+	}
+	/* Look up the FD in the poll set.
+	 */
+	for (k = 0; k < source->num_fds; ++k)
+		if (g_array_index(session->pollfds, GPollFD, fd_index + k)
+				.fd == fd) {
+
+			g_array_remove_index(session->pollfds, fd_index + k);
+			--source->num_fds;
+
+			sr_dbg("Removed poll FD %" G_GINTPTR_FORMAT
+				" from event source %" G_GINTPTR_FORMAT ".",
+				fd, poll_object);
+			return SR_OK;
+		}
+
+	sr_err("Cannot remove non-existing poll FD %" G_GINTPTR_FORMAT
+		" from event source %" G_GINTPTR_FORMAT ".",
+		fd, poll_object);
+
+	return SR_ERR;
+}
+
 /**
  * Remove the source belonging to the specified file descriptor.
  *

src/usb.c

@@ -184,55 +184,88 @@ SR_PRIV void sr_usb_close(struct sr_usb_dev_inst *usb)
 	sr_dbg("Closed USB device %d.%d.", usb->bus, usb->address);
 }
 
+#if (LIBUSB_API_VERSION < 0x01000104)
+typedef int libusb_os_handle;
+#endif
+
+static LIBUSB_CALL void usb_pollfd_added(libusb_os_handle fd,
+		short events, void *user_data)
+{
+	struct sr_session *session;
+	gintptr tag;
+
+	session = user_data;
+	tag = (gintptr)session->ctx->libusb_ctx;
+#ifdef G_OS_WIN32
+	events = G_IO_IN;
+#endif
+	sr_session_source_poll_add(session, tag, (gintptr)fd, events);
+}
+
+static LIBUSB_CALL void usb_pollfd_removed(libusb_os_handle fd, void *user_data)
+{
+	struct sr_session *session;
+	gintptr tag;
+
+	session = user_data;
+	tag = (gintptr)session->ctx->libusb_ctx;
+
+	sr_session_source_poll_remove(session, tag, (gintptr)fd);
+}
+
 SR_PRIV int usb_source_add(struct sr_session *session, struct sr_context *ctx,
 		int timeout, sr_receive_data_callback cb, void *cb_data)
 {
-	const struct libusb_pollfd **lupfd;
+	const struct libusb_pollfd **pollfds;
-	GPollFD *pollfds;
+	gintptr tag;
 	int i;
-	int num_fds = 0;
 	int ret;
+	int events;
 
 	if (ctx->usb_source_present) {
 		sr_err("A USB event source is already present.");
 		return SR_ERR;
 	}
-	lupfd = libusb_get_pollfds(ctx->libusb_ctx);
+	pollfds = libusb_get_pollfds(ctx->libusb_ctx);
-	if (!lupfd || !lupfd[0]) {
+	if (!pollfds) {
-		free(lupfd);
 		sr_err("Failed to get libusb file descriptors.");
 		return SR_ERR;
 	}
-	while (lupfd[num_fds])
+	tag = (gintptr)ctx->libusb_ctx;
-		++num_fds;
+	ret = sr_session_source_add_internal(session,
-	pollfds = g_new(GPollFD, num_fds);
+			timeout, cb, cb_data, tag);
-
-	for (i = 0; i < num_fds; ++i) {
-#if defined(G_OS_WIN32) && (GLIB_SIZEOF_VOID_P == 4)
-		/* Avoid a warning on 32-bit Windows. */
-		pollfds[i].fd = (gintptr)lupfd[i]->fd;
-#else
-		pollfds[i].fd = lupfd[i]->fd;
-#endif
-#ifdef G_OS_WIN32
-		pollfds[i].events = G_IO_IN;
-#else
-		pollfds[i].events = lupfd[i]->events;
-#endif
-		pollfds[i].revents = 0;
-	}
-	free(lupfd);
-	ret = sr_session_source_add_internal(session, pollfds, num_fds,
-			timeout, cb, cb_data, (gintptr)ctx->libusb_ctx);
-	g_free(pollfds);
 
 	ctx->usb_source_present = (ret == SR_OK);
 
+	for (i = 0; ret == SR_OK && pollfds[i]; ++i) {
+#ifdef G_OS_WIN32
+		events = G_IO_IN;
+#else
+		events = pollfds[i]->events;
+#endif
+		ret = sr_session_source_poll_add(session, tag,
+				(gintptr)pollfds[i]->fd, events);
+	}
+#if (LIBUSB_API_VERSION >= 0x01000104)
+	libusb_free_pollfds(pollfds);
+#else
+	free(pollfds);
+#endif
+	if (ret != SR_OK)
 		return ret;
+
+	libusb_set_pollfd_notifiers(ctx->libusb_ctx,
+		&usb_pollfd_added, &usb_pollfd_removed, session);
+
+	return SR_OK;
 }
 
 SR_PRIV int usb_source_remove(struct sr_session *session, struct sr_context *ctx)
 {
+	ctx->usb_source_present = FALSE;
+
+	libusb_set_pollfd_notifiers(ctx->libusb_ctx, NULL, NULL, NULL);
+
 	return sr_session_source_remove_internal(session,
 			(gintptr)ctx->libusb_ctx);
 }