Input plugin fixes and cleanup

This patch fixes all known issues caused by converting the input plugin
to use BtIO.

5 files changed, 101 insertions, 94 deletions

diff --git a/input/device.c b/input/device.c
index 9c4b2e49..40a98bf6 100644
--- a/input/device.c
+++ b/input/device.c
@@ -74,8 +74,8 @@ struct input_conn {
 	DBusMessage		*pending_connect;
 	char			*uuid;
 	char			*alias;
-	int			ctrl_sk;
-	int			intr_sk;
+	GIOChannel		*ctrl_io;
+	GIOChannel		*intr_io;
 	guint			ctrl_watch;
 	guint			intr_watch;
 	int			timeout;
@@ -136,6 +136,12 @@ static void input_conn_free(struct input_conn *iconn)
 	if (iconn->intr_watch)
 		g_source_remove(iconn->intr_watch);
 
+	if (iconn->intr_io)
+		g_io_channel_unref(iconn->intr_io);
+
+	if (iconn->ctrl_io)
+		g_io_channel_unref(iconn->ctrl_io);
+
 	g_free(iconn->uuid);
 	g_free(iconn->alias);
 	g_free(iconn->fake);
@@ -404,58 +410,49 @@ static gboolean intr_watch_cb(GIOChannel *chan, GIOCondition cond, gpointer data
 	struct input_device *idev = iconn->idev;
 	gboolean connected = FALSE;
 
-	if (cond & (G_IO_HUP | G_IO_ERR))
+	/* Checking for ctrl_watch avoids a double g_io_channel_shutdown since
+	 * it's likely that ctrl_watch_cb has been queued for dispatching in
+	 * this mainloop iteration */
+	if ((cond & (G_IO_HUP | G_IO_ERR)) && iconn->ctrl_watch)
 		g_io_channel_shutdown(chan, TRUE, NULL);
 
 	emit_property_changed(idev->conn, idev->path, INPUT_DEVICE_INTERFACE,
 				"Connected", DBUS_TYPE_BOOLEAN, &connected);
 
-	g_source_remove(iconn->ctrl_watch);
-	iconn->ctrl_watch = 0;
 	iconn->intr_watch = 0;
 
+	g_io_channel_unref(iconn->intr_io);
+	iconn->intr_io = NULL;
+
 	/* Close control channel */
-	if (iconn->ctrl_sk > 0) {
-		close(iconn->ctrl_sk);
-		iconn->ctrl_sk = -1;
-	}
+	if (iconn->ctrl_io && !(cond & G_IO_NVAL))
+		g_io_channel_shutdown(iconn->ctrl_io, TRUE, NULL);
 
 	return FALSE;
-
 }
 
 static gboolean ctrl_watch_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
 {
 	struct input_conn *iconn = data;
 
-	if (cond & (G_IO_HUP | G_IO_ERR))
+	/* Checking for intr_watch avoids a double g_io_channel_shutdown since
+	 * it's likely that intr_watch_cb has been queued for dispatching in
+	 * this mainloop iteration */
+	if ((cond & (G_IO_HUP | G_IO_ERR)) && iconn->intr_watch)
 		g_io_channel_shutdown(chan, TRUE, NULL);
 
-	g_source_remove(iconn->intr_watch);
-	iconn->intr_watch = 0;
 	iconn->ctrl_watch = 0;
 
+	g_io_channel_unref(iconn->ctrl_io);
+	iconn->ctrl_io = NULL;
+
 	/* Close interrupt channel */
-	if (iconn->intr_sk > 0) {
-		close(iconn->intr_sk);
-		iconn->intr_sk = -1;
-	}
+	if (iconn->intr_io && !(cond & G_IO_NVAL))
+		g_io_channel_shutdown(iconn->intr_io, TRUE, NULL);
 
 	return FALSE;
 }
 
-static guint create_watch(int sk, GIOFunc cb, struct input_conn *iconn)
-{
-	guint id;
-	GIOChannel *io;
-
-	io = g_io_channel_unix_new(sk);
-	id = g_io_add_watch(io, G_IO_HUP | G_IO_ERR | G_IO_NVAL, cb, iconn);
-	g_io_channel_unref(io);
-
-	return id;
-}
-
 static gboolean fake_hid_connect(struct input_conn *iconn, GError **err)
 {
 	struct fake_hid *fhid = iconn->fake->priv;
@@ -606,8 +603,8 @@ static int hidp_add_connection(const struct input_device *idev,
 	int err;
 
 	req = g_new0(struct hidp_connadd_req, 1);
-	req->ctrl_sock = iconn->ctrl_sk;
-	req->intr_sock = iconn->intr_sk;
+	req->ctrl_sock = g_io_channel_unix_get_fd(iconn->ctrl_io);
+	req->intr_sock = g_io_channel_unix_get_fd(iconn->intr_io);
 	req->flags     = 0;
 	req->idle_to   = iconn->timeout;
 
@@ -633,7 +630,7 @@ static int hidp_add_connection(const struct input_device *idev,
 		fake->connect = fake_hid_connect;
 		fake->disconnect = fake_hid_disconnect;
 		fake->priv = fake_hid;
-		err = fake_hid_connadd(fake, iconn->intr_sk, fake_hid);
+		err = fake_hid_connadd(fake, iconn->intr_io, fake_hid);
 		goto cleanup;
 	}
 
@@ -657,7 +654,8 @@ static int hidp_add_connection(const struct input_device *idev,
 
 	if (req->vendor == 0x054c && req->product == 0x0268) {
 		unsigned char buf[] = { 0x53, 0xf4,  0x42, 0x03, 0x00, 0x00 };
-		err = write(iconn->ctrl_sk, buf, sizeof(buf));
+		int sk = g_io_channel_unix_get_fd(iconn->ctrl_io);
+		err = write(sk, buf, sizeof(buf));
 	}
 
 	err = ioctl_connadd(req);
@@ -670,6 +668,30 @@ cleanup:
 	return err;
 }
 
+static int input_device_connected(struct input_device *idev,
+						struct input_conn *iconn)
+{
+	dbus_bool_t connected;
+	int err;
+
+	err = hidp_add_connection(idev, iconn);
+	if (err < 0)
+		return err;
+
+	iconn->intr_watch = g_io_add_watch(iconn->intr_io,
+					G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+					intr_watch_cb, iconn);
+	iconn->ctrl_watch = g_io_add_watch(iconn->ctrl_io,
+					G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+					ctrl_watch_cb, iconn);
+
+	connected = TRUE;
+	emit_property_changed(idev->conn, idev->path, INPUT_DEVICE_INTERFACE,
+				"Connected", DBUS_TYPE_BOOLEAN, &connected);
+
+	return 0;
+}
+
 static void interrupt_connect_cb(GIOChannel *chan, GError *conn_err,
 							gpointer user_data)
 {
@@ -681,37 +703,38 @@ static void interrupt_connect_cb(GIOChannel *chan, GError *conn_err,
 
 	if (conn_err) {
 		err_msg = conn_err->message;
+		g_io_channel_unref(iconn->intr_io);
+		iconn->intr_io = NULL;
 		goto failed;
 	}
 
-	err = hidp_add_connection(idev, iconn);
+	err = input_device_connected(idev, iconn);
 	if (err < 0) {
 		err_msg = strerror(-err);
-		g_io_channel_shutdown(chan, TRUE, NULL);
 		goto failed;
 	}
 
-	iconn->intr_sk = g_io_channel_unix_get_fd(chan);
-
-	iconn->intr_watch = create_watch(iconn->intr_sk, intr_watch_cb, iconn);
-	iconn->ctrl_watch = create_watch(iconn->ctrl_sk, ctrl_watch_cb, iconn);
-
 	/* Replying to the requestor */
 	g_dbus_send_reply(idev->conn, iconn->pending_connect, DBUS_TYPE_INVALID);
 
-	goto cleanup;
+	dbus_message_unref(iconn->pending_connect);
+	iconn->pending_connect = NULL;
+
+	return;
 
 failed:
 	error("%s", err_msg);
 	reply = connection_attempt_failed(iconn->pending_connect, err_msg);
 	g_dbus_send_message(idev->conn, reply);
 
-	iconn->intr_sk = -1;
-	iconn->ctrl_sk = -1;
+	if (iconn->ctrl_io)
+		g_io_channel_shutdown(iconn->ctrl_io, FALSE, NULL);
 
-cleanup:
-	dbus_message_unref(iconn->pending_connect);
-	iconn->pending_connect = NULL;
+	if (iconn->intr_io) {
+		g_io_channel_shutdown(iconn->intr_io, FALSE, NULL);
+		g_io_channel_unref(iconn->intr_io);
+		iconn->intr_io = NULL;
+	}
 }
 
 static void control_connect_cb(GIOChannel *chan, GError *conn_err,
@@ -746,10 +769,7 @@ static void control_connect_cb(GIOChannel *chan, GError *conn_err,
 		goto failed;
 	}
 
-	g_io_channel_unref(io);
-
-	/* Set HID control channel */
-	iconn->ctrl_sk = g_io_channel_unix_get_fd(chan);
+	iconn->intr_io = io;
 
 	return;
 
@@ -827,14 +847,10 @@ static int connection_disconnect(struct input_conn *iconn, uint32_t flags)
 	}
 
 	/* Standard HID disconnect */
-	if (iconn->ctrl_sk >= 0) {
-		close(iconn->ctrl_sk);
-		iconn->ctrl_sk = -1;
-	}
-	if (iconn->intr_sk >= 0) {
-		close(iconn->intr_sk);
-		iconn->intr_sk = -1;
-	}
+	if (iconn->intr_io)
+		g_io_channel_shutdown(iconn->intr_io, TRUE, NULL);
+	if (iconn->ctrl_io)
+		g_io_channel_shutdown(iconn->ctrl_io, TRUE, NULL);
 
 	ctl = socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HIDP);
 	if (ctl < 0) {
@@ -928,8 +944,7 @@ static DBusMessage *device_connect(DBusConnection *conn,
 					BT_IO_OPT_DEST_BDADDR, &idev->dst,
 					BT_IO_OPT_PSM, L2CAP_PSM_HIDP_CTRL,
 					BT_IO_OPT_INVALID);
-		if (io)
-			g_io_channel_unref(io);
+		iconn->ctrl_io = io;
 	}
 
 	if (err == NULL)
@@ -1065,8 +1080,6 @@ static struct input_conn *input_conn_new(struct input_device *idev,
 	struct input_conn *iconn;
 
 	iconn = g_new0(struct input_conn, 1);
-	iconn->ctrl_sk = -1;
-	iconn->intr_sk = -1;
 	iconn->timeout = timeout;
 	iconn->uuid = g_strdup(uuid);
 	iconn->alias = g_strdup(alias);
@@ -1170,7 +1183,8 @@ int input_device_unregister(const char *path, const char *uuid)
 	return 0;
 }
 
-int input_device_set_channel(const bdaddr_t *src, const bdaddr_t *dst, int psm, int nsk)
+int input_device_set_channel(const bdaddr_t *src, const bdaddr_t *dst, int psm,
+								GIOChannel *io)
 {
 	struct input_device *idev = find_device(src, dst);
 	struct input_conn *iconn;
@@ -1184,10 +1198,10 @@ int input_device_set_channel(const bdaddr_t *src, const bdaddr_t *dst, int psm,
 
 	switch (psm) {
 	case L2CAP_PSM_HIDP_CTRL:
-		iconn->ctrl_sk = nsk;
+		iconn->ctrl_io = g_io_channel_ref(io);
 		break;
 	case L2CAP_PSM_HIDP_INTR:
-		iconn->intr_sk = nsk;
+		iconn->intr_io = g_io_channel_ref(io);
 		break;
 	}
 
@@ -1206,15 +1220,11 @@ int input_device_close_channels(const bdaddr_t *src, const bdaddr_t *dst)
 	if (!iconn)
 		return -ENOENT;
 
-	if (iconn->ctrl_sk >= 0) {
-		close(iconn->ctrl_sk);
-		iconn->ctrl_sk = -1;
-	}
+	if (iconn->intr_io)
+		g_io_channel_shutdown(iconn->intr_io, TRUE, NULL);
 
-	if (iconn->intr_sk >= 0) {
-		close(iconn->intr_sk);
-		iconn->intr_sk = -1;
-	}
+	if (iconn->ctrl_io)
+		g_io_channel_shutdown(iconn->ctrl_io, TRUE, NULL);
 
 	return 0;
 }
@@ -1224,7 +1234,6 @@ int input_device_connadd(const bdaddr_t *src, const bdaddr_t *dst)
 	struct input_device *idev;
 	struct input_conn *iconn;
 	int err;
-	gboolean connected;
 
 	idev = find_device(src, dst);
 	if (!idev)
@@ -1234,23 +1243,23 @@ int input_device_connadd(const bdaddr_t *src, const bdaddr_t *dst)
 	if (!iconn)
 		return -ENOENT;
 
-	err = hidp_add_connection(idev, iconn);
+	err = input_device_connected(idev, iconn);
 	if (err < 0)
 		goto error;
 
-	iconn->intr_watch = create_watch(iconn->intr_sk, intr_watch_cb, iconn);
-	iconn->ctrl_watch = create_watch(iconn->ctrl_sk, ctrl_watch_cb, iconn);
-	connected = TRUE;
-	emit_property_changed(idev->conn, idev->path, INPUT_DEVICE_INTERFACE,
-				"Connected", DBUS_TYPE_BOOLEAN, &connected);
-
 	return 0;
 
 error:
-	close(iconn->ctrl_sk);
-	close(iconn->intr_sk);
-	iconn->ctrl_sk = -1;
-	iconn->intr_sk = -1;
+	if (iconn->ctrl_io) {
+		g_io_channel_shutdown(iconn->ctrl_io, FALSE, NULL);
+		g_io_channel_unref(iconn->ctrl_io);
+		iconn->ctrl_io = NULL;
+	}
+	if (iconn->intr_io) {
+		g_io_channel_shutdown(iconn->intr_io, FALSE, NULL);
+		g_io_channel_unref(iconn->intr_io);
+		iconn->intr_io = NULL;
+	}
 
 	return err;
 }
diff --git a/input/device.h b/input/device.h
index cb305d62..237b09af 100644
--- a/input/device.h
+++ b/input/device.h
@@ -48,6 +48,7 @@ int fake_input_register(DBusConnection *conn, const char *path, bdaddr_t *src,
 			bdaddr_t *dst, const char *uuid, uint8_t channel);
 int input_device_unregister(const char *path, const char *uuid);
 
-int input_device_set_channel(const bdaddr_t *src, const bdaddr_t *dst, int psm, int nsk);
+int input_device_set_channel(const bdaddr_t *src, const bdaddr_t *dst, int psm,
+							GIOChannel *io);
 int input_device_close_channels(const bdaddr_t *src, const bdaddr_t *dst);
 int input_device_connadd(const bdaddr_t *src, const bdaddr_t *dst);
diff --git a/input/fakehid.c b/input/fakehid.c
index 077dc21f..207f78d5 100644
--- a/input/fakehid.c
+++ b/input/fakehid.c
@@ -366,7 +366,7 @@ struct fake_hid *get_fake_hid(uint16_t vendor, uint16_t product)
 	return NULL;
 }
 
-int fake_hid_connadd(struct fake_input *fake, int intr_sk,
+int fake_hid_connadd(struct fake_input *fake, GIOChannel *intr_io,
 						struct fake_hid *fake_hid)
 {
 	if (fake_hid->setup_uinput(fake, fake_hid)) {
@@ -374,7 +374,7 @@ int fake_hid_connadd(struct fake_input *fake, int intr_sk,
 		return ENOMEM;
 	}
 
-	fake->io = g_io_channel_unix_new(intr_sk);
+	fake->io = g_io_channel_ref(intr_io);
 	g_io_channel_set_close_on_unref(fake->io, TRUE);
 	g_io_add_watch(fake->io, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
 					(GIOFunc) fake_hid->event, fake);
diff --git a/input/fakehid.h b/input/fakehid.h
index e61a7275..90aacb48 100644
--- a/input/fakehid.h
+++ b/input/fakehid.h
@@ -35,5 +35,5 @@ struct fake_hid {
 
 struct fake_hid *get_fake_hid(uint16_t vendor, uint16_t product);
 
-int fake_hid_connadd(struct fake_input *fake, int intr_sk,
+int fake_hid_connadd(struct fake_input *fake, GIOChannel *intr_io,
 						struct fake_hid *fake_hid);
diff --git a/input/server.c b/input/server.c
index 6d08804b..041f302c 100644
--- a/input/server.c
+++ b/input/server.c
@@ -89,7 +89,6 @@ static int authorize_device(const bdaddr_t *src, const bdaddr_t *dst)
 
 static void connect_event_cb(GIOChannel *chan, GError *err, gpointer data)
 {
-	int sk;
 	uint16_t psm;
 	bdaddr_t src, dst;
 	GError *gerr = NULL;
@@ -113,13 +112,11 @@ static void connect_event_cb(GIOChannel *chan, GError *err, gpointer data)
 
 	debug("Incoming connection on PSM %d", psm);
 
-	sk = g_io_channel_unix_get_fd(chan);
-
-	if (input_device_set_channel(&src, &dst, psm, sk) < 0) {
+	if (input_device_set_channel(&src, &dst, psm, chan) < 0) {
 		/* Send unplug virtual cable to unknown devices */
 		if (psm == L2CAP_PSM_HIDP_CTRL) {
 			unsigned char unplug[] = { 0x15 };
-			int err;
+			int err, sk = g_io_channel_unix_get_fd(chan);
 			err = write(sk, unplug, sizeof(unplug));
 		}
 		g_io_channel_shutdown(chan, TRUE, NULL);