[PATCH_v6 1/5] android/pan: shutdown io channel on disconnect call

[PATCH_v6 1/5] android/pan: shutdown io channel on disconnect call

[Ravi kumar Veeramally]

Shutdown io channel and send DISCONNECTING notification and send
DISCONNECTED notification and free the device on callback.
---
 android/pan.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/android/pan.c b/android/pan.c
index b83f534..187953b 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -284,14 +284,16 @@ static void bt_pan_disconnect(const void *buf, uint16_t len)
 	}
 
 	dev = l->data;
-	if (dev->watch) {
-		g_source_remove(dev->watch);
-		dev->watch = 0;
+
+	if (dev->io)
+		g_io_channel_shutdown(dev->io, TRUE, NULL);
+
+	if (dev->conn_state == HAL_PAN_STATE_CONNECTED) {
+		bnep_if_down(dev->iface);
+		bnep_conndel(&dst);
 	}
 
-	bnep_if_down(dev->iface);
-	bnep_conndel(&dst);
-	bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
+	bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTING);
 	status = HAL_STATUS_SUCCESS;
 
 failed:
-- 
1.8.3.2

[PATCH_v6 2/5] bnep: Rename struct bnep_conn to struct bnep for better readability

[Ravi kumar Veeramally]

---
 profiles/network/bnep.c | 80 ++++++++++++++++++++++++-------------------------
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 08037e6..02e2647 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -67,7 +67,7 @@ struct __service_16 {
 	uint16_t src;
 } __attribute__ ((packed));
 
-struct bnep_conn {
+struct bnep {
 	GIOChannel	*io;
 	uint16_t	src;
 	uint16_t	dst;
@@ -77,17 +77,17 @@ struct bnep_conn {
 	bnep_connect_cb	conn_cb;
 };
 
-static void free_bnep_connect(struct bnep_conn *bc)
+static void free_bnep_connect(struct bnep *b)
 {
-	if (!bc)
+	if (!b)
 		return;
 
-	if (bc->io) {
-		g_io_channel_unref(bc->io);
-		bc->io = NULL;
+	if (b->io) {
+		g_io_channel_unref(b->io);
+		b->io = NULL;
 	}
 
-	g_free(bc);
+	g_free(b);
 }
 
 uint16_t bnep_service_id(const char *svc)
@@ -249,7 +249,7 @@ int bnep_if_down(const char *devname)
 static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
 								gpointer data)
 {
-	struct bnep_conn *bc = data;
+	struct bnep *b = data;
 	struct bnep_control_rsp *rsp;
 	struct timeval timeo;
 	char pkt[BNEP_MTU];
@@ -260,9 +260,9 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
 	if (cond & G_IO_NVAL)
 		goto failed;
 
-	if (bc->setup_to > 0) {
-		g_source_remove(bc->setup_to);
-		bc->setup_to = 0;
+	if (b->setup_to > 0) {
+		g_source_remove(b->setup_to);
+		b->setup_to = 0;
 	}
 
 	if (cond & (G_IO_HUP | G_IO_ERR)) {
@@ -309,8 +309,8 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
 	timeo.tv_sec = 0;
 	setsockopt(sk, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo));
 
-	sk = g_io_channel_unix_get_fd(bc->io);
-	if (bnep_connadd(sk, bc->src, iface)) {
+	sk = g_io_channel_unix_get_fd(b->io);
+	if (bnep_connadd(sk, b->src, iface)) {
 		error("bnep conn could not be added");
 		goto failed;
 	}
@@ -320,19 +320,19 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
 		goto failed;
 	}
 
-	bc->conn_cb(chan, iface, 0, bc->data);
-	free_bnep_connect(bc);
+	b->conn_cb(chan, iface, 0, b->data);
+	free_bnep_connect(b);
 
 	return FALSE;
 
 failed:
-	bc->conn_cb(NULL, NULL, -EIO, bc->data);
-	free_bnep_connect(bc);
+	b->conn_cb(NULL, NULL, -EIO, b->data);
+	free_bnep_connect(b);
 
 	return FALSE;
 }
 
-static int bnep_setup_conn_req(struct bnep_conn *bc)
+static int bnep_setup_conn_req(struct bnep *b)
 {
 	struct bnep_setup_conn_req *req;
 	struct __service_16 *s;
@@ -345,34 +345,34 @@ static int bnep_setup_conn_req(struct bnep_conn *bc)
 	req->ctrl = BNEP_SETUP_CONN_REQ;
 	req->uuid_size = 2;     /* 16bit UUID */
 	s = (void *) req->service;
-	s->src = htons(bc->src);
-	s->dst = htons(bc->dst);
+	s->src = htons(b->src);
+	s->dst = htons(b->dst);
 
-	fd = g_io_channel_unix_get_fd(bc->io);
+	fd = g_io_channel_unix_get_fd(b->io);
 	if (write(fd, pkt, sizeof(*req) + sizeof(*s)) < 0) {
 		error("bnep connection req send failed: %s", strerror(errno));
 		return -errno;
 	}
 
-	bc->attempts++;
+	b->attempts++;
 
 	return 0;
 }
 
 static gboolean bnep_conn_req_to(gpointer user_data)
 {
-	struct bnep_conn *bc = user_data;
+	struct bnep *b = user_data;
 
-	if (bc->attempts == CON_SETUP_RETRIES) {
+	if (b->attempts == CON_SETUP_RETRIES) {
 		error("Too many bnep connection attempts");
 	} else {
 		error("bnep connection setup TO, retrying...");
-		if (bnep_setup_conn_req(bc) == 0)
+		if (bnep_setup_conn_req(b) == 0)
 			return TRUE;
 	}
 
-	bc->conn_cb(NULL, NULL, -ETIMEDOUT, bc->data);
-	free_bnep_connect(bc);
+	b->conn_cb(NULL, NULL, -ETIMEDOUT, b->data);
+	free_bnep_connect(b);
 
 	return FALSE;
 }
@@ -380,28 +380,28 @@ static gboolean bnep_conn_req_to(gpointer user_data)
 int bnep_connect(int sk, uint16_t src, uint16_t dst, bnep_connect_cb conn_cb,
 								void *data)
 {
-	struct bnep_conn *bc;
+	struct bnep *b;
 	int err;
 
 	if (!conn_cb)
 		return -EINVAL;
 
-	bc = g_new0(struct bnep_conn, 1);
-	bc->io = g_io_channel_unix_new(sk);
-	bc->attempts = 0;
-	bc->src = src;
-	bc->dst = dst;
-	bc->conn_cb = conn_cb;
-	bc->data = data;
+	b = g_new0(struct bnep, 1);
+	b->io = g_io_channel_unix_new(sk);
+	b->attempts = 0;
+	b->src = src;
+	b->dst = dst;
+	b->conn_cb = conn_cb;
+	b->data = data;
 
-	err = bnep_setup_conn_req(bc);
+	err = bnep_setup_conn_req(b);
 	if (err < 0)
 		return err;
 
-	bc->setup_to = g_timeout_add_seconds(CON_SETUP_TO,
-							bnep_conn_req_to, bc);
-	g_io_add_watch(bc->io, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
-							bnep_setup_cb, bc);
+	b->setup_to = g_timeout_add_seconds(CON_SETUP_TO,
+							bnep_conn_req_to, b);
+	g_io_add_watch(b->io, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+							bnep_setup_cb, b);
 	return 0;
 }
 
-- 
1.8.3.2

Re: [PATCH_v6 2/5] bnep: Rename struct bnep_conn to struct bnep for better readability

[Luiz Augusto von Dentz]

Hi Ravi,

On Thu, Dec 19, 2013 at 4:39 PM, Ravi kumar Veeramally
<ravikumar.veeramally@linux.intel.com> wrote:
> ---
>  profiles/network/bnep.c | 80 ++++++++++++++++++++++++-------------------------
>  1 file changed, 40 insertions(+), 40 deletions(-)
>
> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> index 08037e6..02e2647 100644
> --- a/profiles/network/bnep.c
> +++ b/profiles/network/bnep.c
> @@ -67,7 +67,7 @@ struct __service_16 {
>         uint16_t src;
>  } __attribute__ ((packed));
>
> -struct bnep_conn {
> +struct bnep {
>         GIOChannel      *io;
>         uint16_t        src;
>         uint16_t        dst;
> @@ -77,17 +77,17 @@ struct bnep_conn {
>         bnep_connect_cb conn_cb;
>  };
>
> -static void free_bnep_connect(struct bnep_conn *bc)
> +static void free_bnep_connect(struct bnep *b)

I guess if you want to be consist you should name the var session here as well.


-- 
Luiz Augusto von Dentz

[PATCH_v6 3/5] bnep: Add bnep_new and bnep_free api's

[Ravi kumar Veeramally]

Refacoring connect and disconnect mechanisms. It would be more
convinient for caller to maintain just bnep connection reference
and delete whenever it is not required.
---
 profiles/network/bnep.c | 35 +++++++++++++++++++++++++++++++++++
 profiles/network/bnep.h |  4 ++++
 2 files changed, 39 insertions(+)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 02e2647..d09f369 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -73,6 +73,7 @@ struct bnep {
 	uint16_t	dst;
 	guint	attempts;
 	guint	setup_to;
+	guint	watch;
 	void	*data;
 	bnep_connect_cb	conn_cb;
 };
@@ -377,6 +378,40 @@ static gboolean bnep_conn_req_to(gpointer user_data)
 	return FALSE;
 }
 
+struct bnep *bnep_new(int sk, uint16_t local_role, uint16_t remote_role)
+{
+	struct bnep *b;
+
+	b = g_new0(struct bnep, 1);
+	b->io = g_io_channel_unix_new(sk);
+	b->src = local_role;
+	b->dst = remote_role;
+
+	b->watch = g_io_add_watch_full(b->io, G_PRIORITY_LOW,
+				G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+					(GIOFunc) bnep_setup_cb, b, NULL);
+
+	return b;
+}
+
+void bnep_free(struct bnep *b)
+{
+	if (!b)
+		return;
+
+	if (b->io) {
+		g_io_channel_unref(b->io);
+		b->io = NULL;
+	}
+
+	if (b->watch > 0) {
+		g_source_remove(b->watch);
+		b->watch = 0;
+	}
+
+	g_free(b);
+}
+
 int bnep_connect(int sk, uint16_t src, uint16_t dst, bnep_connect_cb conn_cb,
 								void *data)
 {
diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
index dd22c40..091a7f2 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -21,6 +21,8 @@
  *
  */
 
+struct bnep;
+
 int bnep_init(void);
 int bnep_cleanup(void);
 
@@ -28,6 +30,8 @@ uint16_t bnep_service_id(const char *svc);
 const char *bnep_uuid(uint16_t id);
 const char *bnep_name(uint16_t id);
 
+struct bnep *bnep_new(int sk, uint16_t local_role, uint16_t remote_role);
+void bnep_free(struct bnep *b);
 int bnep_connadd(int sk, uint16_t role, char *dev);
 int bnep_conndel(const bdaddr_t *dst);
 int bnep_if_up(const char *devname);
-- 
1.8.3.2

[PATCH_v6 4/5] bnep: Refactored bnep connect and disconnect calls

[Ravi kumar Veeramally]

Refactored bnep connect and disconnect calls to simplify and
keeping bnep related functionality behind curtains.
Provided bnep struct globally. bnep_connect calls takes
care of bnep_setup until interface up then connect callback
will be called. Provided bnep_set_disconnect. Set disconnect
when connect call succeeds. bnep_disconnect should be
called only when iface is up/connected.
---
 android/pan.c                 |  60 +++++++++++++-----------
 profiles/network/bnep.c       | 105 ++++++++++++++++++++++++++++--------------
 profiles/network/bnep.h       |   8 ++--
 profiles/network/connection.c |  60 ++++++++++++++++--------
 4 files changed, 152 insertions(+), 81 deletions(-)

diff --git a/android/pan.c b/android/pan.c
index 187953b..51a11dd 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -55,6 +55,7 @@ struct pan_device {
 	uint8_t		role;
 	GIOChannel	*io;
 	guint		watch;
+	struct bnep	*session;
 };
 
 static int device_cmp(gconstpointer s, gconstpointer user_data)
@@ -77,6 +78,7 @@ static void pan_device_free(struct pan_device *dev)
 		dev->io = NULL;
 	}
 
+	bnep_free(dev->session);
 	devices = g_slist_remove(devices, dev);
 	g_free(dev);
 
@@ -104,8 +106,10 @@ static void bt_pan_notify_conn_state(struct pan_device *dev, uint8_t state)
 
 	ipc_send_notif(HAL_SERVICE_ID_PAN, HAL_EV_PAN_CONN_STATE, sizeof(ev),
 									&ev);
-	if (dev->conn_state == HAL_PAN_STATE_DISCONNECTED)
-		pan_device_free(dev);
+	if (state != HAL_PAN_STATE_DISCONNECTED)
+		return;
+
+	pan_device_free(dev);
 }
 
 static void bt_pan_notify_ctrl_state(struct pan_device *dev, uint8_t state)
@@ -124,21 +128,27 @@ static void bt_pan_notify_ctrl_state(struct pan_device *dev, uint8_t state)
 									&ev);
 }
 
-static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
-								gpointer data)
+static gboolean watch_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
 {
 	struct pan_device *dev = data;
 
-	DBG("%s disconnected", dev->iface);
+	DBG("disconnected");
 
-	bnep_if_down(dev->iface);
-	bnep_conndel(&dev->dst);
 	bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
 
 	return FALSE;
 }
 
-static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
+static void bnep_disconn_cb(void *data)
+{
+	struct pan_device *dev = data;
+
+	DBG("%s disconnected", dev->iface);
+
+	bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
+}
+
+static void bnep_conn_cb(char *iface, int err, void *data)
 {
 	struct pan_device *dev = data;
 
@@ -146,28 +156,21 @@ static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
 
 	if (err < 0) {
 		error("bnep connect req failed: %s", strerror(-err));
-		bnep_conndel(&dev->dst);
 		bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
 		return;
 	}
 
-	memcpy(dev->iface, iface, sizeof(dev->iface));
-
-	DBG("%s connected", dev->iface);
+	DBG("%s connected", iface);
 
+	memcpy(dev->iface, iface, sizeof(dev->iface));
 	bt_pan_notify_ctrl_state(dev, HAL_PAN_CTRL_ENABLED);
 	bt_pan_notify_conn_state(dev, HAL_PAN_STATE_CONNECTED);
-
-	dev->watch = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP | G_IO_NVAL,
-							bnep_watchdog_cb, dev);
-	g_io_channel_unref(dev->io);
-	dev->io = NULL;
 }
 
 static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
 {
 	struct pan_device *dev = data;
-	uint16_t src, dst;
+	uint16_t l_role, r_role;
 	int perr, sk;
 
 	DBG("");
@@ -177,16 +180,23 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
 		goto fail;
 	}
 
-	src = (local_role == HAL_PAN_ROLE_NAP) ? BNEP_SVC_NAP : BNEP_SVC_PANU;
-	dst = (dev->role == HAL_PAN_ROLE_NAP) ? BNEP_SVC_NAP : BNEP_SVC_PANU;
+	l_role = (local_role == HAL_PAN_ROLE_NAP) ? BNEP_SVC_NAP :
+								BNEP_SVC_PANU;
+	r_role = (dev->role == HAL_PAN_ROLE_NAP) ? BNEP_SVC_NAP : BNEP_SVC_PANU;
 	sk = g_io_channel_unix_get_fd(dev->io);
-
-	perr = bnep_connect(sk, src, dst, bnep_conn_cb, dev);
+	dev->session = bnep_new(sk, l_role, r_role);
+	perr = bnep_connect(dev->session, bnep_conn_cb, dev);
 	if (perr < 0) {
 		error("bnep connect req failed: %s", strerror(-perr));
 		goto fail;
 	}
 
+	bnep_set_disconnect(dev->session, bnep_disconn_cb, dev);
+	dev->watch = g_io_add_watch(dev->io, G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+								watch_cb, dev);
+	g_io_channel_unref(dev->io);
+	dev->io = NULL;
+
 	return;
 
 fail:
@@ -288,10 +298,8 @@ static void bt_pan_disconnect(const void *buf, uint16_t len)
 	if (dev->io)
 		g_io_channel_shutdown(dev->io, TRUE, NULL);
 
-	if (dev->conn_state == HAL_PAN_STATE_CONNECTED) {
-		bnep_if_down(dev->iface);
-		bnep_conndel(&dst);
-	}
+	if (dev->conn_state == HAL_PAN_STATE_CONNECTED)
+		bnep_disconnect(dev->session);
 
 	bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTING);
 	status = HAL_STATUS_SUCCESS;
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index d09f369..a4b1670 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -39,6 +39,7 @@
 #include <bluetooth/bluetooth.h>
 #include <bluetooth/l2cap.h>
 #include <bluetooth/bnep.h>
+#include <btio/btio.h>
 
 #include <glib.h>
 
@@ -71,26 +72,18 @@ struct bnep {
 	GIOChannel	*io;
 	uint16_t	src;
 	uint16_t	dst;
+	bdaddr_t	dst_addr;
+	char	iface[16];
 	guint	attempts;
 	guint	setup_to;
 	guint	watch;
 	void	*data;
 	bnep_connect_cb	conn_cb;
+	void	*conn_data;
+	bnep_disconnect_cb disconn_cb;
+	void	*disconn_data;
 };
 
-static void free_bnep_connect(struct bnep *b)
-{
-	if (!b)
-		return;
-
-	if (b->io) {
-		g_io_channel_unref(b->io);
-		b->io = NULL;
-	}
-
-	g_free(b);
-}
-
 uint16_t bnep_service_id(const char *svc)
 {
 	int i;
@@ -247,6 +240,17 @@ int bnep_if_down(const char *devname)
 	return 0;
 }
 
+static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
+								gpointer data)
+{
+	struct bnep *b = data;
+
+	if (b->disconn_cb)
+		b->disconn_cb(b->disconn_data);
+
+	return FALSE;
+}
+
 static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
 								gpointer data)
 {
@@ -254,7 +258,6 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
 	struct bnep_control_rsp *rsp;
 	struct timeval timeo;
 	char pkt[BNEP_MTU];
-	char iface[16];
 	ssize_t r;
 	int sk;
 
@@ -311,24 +314,27 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
 	setsockopt(sk, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo));
 
 	sk = g_io_channel_unix_get_fd(b->io);
-	if (bnep_connadd(sk, b->src, iface)) {
+	if (bnep_connadd(sk, b->src, b->iface)) {
 		error("bnep conn could not be added");
 		goto failed;
 	}
 
-	if (bnep_if_up(iface)) {
-		error("could not up %s", iface);
+	if (bnep_if_up(b->iface)) {
+		error("could not up %s", b->iface);
+		bnep_conndel(&b->dst_addr);
 		goto failed;
 	}
 
-	b->conn_cb(chan, iface, 0, b->data);
-	free_bnep_connect(b);
+	b->conn_cb(b->iface, 0, b->conn_data);
+	b->watch = g_io_add_watch(b->io, G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+						(GIOFunc) bnep_watchdog_cb, b);
+	g_io_channel_unref(b->io);
+	b->io = NULL;
 
 	return FALSE;
 
 failed:
-	b->conn_cb(NULL, NULL, -EIO, b->data);
-	free_bnep_connect(b);
+	b->conn_cb(NULL, -EIO, b->conn_data);
 
 	return FALSE;
 }
@@ -372,8 +378,7 @@ static gboolean bnep_conn_req_to(gpointer user_data)
 			return TRUE;
 	}
 
-	b->conn_cb(NULL, NULL, -ETIMEDOUT, b->data);
-	free_bnep_connect(b);
+	b->conn_cb(NULL, -ETIMEDOUT, b->conn_data);
 
 	return FALSE;
 }
@@ -412,22 +417,25 @@ void bnep_free(struct bnep *b)
 	g_free(b);
 }
 
-int bnep_connect(int sk, uint16_t src, uint16_t dst, bnep_connect_cb conn_cb,
-								void *data)
+int bnep_connect(struct bnep *b, bnep_connect_cb conn_cb, void *data)
 {
-	struct bnep *b;
+	GError *gerr = NULL;
 	int err;
 
-	if (!conn_cb)
+	if (!b || !conn_cb)
 		return -EINVAL;
 
-	b = g_new0(struct bnep, 1);
-	b->io = g_io_channel_unix_new(sk);
 	b->attempts = 0;
-	b->src = src;
-	b->dst = dst;
 	b->conn_cb = conn_cb;
-	b->data = data;
+	b->conn_data = data;
+
+	bt_io_get(b->io, &gerr, BT_IO_OPT_DEST_BDADDR, &b->dst_addr,
+							BT_IO_OPT_INVALID);
+	if (gerr) {
+		error("%s", gerr->message);
+		g_error_free(gerr);
+		return -EINVAL;
+	}
 
 	err = bnep_setup_conn_req(b);
 	if (err < 0)
@@ -435,11 +443,40 @@ int bnep_connect(int sk, uint16_t src, uint16_t dst, bnep_connect_cb conn_cb,
 
 	b->setup_to = g_timeout_add_seconds(CON_SETUP_TO,
 							bnep_conn_req_to, b);
-	g_io_add_watch(b->io, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
-							bnep_setup_cb, b);
 	return 0;
 }
 
+void bnep_disconnect(struct bnep *b)
+{
+	if (!b)
+		return;
+
+	if (b->watch > 0) {
+		g_source_remove(b->watch);
+		b->watch = 0;
+	}
+
+	if (b->io) {
+		g_io_channel_unref(b->io);
+		b->io = NULL;
+	}
+
+	bnep_if_down(b->iface);
+	bnep_conndel(&b->dst_addr);
+}
+
+void bnep_set_disconnect(struct bnep *b, bnep_disconnect_cb disconn_cb,
+								void *data)
+{
+	if (!b || !disconn_cb)
+		return;
+
+	if (!b->disconn_cb && !b->disconn_data) {
+		b->disconn_cb = disconn_cb;
+		b->disconn_data = data;
+	}
+}
+
 int bnep_add_to_bridge(const char *devname, const char *bridge)
 {
 	int ifindex;
diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
index 091a7f2..cff5c78 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -39,10 +39,12 @@ int bnep_if_down(const char *devname);
 int bnep_add_to_bridge(const char *devname, const char *bridge);
 int bnep_del_from_bridge(const char *devname, const char *bridge);
 
-typedef void (*bnep_connect_cb) (GIOChannel *chan, char *iface, int err,
-								void *data);
-int bnep_connect(int sk, uint16_t src, uint16_t dst, bnep_connect_cb conn_cb,
+typedef void (*bnep_connect_cb) (char *iface, int err, void *data);
+typedef void (*bnep_disconnect_cb) (void *data);
+int bnep_connect(struct bnep *b, bnep_connect_cb conn_cb, void *data);
+void bnep_set_disconnect(struct bnep *b, bnep_disconnect_cb disconn_cb,
 								void *data);
+void bnep_disconnect(struct bnep *b);
 
 ssize_t bnep_send_ctrl_rsp(int sk, uint8_t type, uint8_t ctrl, uint16_t resp);
 uint16_t bnep_setup_chk(uint16_t dst_role, uint16_t src_role);
diff --git a/profiles/network/connection.c b/profiles/network/connection.c
index fb3e1ce..4539972 100644
--- a/profiles/network/connection.c
+++ b/profiles/network/connection.c
@@ -70,8 +70,10 @@ struct network_conn {
 	conn_state	state;
 	GIOChannel	*io;
 	guint		dc_id;
+	guint		watch;
 	struct network_peer *peer;
 	DBusMessage	*connect;
+	struct bnep	*session;
 };
 
 static GSList *peers = NULL;
@@ -106,8 +108,7 @@ static struct network_conn *find_connection_by_state(GSList *list,
 	return NULL;
 }
 
-static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
-				gpointer data)
+static void bnep_disconn_cb(gpointer data)
 {
 	struct network_conn *nc = data;
 	DBusConnection *conn = btd_get_dbus_connection();
@@ -126,11 +127,23 @@ static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
 
 	info("%s disconnected", nc->dev);
 
-	bnep_if_down(nc->dev);
 	nc->state = DISCONNECTED;
 	memset(nc->dev, 0, sizeof(nc->dev));
 	strcpy(nc->dev, "bnep%d");
 
+	bnep_free(nc->session);
+	nc->session = NULL;
+
+	if (nc->io) {
+		g_io_channel_unref(nc->io);
+		nc->io = NULL;
+	}
+}
+
+static gboolean watch_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
+{
+	bnep_disconn_cb(data);
+
 	return FALSE;
 }
 
@@ -155,12 +168,25 @@ static void local_connect_cb(struct network_conn *nc, int err)
 static void cancel_connection(struct network_conn *nc, int err)
 {
 	btd_service_connecting_complete(nc->service, err);
+
 	if (nc->connect)
 		local_connect_cb(nc, err);
 
-	g_io_channel_shutdown(nc->io, TRUE, NULL);
-	g_io_channel_unref(nc->io);
-	nc->io = NULL;
+	if (nc->watch > 0) {
+		g_source_remove(nc->watch);
+		nc->watch = 0;
+	}
+
+	if (nc->io) {
+		g_io_channel_unref(nc->io);
+		nc->io = NULL;
+	}
+
+	if (nc->state == CONNECTED)
+		bnep_disconnect(nc->session);
+
+	bnep_free(nc->session);
+	nc->session = NULL;
 
 	nc->state = DISCONNECTED;
 }
@@ -169,11 +195,7 @@ static void connection_destroy(DBusConnection *conn, void *user_data)
 {
 	struct network_conn *nc = user_data;
 
-	if (nc->state == CONNECTED) {
-		bnep_if_down(nc->dev);
-		bnep_conndel(device_get_address(nc->peer->device));
-	} else if (nc->io)
-		cancel_connection(nc, -EIO);
+	cancel_connection(nc, -EIO);
 }
 
 static void disconnect_cb(struct btd_device *device, gboolean removal,
@@ -186,7 +208,7 @@ static void disconnect_cb(struct btd_device *device, gboolean removal,
 	connection_destroy(NULL, user_data);
 }
 
-static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
+static void bnep_conn_cb(char *iface, int err, void *data)
 {
 	struct network_conn *nc = data;
 	const char *path;
@@ -220,11 +242,6 @@ static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
 	nc->state = CONNECTED;
 	nc->dc_id = device_add_disconnect_watch(nc->peer->device, disconnect_cb,
 								nc, NULL);
-	g_io_add_watch(chan, G_IO_ERR | G_IO_HUP | G_IO_NVAL,
-							bnep_watchdog_cb, nc);
-	g_io_channel_unref(nc->io);
-	nc->io = NULL;
-
 	return;
 
 failed:
@@ -242,12 +259,19 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
 	}
 
 	sk = g_io_channel_unix_get_fd(nc->io);
-	perr = bnep_connect(sk, BNEP_SVC_PANU, nc->id, bnep_conn_cb, nc);
+	nc->session = bnep_new(sk, BNEP_SVC_PANU, nc->id);
+	perr = bnep_connect(nc->session, bnep_conn_cb, nc);
 	if (perr < 0) {
 		error("bnep connect(): %s (%d)", strerror(-perr), -perr);
 		goto failed;
 	}
 
+	bnep_set_disconnect(nc->session, bnep_disconn_cb, nc);
+	nc->watch = g_io_add_watch(nc->io, G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+								watch_cb, nc);
+	g_io_channel_unref(nc->io);
+	nc->io = NULL;
+
 	return;
 
 failed:
-- 
1.8.3.2

Re: [PATCH_v6 4/5] bnep: Refactored bnep connect and disconnect calls

[Luiz Augusto von Dentz]

Hi Ravi,

On Thu, Dec 19, 2013 at 4:39 PM, Ravi kumar Veeramally
<ravikumar.veeramally@linux.intel.com> wrote:
> Refactored bnep connect and disconnect calls to simplify and
> keeping bnep related functionality behind curtains.
> Provided bnep struct globally. bnep_connect calls takes
> care of bnep_setup until interface up then connect callback
> will be called. Provided bnep_set_disconnect. Set disconnect
> when connect call succeeds. bnep_disconnect should be
> called only when iface is up/connected.
> ---
>  android/pan.c                 |  60 +++++++++++++-----------
>  profiles/network/bnep.c       | 105 ++++++++++++++++++++++++++++--------------
>  profiles/network/bnep.h       |   8 ++--
>  profiles/network/connection.c |  60 ++++++++++++++++--------
>  4 files changed, 152 insertions(+), 81 deletions(-)
>
> diff --git a/android/pan.c b/android/pan.c
> index 187953b..51a11dd 100644
> --- a/android/pan.c
> +++ b/android/pan.c
> @@ -55,6 +55,7 @@ struct pan_device {
>         uint8_t         role;
>         GIOChannel      *io;
>         guint           watch;
> +       struct bnep     *session;
>  };
>
>  static int device_cmp(gconstpointer s, gconstpointer user_data)
> @@ -77,6 +78,7 @@ static void pan_device_free(struct pan_device *dev)
>                 dev->io = NULL;
>         }
>
> +       bnep_free(dev->session);
>         devices = g_slist_remove(devices, dev);
>         g_free(dev);
>
> @@ -104,8 +106,10 @@ static void bt_pan_notify_conn_state(struct pan_device *dev, uint8_t state)
>
>         ipc_send_notif(HAL_SERVICE_ID_PAN, HAL_EV_PAN_CONN_STATE, sizeof(ev),
>                                                                         &ev);
> -       if (dev->conn_state == HAL_PAN_STATE_DISCONNECTED)
> -               pan_device_free(dev);
> +       if (state != HAL_PAN_STATE_DISCONNECTED)
> +               return;
> +
> +       pan_device_free(dev);
>  }

This change is completely unrelated and probably doesn't make much sense.

>  static void bt_pan_notify_ctrl_state(struct pan_device *dev, uint8_t state)
> @@ -124,21 +128,27 @@ static void bt_pan_notify_ctrl_state(struct pan_device *dev, uint8_t state)
>                                                                         &ev);
>  }
>
> -static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
> -                                                               gpointer data)
> +static gboolean watch_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
>  {
>         struct pan_device *dev = data;
>
> -       DBG("%s disconnected", dev->iface);
> +       DBG("disconnected");
>
> -       bnep_if_down(dev->iface);
> -       bnep_conndel(&dev->dst);
>         bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
>
>         return FALSE;
>  }
>
> -static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
> +static void bnep_disconn_cb(void *data)
> +{
> +       struct pan_device *dev = data;
> +
> +       DBG("%s disconnected", dev->iface);
> +
> +       bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
> +}

There does seems to have 2 watches for the exact same thing.

> +static void bnep_conn_cb(char *iface, int err, void *data)
>  {
>         struct pan_device *dev = data;
>
> @@ -146,28 +156,21 @@ static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
>
>         if (err < 0) {
>                 error("bnep connect req failed: %s", strerror(-err));
> -               bnep_conndel(&dev->dst);
>                 bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
>                 return;
>         }
>
> -       memcpy(dev->iface, iface, sizeof(dev->iface));
> -
> -       DBG("%s connected", dev->iface);
> +       DBG("%s connected", iface);
>
> +       memcpy(dev->iface, iface, sizeof(dev->iface));
>         bt_pan_notify_ctrl_state(dev, HAL_PAN_CTRL_ENABLED);
>         bt_pan_notify_conn_state(dev, HAL_PAN_STATE_CONNECTED);
> -
> -       dev->watch = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP | G_IO_NVAL,
> -                                                       bnep_watchdog_cb, dev);
> -       g_io_channel_unref(dev->io);
> -       dev->io = NULL;
>  }
>
>  static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
>  {
>         struct pan_device *dev = data;
> -       uint16_t src, dst;
> +       uint16_t l_role, r_role;
>         int perr, sk;
>
>         DBG("");
> @@ -177,16 +180,23 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
>                 goto fail;
>         }
>
> -       src = (local_role == HAL_PAN_ROLE_NAP) ? BNEP_SVC_NAP : BNEP_SVC_PANU;
> -       dst = (dev->role == HAL_PAN_ROLE_NAP) ? BNEP_SVC_NAP : BNEP_SVC_PANU;
> +       l_role = (local_role == HAL_PAN_ROLE_NAP) ? BNEP_SVC_NAP :
> +                                                               BNEP_SVC_PANU;
> +       r_role = (dev->role == HAL_PAN_ROLE_NAP) ? BNEP_SVC_NAP : BNEP_SVC_PANU;
>         sk = g_io_channel_unix_get_fd(dev->io);
> -
> -       perr = bnep_connect(sk, src, dst, bnep_conn_cb, dev);
> +       dev->session = bnep_new(sk, l_role, r_role);
> +       perr = bnep_connect(dev->session, bnep_conn_cb, dev);
>         if (perr < 0) {
>                 error("bnep connect req failed: %s", strerror(-perr));
>                 goto fail;
>         }
>
> +       bnep_set_disconnect(dev->session, bnep_disconn_cb, dev);
> +       dev->watch = g_io_add_watch(dev->io, G_IO_HUP | G_IO_ERR | G_IO_NVAL,
> +                                                               watch_cb, dev);
> +       g_io_channel_unref(dev->io);
> +       dev->io = NULL;

You probably don't need the dev->watch anymore.

>         return;
>
>  fail:
> @@ -288,10 +298,8 @@ static void bt_pan_disconnect(const void *buf, uint16_t len)
>         if (dev->io)
>                 g_io_channel_shutdown(dev->io, TRUE, NULL);
>
> -       if (dev->conn_state == HAL_PAN_STATE_CONNECTED) {
> -               bnep_if_down(dev->iface);
> -               bnep_conndel(&dst);
> -       }
> +       if (dev->conn_state == HAL_PAN_STATE_CONNECTED)
> +               bnep_disconnect(dev->session);
>
>         bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTING);
>         status = HAL_STATUS_SUCCESS;
> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> index d09f369..a4b1670 100644
> --- a/profiles/network/bnep.c
> +++ b/profiles/network/bnep.c
> @@ -39,6 +39,7 @@
>  #include <bluetooth/bluetooth.h>
>  #include <bluetooth/l2cap.h>
>  #include <bluetooth/bnep.h>
> +#include <btio/btio.h>
>
>  #include <glib.h>
>
> @@ -71,26 +72,18 @@ struct bnep {
>         GIOChannel      *io;
>         uint16_t        src;
>         uint16_t        dst;
> +       bdaddr_t        dst_addr;
> +       char    iface[16];
>         guint   attempts;
>         guint   setup_to;
>         guint   watch;
>         void    *data;
>         bnep_connect_cb conn_cb;
> +       void    *conn_data;
> +       bnep_disconnect_cb disconn_cb;
> +       void    *disconn_data;
>  };
>
> -static void free_bnep_connect(struct bnep *b)
> -{
> -       if (!b)
> -               return;
> -
> -       if (b->io) {
> -               g_io_channel_unref(b->io);
> -               b->io = NULL;
> -       }
> -
> -       g_free(b);
> -}
> -
>  uint16_t bnep_service_id(const char *svc)
>  {
>         int i;
> @@ -247,6 +240,17 @@ int bnep_if_down(const char *devname)
>         return 0;
>  }
>
> +static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
> +                                                               gpointer data)
> +{
> +       struct bnep *b = data;
> +
> +       if (b->disconn_cb)
> +               b->disconn_cb(b->disconn_data);
> +
> +       return FALSE;
> +}
> +
>  static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
>                                                                 gpointer data)
>  {
> @@ -254,7 +258,6 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
>         struct bnep_control_rsp *rsp;
>         struct timeval timeo;
>         char pkt[BNEP_MTU];
> -       char iface[16];
>         ssize_t r;
>         int sk;
>
> @@ -311,24 +314,27 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
>         setsockopt(sk, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo));
>
>         sk = g_io_channel_unix_get_fd(b->io);
> -       if (bnep_connadd(sk, b->src, iface)) {
> +       if (bnep_connadd(sk, b->src, b->iface)) {
>                 error("bnep conn could not be added");
>                 goto failed;
>         }
>
> -       if (bnep_if_up(iface)) {
> -               error("could not up %s", iface);
> +       if (bnep_if_up(b->iface)) {
> +               error("could not up %s", b->iface);
> +               bnep_conndel(&b->dst_addr);
>                 goto failed;
>         }
>
> -       b->conn_cb(chan, iface, 0, b->data);
> -       free_bnep_connect(b);
> +       b->conn_cb(b->iface, 0, b->conn_data);
> +       b->watch = g_io_add_watch(b->io, G_IO_ERR | G_IO_HUP | G_IO_NVAL,
> +                                               (GIOFunc) bnep_watchdog_cb, b);
> +       g_io_channel_unref(b->io);
> +       b->io = NULL;
>
>         return FALSE;
>
>  failed:
> -       b->conn_cb(NULL, NULL, -EIO, b->data);
> -       free_bnep_connect(b);
> +       b->conn_cb(NULL, -EIO, b->conn_data);
>
>         return FALSE;
>  }
> @@ -372,8 +378,7 @@ static gboolean bnep_conn_req_to(gpointer user_data)
>                         return TRUE;
>         }
>
> -       b->conn_cb(NULL, NULL, -ETIMEDOUT, b->data);
> -       free_bnep_connect(b);
> +       b->conn_cb(NULL, -ETIMEDOUT, b->conn_data);
>
>         return FALSE;
>  }
> @@ -412,22 +417,25 @@ void bnep_free(struct bnep *b)
>         g_free(b);
>  }
>
> -int bnep_connect(int sk, uint16_t src, uint16_t dst, bnep_connect_cb conn_cb,
> -                                                               void *data)
> +int bnep_connect(struct bnep *b, bnep_connect_cb conn_cb, void *data)
>  {
> -       struct bnep *b;
> +       GError *gerr = NULL;
>         int err;
>
> -       if (!conn_cb)
> +       if (!b || !conn_cb)
>                 return -EINVAL;
>
> -       b = g_new0(struct bnep, 1);
> -       b->io = g_io_channel_unix_new(sk);
>         b->attempts = 0;
> -       b->src = src;
> -       b->dst = dst;
>         b->conn_cb = conn_cb;
> -       b->data = data;
> +       b->conn_data = data;
> +
> +       bt_io_get(b->io, &gerr, BT_IO_OPT_DEST_BDADDR, &b->dst_addr,
> +                                                       BT_IO_OPT_INVALID);
> +       if (gerr) {
> +               error("%s", gerr->message);
> +               g_error_free(gerr);
> +               return -EINVAL;
> +       }
>
>         err = bnep_setup_conn_req(b);
>         if (err < 0)
> @@ -435,11 +443,40 @@ int bnep_connect(int sk, uint16_t src, uint16_t dst, bnep_connect_cb conn_cb,
>
>         b->setup_to = g_timeout_add_seconds(CON_SETUP_TO,
>                                                         bnep_conn_req_to, b);
> -       g_io_add_watch(b->io, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
> -                                                       bnep_setup_cb, b);
>         return 0;
>  }
>
> +void bnep_disconnect(struct bnep *b)
> +{
> +       if (!b)
> +               return;
> +
> +       if (b->watch > 0) {
> +               g_source_remove(b->watch);
> +               b->watch = 0;
> +       }
> +
> +       if (b->io) {
> +               g_io_channel_unref(b->io);
> +               b->io = NULL;
> +       }
> +
> +       bnep_if_down(b->iface);
> +       bnep_conndel(&b->dst_addr);
> +}
> +
> +void bnep_set_disconnect(struct bnep *b, bnep_disconnect_cb disconn_cb,
> +                                                               void *data)
> +{
> +       if (!b || !disconn_cb)
> +               return;
> +
> +       if (!b->disconn_cb && !b->disconn_data) {
> +               b->disconn_cb = disconn_cb;
> +               b->disconn_data = data;
> +       }
> +}
> +
>  int bnep_add_to_bridge(const char *devname, const char *bridge)
>  {
>         int ifindex;
> diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
> index 091a7f2..cff5c78 100644
> --- a/profiles/network/bnep.h
> +++ b/profiles/network/bnep.h
> @@ -39,10 +39,12 @@ int bnep_if_down(const char *devname);
>  int bnep_add_to_bridge(const char *devname, const char *bridge);
>  int bnep_del_from_bridge(const char *devname, const char *bridge);
>
> -typedef void (*bnep_connect_cb) (GIOChannel *chan, char *iface, int err,
> -                                                               void *data);
> -int bnep_connect(int sk, uint16_t src, uint16_t dst, bnep_connect_cb conn_cb,
> +typedef void (*bnep_connect_cb) (char *iface, int err, void *data);
> +typedef void (*bnep_disconnect_cb) (void *data);
> +int bnep_connect(struct bnep *b, bnep_connect_cb conn_cb, void *data);
> +void bnep_set_disconnect(struct bnep *b, bnep_disconnect_cb disconn_cb,
>                                                                 void *data);
> +void bnep_disconnect(struct bnep *b);
>
>  ssize_t bnep_send_ctrl_rsp(int sk, uint8_t type, uint8_t ctrl, uint16_t resp);
>  uint16_t bnep_setup_chk(uint16_t dst_role, uint16_t src_role);
> diff --git a/profiles/network/connection.c b/profiles/network/connection.c
> index fb3e1ce..4539972 100644
> --- a/profiles/network/connection.c
> +++ b/profiles/network/connection.c
> @@ -70,8 +70,10 @@ struct network_conn {
>         conn_state      state;
>         GIOChannel      *io;
>         guint           dc_id;
> +       guint           watch;
>         struct network_peer *peer;
>         DBusMessage     *connect;
> +       struct bnep     *session;
>  };
>
>  static GSList *peers = NULL;
> @@ -106,8 +108,7 @@ static struct network_conn *find_connection_by_state(GSList *list,
>         return NULL;
>  }
>
> -static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
> -                               gpointer data)
> +static void bnep_disconn_cb(gpointer data)
>  {
>         struct network_conn *nc = data;
>         DBusConnection *conn = btd_get_dbus_connection();
> @@ -126,11 +127,23 @@ static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
>
>         info("%s disconnected", nc->dev);
>
> -       bnep_if_down(nc->dev);
>         nc->state = DISCONNECTED;
>         memset(nc->dev, 0, sizeof(nc->dev));
>         strcpy(nc->dev, "bnep%d");
>
> +       bnep_free(nc->session);
> +       nc->session = NULL;
> +
> +       if (nc->io) {
> +               g_io_channel_unref(nc->io);
> +               nc->io = NULL;
> +       }
> +}
> +
> +static gboolean watch_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
> +{
> +       bnep_disconn_cb(data);
> +
>         return FALSE;
>  }
>
> @@ -155,12 +168,25 @@ static void local_connect_cb(struct network_conn *nc, int err)
>  static void cancel_connection(struct network_conn *nc, int err)
>  {
>         btd_service_connecting_complete(nc->service, err);
> +
>         if (nc->connect)
>                 local_connect_cb(nc, err);
>
> -       g_io_channel_shutdown(nc->io, TRUE, NULL);
> -       g_io_channel_unref(nc->io);
> -       nc->io = NULL;
> +       if (nc->watch > 0) {
> +               g_source_remove(nc->watch);
> +               nc->watch = 0;
> +       }
> +
> +       if (nc->io) {
> +               g_io_channel_unref(nc->io);
> +               nc->io = NULL;
> +       }
> +
> +       if (nc->state == CONNECTED)
> +               bnep_disconnect(nc->session);
> +
> +       bnep_free(nc->session);
> +       nc->session = NULL;
>
>         nc->state = DISCONNECTED;
>  }
> @@ -169,11 +195,7 @@ static void connection_destroy(DBusConnection *conn, void *user_data)
>  {
>         struct network_conn *nc = user_data;
>
> -       if (nc->state == CONNECTED) {
> -               bnep_if_down(nc->dev);
> -               bnep_conndel(device_get_address(nc->peer->device));
> -       } else if (nc->io)
> -               cancel_connection(nc, -EIO);
> +       cancel_connection(nc, -EIO);
>  }
>
>  static void disconnect_cb(struct btd_device *device, gboolean removal,
> @@ -186,7 +208,7 @@ static void disconnect_cb(struct btd_device *device, gboolean removal,
>         connection_destroy(NULL, user_data);
>  }
>
> -static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
> +static void bnep_conn_cb(char *iface, int err, void *data)
>  {
>         struct network_conn *nc = data;
>         const char *path;
> @@ -220,11 +242,6 @@ static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
>         nc->state = CONNECTED;
>         nc->dc_id = device_add_disconnect_watch(nc->peer->device, disconnect_cb,
>                                                                 nc, NULL);
> -       g_io_add_watch(chan, G_IO_ERR | G_IO_HUP | G_IO_NVAL,
> -                                                       bnep_watchdog_cb, nc);
> -       g_io_channel_unref(nc->io);
> -       nc->io = NULL;
> -
>         return;
>
>  failed:
> @@ -242,12 +259,19 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
>         }
>
>         sk = g_io_channel_unix_get_fd(nc->io);
> -       perr = bnep_connect(sk, BNEP_SVC_PANU, nc->id, bnep_conn_cb, nc);
> +       nc->session = bnep_new(sk, BNEP_SVC_PANU, nc->id);
> +       perr = bnep_connect(nc->session, bnep_conn_cb, nc);
>         if (perr < 0) {
>                 error("bnep connect(): %s (%d)", strerror(-perr), -perr);
>                 goto failed;
>         }
>
> +       bnep_set_disconnect(nc->session, bnep_disconn_cb, nc);
> +       nc->watch = g_io_add_watch(nc->io, G_IO_HUP | G_IO_ERR | G_IO_NVAL,
> +                                                               watch_cb, nc);
> +       g_io_channel_unref(nc->io);
> +       nc->io = NULL;
> +
>         return;
>
>  failed:
> --
> 1.8.3.2

This patch is packing a lot of changes perhaps you should consider splitting.


-- 
Luiz Augusto von Dentz

[PATCH_v6 5/5] bnep: Refactored bnep server apis for bridge addition and deletion

[Ravi kumar Veeramally]

To simplify bnep server realted bridge creation and deletion calls
provided extra apis and moved existing apis to static.
---
 profiles/network/bnep.c   | 50 +++++++++++++++++++++++++++++++++++++++++------
 profiles/network/bnep.h   | 11 ++++-------
 profiles/network/server.c | 37 +++--------------------------------
 3 files changed, 51 insertions(+), 47 deletions(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index a4b1670..9e93949 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -154,7 +154,7 @@ int bnep_cleanup(void)
 	return 0;
 }
 
-int bnep_conndel(const bdaddr_t *dst)
+static int bnep_conndel(const bdaddr_t *dst)
 {
 	struct bnep_conndel_req req;
 
@@ -170,7 +170,7 @@ int bnep_conndel(const bdaddr_t *dst)
 	return 0;
 }
 
-int bnep_connadd(int sk, uint16_t role, char *dev)
+static int bnep_connadd(int sk, uint16_t role, char *dev)
 {
 	struct bnep_connadd_req req;
 
@@ -190,7 +190,7 @@ int bnep_connadd(int sk, uint16_t role, char *dev)
 	return 0;
 }
 
-int bnep_if_up(const char *devname)
+static int bnep_if_up(const char *devname)
 {
 	struct ifreq ifr;
 	int sk, err;
@@ -215,7 +215,7 @@ int bnep_if_up(const char *devname)
 	return 0;
 }
 
-int bnep_if_down(const char *devname)
+static int bnep_if_down(const char *devname)
 {
 	struct ifreq ifr;
 	int sk, err;
@@ -477,7 +477,7 @@ void bnep_set_disconnect(struct bnep *b, bnep_disconnect_cb disconn_cb,
 	}
 }
 
-int bnep_add_to_bridge(const char *devname, const char *bridge)
+static int bnep_add_to_bridge(const char *devname, const char *bridge)
 {
 	int ifindex;
 	struct ifreq ifr;
@@ -508,7 +508,7 @@ int bnep_add_to_bridge(const char *devname, const char *bridge)
 	return 0;
 }
 
-int bnep_del_from_bridge(const char *devname, const char *bridge)
+static int bnep_del_from_bridge(const char *devname, const char *bridge)
 {
 	int ifindex = if_nametoindex(devname);
 	struct ifreq ifr;
@@ -537,6 +537,44 @@ int bnep_del_from_bridge(const char *devname, const char *bridge)
 	return 0;
 }
 
+int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
+						const bdaddr_t *addr)
+{
+	if (!bridge || !bridge || !iface || !addr)
+		return -EINVAL;
+
+	if (bnep_connadd(sk, dst, iface) < 0) {
+		error("Can't add connection to the bridge %s: %s(%d)",
+						bridge, strerror(errno), errno);
+		return -errno;
+	}
+
+	if (bnep_add_to_bridge(iface, bridge) < 0) {
+		error("Can't add %s to the bridge %s: %s(%d)",
+					iface, bridge, strerror(errno), errno);
+		bnep_conndel(addr);
+		return -errno;
+	}
+
+	if (bnep_if_up(iface) < 0) {
+		error("Can't up the interface %s: %s(%d)",
+						iface, strerror(errno), errno);
+		return -errno;
+	}
+
+	return 0;
+}
+
+void bnep_server_delete(char *bridge, char *iface, const bdaddr_t *addr)
+{
+	if (!bridge || !iface || !addr)
+		return;
+
+	bnep_del_from_bridge(iface, bridge);
+	bnep_if_down(iface);
+	bnep_conndel(addr);
+}
+
 ssize_t bnep_send_ctrl_rsp(int sk, uint8_t type, uint8_t ctrl, uint16_t resp)
 {
 	struct bnep_control_rsp rsp;
diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
index cff5c78..ea28c2f 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -32,13 +32,6 @@ const char *bnep_name(uint16_t id);
 
 struct bnep *bnep_new(int sk, uint16_t local_role, uint16_t remote_role);
 void bnep_free(struct bnep *b);
-int bnep_connadd(int sk, uint16_t role, char *dev);
-int bnep_conndel(const bdaddr_t *dst);
-int bnep_if_up(const char *devname);
-int bnep_if_down(const char *devname);
-int bnep_add_to_bridge(const char *devname, const char *bridge);
-int bnep_del_from_bridge(const char *devname, const char *bridge);
-
 typedef void (*bnep_connect_cb) (char *iface, int err, void *data);
 typedef void (*bnep_disconnect_cb) (void *data);
 int bnep_connect(struct bnep *b, bnep_connect_cb conn_cb, void *data);
@@ -46,6 +39,10 @@ void bnep_set_disconnect(struct bnep *b, bnep_disconnect_cb disconn_cb,
 								void *data);
 void bnep_disconnect(struct bnep *b);
 
+int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
+							const bdaddr_t *addr);
+void bnep_server_delete(char *bridge, char *iface, const bdaddr_t *addr);
+
 ssize_t bnep_send_ctrl_rsp(int sk, uint8_t type, uint8_t ctrl, uint16_t resp);
 uint16_t bnep_setup_chk(uint16_t dst_role, uint16_t src_role);
 uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 73741ec..7cb5a1e 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -251,35 +251,6 @@ static sdp_record_t *server_record_new(const char *name, uint16_t id)
 	return record;
 }
 
-static int server_connadd(struct network_server *ns,
-				struct network_session *session,
-				uint16_t dst_role)
-{
-	char devname[16];
-	int err, nsk;
-
-	nsk = g_io_channel_unix_get_fd(session->io);
-	err = bnep_connadd(nsk, dst_role, devname);
-	if (err < 0)
-		return err;
-
-	info("Added new connection: %s", devname);
-
-	if (bnep_add_to_bridge(devname, ns->bridge) < 0) {
-		error("Can't add %s to the bridge %s: %s(%d)",
-				devname, ns->bridge, strerror(errno), errno);
-		return -EPERM;
-	}
-
-	bnep_if_up(devname);
-
-	strncpy(session->dev, devname, sizeof(devname));
-
-	ns->sessions = g_slist_append(ns->sessions, session);
-
-	return 0;
-}
-
 static void session_free(void *data)
 {
 	struct network_session *session = data;
@@ -377,7 +348,8 @@ static gboolean bnep_setup(GIOChannel *chan,
 		goto reply;
 	}
 
-	if (server_connadd(ns, na->setup, dst_role) < 0)
+	if (bnep_server_add(sk, dst_role, ns->bridge, na->setup->dev,
+							&na->setup->dst) < 0)
 		goto reply;
 
 	na->setup = NULL;
@@ -524,10 +496,7 @@ static void server_remove_sessions(struct network_server *ns)
 		if (*session->dev == '\0')
 			continue;
 
-		bnep_del_from_bridge(session->dev, ns->bridge);
-		bnep_if_down(session->dev);
-
-		bnep_conndel(&session->dst);
+		bnep_server_delete(ns->bridge, session->dev, &session->dst);
 	}
 
 	g_slist_free_full(ns->sessions, session_free);
-- 
1.8.3.2