Cleanup/fix glib-ectomy.c and its usage

8 files changed, 68 insertions, 37 deletions

diff --git a/common/glib-ectomy.c b/common/glib-ectomy.c
index 41641ee6..33d20533 100644
--- a/common/glib-ectomy.c
+++ b/common/glib-ectomy.c
@@ -24,6 +24,11 @@ struct timeout {
 	GSourceFunc function;
 };
 
+struct _GIOChannel {
+	int fd;
+	gboolean close_on_unref;
+};
+
 struct _GMainContext {
 	guint next_id;
 	glong next_timeout;
@@ -82,8 +87,17 @@ void g_io_channel_close(GIOChannel *channel)
 		return;
 
 	close(channel->fd);
+	channel->fd = -1;
+}
+
+void g_io_channel_unref(GIOChannel *channel)
+{
+	if (!channel)
+		return;
+
+	if (channel->close_on_unref && channel->fd >= 0)
+		g_io_channel_close(channel);
 
-	memset(channel, 0, sizeof(channel));
 	free(channel);
 }
 
@@ -95,11 +109,18 @@ GIOChannel *g_io_channel_unix_new(int fd)
 	if (!channel)
 		return NULL;
 
+	memset(channel, 0, sizeof(GIOChannel));
+
 	channel->fd = fd;
 
 	return channel;
 }
 
+void g_io_channel_set_close_on_unref(GIOChannel *channel, gboolean do_close)
+{
+	channel->close_on_unref = do_close;
+}
+
 gint g_io_channel_unix_get_fd(GIOChannel *channel)
 {
 	return channel->fd;
diff --git a/common/glib-ectomy.h b/common/glib-ectomy.h
index 16f4810d..2885f019 100644
--- a/common/glib-ectomy.h
+++ b/common/glib-ectomy.h
@@ -30,9 +30,7 @@ typedef ssize_t	gssize;
 
 #define MIN_TIMEOUT(a, b)  (((a) < (b)) ? (a) : (b))
 
-typedef struct _GIOChannel {
-	int fd;
-} GIOChannel;
+typedef struct _GIOChannel GIOChannel;
 
 typedef gboolean (*GSourceFunc) (gpointer data);
 
@@ -78,6 +76,8 @@ GIOError g_io_channel_read(GIOChannel *channel, gchar *buf, gsize count, gsize *
 void g_io_channel_close(GIOChannel *channel);
 
 GIOChannel *g_io_channel_unix_new(int fd);
+void g_io_channel_unref(GIOChannel *channel);
+void g_io_channel_set_close_on_unref(GIOChannel *channel, gboolean do_close);
 gint g_io_channel_unix_get_fd(GIOChannel *channel);
 guint g_io_add_watch(GIOChannel *channel, GIOCondition condition,
 					GIOFunc func, gpointer user_data);
diff --git a/hcid/dbus-rfcomm.c b/hcid/dbus-rfcomm.c
index ebeeb462..0fba2226 100644
--- a/hcid/dbus-rfcomm.c
+++ b/hcid/dbus-rfcomm.c
@@ -88,8 +88,8 @@ static char *rfcomm_node_name_from_id(int16_t id, char *dev, size_t len)
 static void rfcomm_node_free(struct rfcomm_node *node)
 {
 	if (node->io) {
-		g_io_channel_close(node->io);
 		g_io_remove_watch(node->io_id);
+		g_io_channel_unref(node->io);
 	}
 	if (node->owner)
 		free(node->owner);
@@ -131,7 +131,7 @@ static void pending_connect_free(struct pending_connect *c)
 	if (c->svc)
 		free(c->svc);
 	if (c->io)
-		g_io_channel_close(c->io);
+		g_io_channel_unref(c->io);
 	if (c->msg)
 		dbus_message_unref(c->msg);
 	if (c->conn)
@@ -174,8 +174,8 @@ static int rfcomm_release(struct rfcomm_node *node, int *err)
 	debug("rfcomm_release(%s)", node->name);
 
 	if (node->io) {
-		g_io_channel_close(node->io);
 		g_io_remove_watch(node->io_id);
+		g_io_channel_unref(node->io);
 		node->io = NULL;
 	}
 
@@ -310,6 +310,7 @@ static gboolean rfcomm_connect_cb(GIOChannel *chan, GIOCondition cond,
 	}
 
         node->io = g_io_channel_unix_new(fd);
+	g_io_channel_set_close_on_unref(node->io, TRUE);
 	node->io_id = g_io_add_watch(node->io, G_IO_ERR | G_IO_HUP,
 					(GIOFunc) rfcomm_disconnect_cb, node);
 
@@ -388,6 +389,7 @@ static int rfcomm_connect(DBusConnection *conn, DBusMessage *msg, bdaddr_t *src,
 	c->conn = dbus_connection_ref(conn);
 
 	c->io = g_io_channel_unix_new(sk);
+	g_io_channel_set_close_on_unref(c->io, TRUE);
 
 	if (connect(sk, (struct sockaddr *) &c->raddr, sizeof(c->raddr)) < 0) {
 		/* BlueZ returns EAGAIN eventhough it should return EINPROGRESS */
diff --git a/hcid/dbus-sdp.c b/hcid/dbus-sdp.c
index 2a448af4..c378b5b5 100644
--- a/hcid/dbus-sdp.c
+++ b/hcid/dbus-sdp.c
@@ -506,7 +506,7 @@ static gboolean search_session_handle_cb(GIOChannel *chan, GIOCondition cond, vo
 		return TRUE;
 
 fail:
-	g_io_channel_close(chan);
+	g_io_channel_unref(chan);
 	return retval;
 }
 
@@ -671,6 +671,7 @@ static int search_request(DBusConnection *conn, DBusMessage *msg, uint16_t dev_i
 	}
 
 	chan = g_io_channel_unix_new(sk);
+	g_io_channel_set_close_on_unref(chan, TRUE);
 
 	sa.l2_family = AF_BLUETOOTH;
 	sa.l2_psm = 0;
@@ -712,7 +713,7 @@ static int search_request(DBusConnection *conn, DBusMessage *msg, uint16_t dev_i
 	return 0;
 fail:
 	if (chan)
-		g_io_channel_close(chan);
+		g_io_channel_unref(chan);
 
 	if (c)
 		pending_connect_free(c);
diff --git a/hcid/dbus.c b/hcid/dbus.c
index fc778757..b9d5d905 100644
--- a/hcid/dbus.c
+++ b/hcid/dbus.c
@@ -56,12 +56,16 @@ static int experimental = 0;
 #define RECONNECT_RETRY_TIMEOUT		5000
 #define DISPATCH_TIMEOUT		0
 
-typedef struct
-{
+typedef struct {
 	uint32_t id;
 	DBusTimeout *timeout;
 } timeout_handler_t;
 
+struct watch_info {
+	guint watch_id;
+	GIOChannel *io;
+};
+
 void hcid_dbus_set_experimental(void)
 {
 	experimental = 1;
@@ -1420,7 +1424,7 @@ static gboolean message_dispatch_cb(void *data)
 
 static gboolean watch_func(GIOChannel *chan, GIOCondition cond, gpointer data)
 {
-	DBusWatch *watch = (DBusWatch *) data;
+	DBusWatch *watch = data;
 	int flags = 0;
 
 	if (cond & G_IO_IN)  flags |= DBUS_WATCH_READABLE;
@@ -1439,40 +1443,40 @@ static gboolean watch_func(GIOChannel *chan, GIOCondition cond, gpointer data)
 static dbus_bool_t add_watch(DBusWatch *watch, void *data)
 {
 	GIOCondition cond = G_IO_HUP | G_IO_ERR;
-	GIOChannel *io;
-	guint *id;
+	struct watch_info *info;
 	int fd, flags;
 
 	if (!dbus_watch_get_enabled(watch))
 		return TRUE;
 
-	id = malloc(sizeof(guint));
-	if (id == NULL)
+	info = malloc(sizeof(struct watch_info));
+	if (info == NULL)
 		return FALSE;
 
 	fd = dbus_watch_get_fd(watch);
-	io = g_io_channel_unix_new(fd);
+	info->io = g_io_channel_unix_new(fd);
 	flags = dbus_watch_get_flags(watch);
 
 	if (flags & DBUS_WATCH_READABLE) cond |= G_IO_IN;
 	if (flags & DBUS_WATCH_WRITABLE) cond |= G_IO_OUT;
 
-	*id = g_io_add_watch(io, cond, watch_func, watch);
+	info->watch_id = g_io_add_watch(info->io, cond, watch_func, watch);
 
-	dbus_watch_set_data(watch, id, NULL);
+	dbus_watch_set_data(watch, info, NULL);
 
 	return TRUE;
 }
 
 static void remove_watch(DBusWatch *watch, void *data)
 {
-	guint *id = dbus_watch_get_data(watch);
+	struct watch_info *info = dbus_watch_get_data(watch);
 
 	dbus_watch_set_data(watch, NULL, NULL);
 
-	if (id) {
-		g_io_remove_watch(*id);
-		free(id);
+	if (info) {
+		g_io_remove_watch(info->watch_id);
+		g_io_channel_unref(info->io);
+		free(info);
 	}
 }
 
diff --git a/hcid/main.c b/hcid/main.c
index b15634fe..dd3367b7 100644
--- a/hcid/main.c
+++ b/hcid/main.c
@@ -716,6 +716,7 @@ int main(int argc, char *argv[])
 	event_loop = g_main_new(FALSE);
 
 	ctl_io = g_io_channel_unix_new(hcid.sock);
+	g_io_channel_set_close_on_unref(ctl_io, TRUE);
 
 	g_io_add_watch(ctl_io, G_IO_IN, io_stack_event, NULL);
 
@@ -737,6 +738,8 @@ int main(int argc, char *argv[])
 
 	g_main_unref(event_loop);
 
+	g_io_channel_unref(ctl_io);
+
 	info("Exit");
 
 	stop_logging();
diff --git a/hcid/sdp.c b/hcid/sdp.c
index b1fcaf26..9cde4951 100644
--- a/hcid/sdp.c
+++ b/hcid/sdp.c
@@ -64,8 +64,10 @@ static gboolean session_event(GIOChannel *chan, GIOCondition cond, gpointer data
 	GIOError err;
 	int sk, ret;
 
-	if (cond & (G_IO_HUP | G_IO_ERR))
+	if (cond & (G_IO_HUP | G_IO_ERR)) {
+		g_io_channel_unref(chan);
 		return FALSE;
+	}
 
 	debug("Incoming SDP transaction");
 
@@ -131,8 +133,9 @@ static gboolean connect_event(GIOChannel *chan, GIOCondition cond, gpointer data
 	session_data->imtu = opts.imtu;
 
 	io = g_io_channel_unix_new(nsk);
+	g_io_channel_set_close_on_unref(io, TRUE);
 
-	g_io_add_watch_full(io, 0, G_IO_IN, session_event,
+	g_io_add_watch_full(io, 0, G_IO_IN | G_IO_HUP | G_IO_ERR, session_event,
 					session_data, session_destory);
 
 	return TRUE;
@@ -167,6 +170,7 @@ int start_sdp_server(void)
 	listen(sk, 5);
 
 	io = g_io_channel_unix_new(sk);
+	g_io_channel_set_close_on_unref(io, TRUE);
 
 	g_io_add_watch(io, G_IO_IN, connect_event, NULL);
 
diff --git a/hcid/security.c b/hcid/security.c
index b5eca651..ebdc130c 100644
--- a/hcid/security.c
+++ b/hcid/security.c
@@ -657,23 +657,21 @@ static inline void conn_request(int dev, bdaddr_t *sba, void *ptr)
 static gboolean io_security_event(GIOChannel *chan, GIOCondition cond, gpointer data)
 {
 	unsigned char buf[HCI_MAX_EVENT_SIZE], *ptr = buf;
-	struct hci_dev_info *di = (void *) data;
+	struct hci_dev_info *di = data;
 	int type, dev;
 	size_t len;
 	hci_event_hdr *eh;
 	GIOError err;
 
 	if (cond & (G_IO_NVAL | G_IO_HUP | G_IO_ERR)) {
-		g_io_channel_close(chan);
-		free(data);
+		g_io_channel_unref(chan);
 		return FALSE;
 	}
 
 	if ((err = g_io_channel_read(chan, (gchar *) buf, sizeof(buf), &len))) {
 		if (err == G_IO_ERROR_AGAIN)
 			return TRUE;
-		g_io_channel_close(chan);
-		free(data);
+		g_io_channel_unref(chan);
 		return FALSE;
 	}
 
@@ -836,9 +834,10 @@ void start_security_manager(int hdev)
 	}
 
 	chan = g_io_channel_unix_new(dev);
-	io_data[hdev].watch_id = g_io_add_watch(chan, G_IO_IN | G_IO_NVAL | G_IO_HUP | G_IO_ERR,
-						io_security_event, (void *) di);
-
+	g_io_channel_set_close_on_unref(chan, TRUE);
+	io_data[hdev].watch_id = g_io_add_watch_full(chan, 0,
+						G_IO_IN | G_IO_NVAL | G_IO_HUP | G_IO_ERR,
+						io_security_event, di, (GDestroyNotify)free);
 	io_data[hdev].channel = chan;
 	io_data[hdev].pin_length = -1;
 
@@ -861,11 +860,8 @@ void stop_security_manager(int hdev)
 
 	info("Stopping security manager %d", hdev);
 
-	/* this is a bit sneaky. closing the fd will cause the event
-	   loop to call us right back with G_IO_NVAL set, at which
-	   point we will see it and clean things up */
-	close(g_io_channel_unix_get_fd(chan));
 	g_io_remove_watch(io_data[hdev].watch_id);
+	g_io_channel_unref(io_data[hdev].channel);
 	io_data[hdev].watch_id = -1;
 	io_data[hdev].channel = NULL;
 	io_data[hdev].pin_length = -1;