[PATCH 01/12] profiles/network: Minor style fix

[PATCH 01/12] profiles/network: Minor style fix

[Grzegorz Kolodziejczyk]

This patch fixes style issues e.g. line aligns, braces.
---
 profiles/network/bnep.c   |  6 ++++--
 profiles/network/server.c | 11 +++++------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 09d4b65..e3a2b89 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -508,8 +508,9 @@ static int bnep_add_to_bridge(const char *devname, const char *bridge)
 		err = -errno;
 		error("bnep: Can't add %s to the bridge %s: %s(%d)",
 					devname, bridge, strerror(-err), -err);
-	} else
+	} else {
 		info("bridge %s: interface %s added", bridge, devname);
+	}
 
 	close(sk);
 
@@ -539,8 +540,9 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
 		err = -errno;
 		error("bnep: Can't delete %s from the bridge %s: %s(%d)",
 					devname, bridge, strerror(-err), -err);
-	} else
+	} else {
 		info("bridge %s: interface %s removed", bridge, devname);
+	}
 
 	close(sk);
 
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 3fb031f..ebbe056 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -650,8 +650,7 @@ static struct network_adapter *create_adapter(struct btd_adapter *adapter)
 	na = g_new0(struct network_adapter, 1);
 	na->adapter = btd_adapter_ref(adapter);
 
-	na->io = bt_io_listen(NULL, confirm_event, na,
-				NULL, &err,
+	na->io = bt_io_listen(NULL, confirm_event, na, NULL, &err,
 				BT_IO_OPT_SOURCE_BDADDR,
 				btd_adapter_get_address(adapter),
 				BT_IO_OPT_PSM, BNEP_PSM,
@@ -697,10 +696,10 @@ int server_register(struct btd_adapter *adapter, uint16_t id)
 	if (g_slist_length(na->servers) > 0)
 		goto done;
 
-	if (!g_dbus_register_interface(btd_get_dbus_connection(),
-					path, NETWORK_SERVER_INTERFACE,
-					server_methods, NULL, NULL,
-					na, path_unregister)) {
+	if (!g_dbus_register_interface(btd_get_dbus_connection(), path,
+						NETWORK_SERVER_INTERFACE,
+						server_methods, NULL, NULL, na,
+						path_unregister)) {
 		error("D-Bus failed to register %s interface",
 						NETWORK_SERVER_INTERFACE);
 		server_free(ns);
-- 
2.1.0

[PATCH 02/12] profiles/network: Simplify id and bridge of server checking

[Grzegorz Kolodziejczyk]

This patch simplifies checking of server record id and bridge, if those
exists.
---
 profiles/network/server.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/profiles/network/server.c b/profiles/network/server.c
index ebbe056..2b252d9 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -434,13 +434,7 @@ static void confirm_event(GIOChannel *chan, gpointer user_data)
 	}
 
 	ns = find_server(na->servers, BNEP_SVC_NAP);
-	if (!ns)
-		goto drop;
-
-	if (!ns->record_id)
-		goto drop;
-
-	if (!ns->bridge)
+	if (!ns || !ns->record_id || !ns->bridge)
 		goto drop;
 
 	na->setup = g_new0(struct network_session, 1);
-- 
2.1.0

[PATCH 03/12] profiles/network: Integrate get and check bnep setup services roles

[Grzegorz Kolodziejczyk]

While bnep setup, roles must be determined and checked if the connection
scanario is possible. Get and check possible service roles routines are
always performed in serie. So there is no need to split it to separate
calls.
---
 android/pan.c             |  8 +-------
 profiles/network/bnep.c   | 37 ++++++++++++++++---------------------
 profiles/network/bnep.h   |  1 -
 profiles/network/server.c |  6 +-----
 4 files changed, 18 insertions(+), 34 deletions(-)

diff --git a/android/pan.c b/android/pan.c
index 972af89..0bf5f71 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -497,17 +497,11 @@ static gboolean nap_setup_cb(GIOChannel *chan, GIOCondition cond,
 	}
 
 	rsp = bnep_setup_decode(req, &dst_role, &src_role);
-	if (rsp) {
+	if (rsp != BNEP_SUCCESS) {
 		error("bnep_setup_decode failed");
 		goto failed;
 	}
 
-	rsp = bnep_setup_chk(dst_role, src_role);
-	if (rsp) {
-		error("benp_setup_chk failed");
-		goto failed;
-	}
-
 	err = nap_create_bridge();
 	if (err < 0) {
 		error("pan: Failed to create bridge: %s (%d)", strerror(-err),
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index e3a2b89..fe13a5b 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -591,26 +591,6 @@ ssize_t bnep_send_ctrl_rsp(int sk, uint8_t type, uint8_t ctrl, uint16_t resp)
 	return send(sk, &rsp, sizeof(rsp), 0);
 }
 
-uint16_t bnep_setup_chk(uint16_t dst, uint16_t src)
-{
-	/* Allowed PAN Profile scenarios */
-	switch (dst) {
-	case BNEP_SVC_NAP:
-	case BNEP_SVC_GN:
-		if (src == BNEP_SVC_PANU)
-			return 0;
-		return BNEP_CONN_INVALID_SRC;
-	case BNEP_SVC_PANU:
-		if (src == BNEP_SVC_PANU ||  src == BNEP_SVC_GN ||
-							src == BNEP_SVC_NAP)
-			return 0;
-
-		return BNEP_CONN_INVALID_SRC;
-	}
-
-	return BNEP_CONN_INVALID_DST;
-}
-
 uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
 								uint16_t *src)
 {
@@ -655,5 +635,20 @@ uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
 		return BNEP_CONN_INVALID_SVC;
 	}
 
-	return BNEP_SUCCESS;
+	/* Allowed PAN Profile scenarios */
+	switch (*dst) {
+	case BNEP_SVC_NAP:
+	case BNEP_SVC_GN:
+		if (*src == BNEP_SVC_PANU)
+			return BNEP_SUCCESS;
+		return BNEP_CONN_INVALID_SRC;
+	case BNEP_SVC_PANU:
+		if (*src == BNEP_SVC_PANU || *src == BNEP_SVC_GN ||
+							*src == BNEP_SVC_NAP)
+			return BNEP_SUCCESS;
+
+		return BNEP_CONN_INVALID_SRC;
+	}
+
+	return BNEP_CONN_INVALID_DST;
 }
diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
index bc43d4f..5aedf38 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -46,6 +46,5 @@ int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
 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,
 								uint16_t *src);
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 2b252d9..12266c9 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -323,11 +323,7 @@ static gboolean bnep_setup(GIOChannel *chan,
 		return FALSE;
 
 	rsp = bnep_setup_decode(req, &dst_role, &src_role);
-	if (rsp)
-		goto reply;
-
-	rsp = bnep_setup_chk(dst_role, src_role);
-	if (rsp)
+	if (rsp != BNEP_SUCCESS)
 		goto reply;
 
 	rsp = BNEP_CONN_NOT_ALLOWED;
-- 
2.1.0

[PATCH 04/12] profiles/network: Move bnep connection setup logic to bnep

[Grzegorz Kolodziejczyk]

BNEP connection set up logic which was added before bnep_server_add,
can be private method of bnep. Moved logic was almost doubled in two
cases: NAP role in PAN, server listening. Now set up and connect
scenario check of bnep connection is only handled in bnep part for
listen connections.
---
 android/pan.c             |  28 ++--------
 profiles/network/bnep.c   | 127 ++++++++++++++++++++++++++++------------------
 profiles/network/bnep.h   |   6 +--
 profiles/network/server.c |  43 ++++++++--------
 4 files changed, 105 insertions(+), 99 deletions(-)

diff --git a/android/pan.c b/android/pan.c
index 0bf5f71..8bafcd0 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -463,8 +463,7 @@ static gboolean nap_setup_cb(GIOChannel *chan, GIOCondition cond,
 {
 	struct pan_device *dev = user_data;
 	uint8_t packet[BNEP_MTU];
-	struct bnep_setup_conn_req *req = (void *) packet;
-	uint16_t src_role, dst_role, rsp = BNEP_CONN_NOT_ALLOWED;
+	uint16_t rsp = BNEP_CONN_NOT_ALLOWED;
 	int sk, n, err;
 
 	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
@@ -481,27 +480,6 @@ static gboolean nap_setup_cb(GIOChannel *chan, GIOCondition cond,
 		goto failed;
 	}
 
-	/* Highest known control command id BNEP_FILTER_MULT_ADDR_RSP 0x06 */
-	if (req->type == BNEP_CONTROL &&
-					req->ctrl > BNEP_FILTER_MULT_ADDR_RSP) {
-		error("cmd not understood");
-		bnep_send_ctrl_rsp(sk, BNEP_CONTROL, BNEP_CMD_NOT_UNDERSTOOD,
-								req->ctrl);
-		goto failed;
-	}
-
-	if (req->type != BNEP_CONTROL || req->ctrl != BNEP_SETUP_CONN_REQ) {
-		error("cmd is not BNEP_SETUP_CONN_REQ %02X %02X", req->type,
-								req->ctrl);
-		goto failed;
-	}
-
-	rsp = bnep_setup_decode(req, &dst_role, &src_role);
-	if (rsp != BNEP_SUCCESS) {
-		error("bnep_setup_decode failed");
-		goto failed;
-	}
-
 	err = nap_create_bridge();
 	if (err < 0) {
 		error("pan: Failed to create bridge: %s (%d)", strerror(-err),
@@ -509,8 +487,8 @@ static gboolean nap_setup_cb(GIOChannel *chan, GIOCondition cond,
 		goto failed;
 	}
 
-	if (bnep_server_add(sk, dst_role, BNEP_BRIDGE, dev->iface,
-							&dev->dst) < 0) {
+	if (bnep_server_add(sk, BNEP_BRIDGE, dev->iface, &dev->dst,
+							(void *) packet) < 0) {
 		nap_remove_bridge();
 		error("server_connadd failed");
 		rsp = BNEP_CONN_NOT_ALLOWED;
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index fe13a5b..7177972 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -549,63 +549,25 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
 	return err;
 }
 
-int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
-						const bdaddr_t *addr)
-{
-	int err;
-
-	if (!bridge || !iface || !addr)
-		return -EINVAL;
-
-	err = bnep_connadd(sk, dst, iface);
-	if (err < 0)
-		return err;
-
-	err = bnep_add_to_bridge(iface, bridge);
-	if (err < 0) {
-		bnep_conndel(addr);
-		return err;
-	}
-
-	return bnep_if_up(iface);
-}
-
-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;
-
-	rsp.type = type;
-	rsp.ctrl = ctrl;
-	rsp.resp = htons(resp);
-
-	return send(sk, &rsp, sizeof(rsp), 0);
-}
-
-uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
-								uint16_t *src)
+static uint16_t bnep_setup_decode(int sk, struct bnep_setup_conn_req *req,
+								uint16_t *dst)
 {
 	const uint8_t bt_base[] = { 0x00, 0x00, 0x10, 0x00, 0x80, 0x00,
 					0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB };
+	uint16_t src;
 	uint8_t *dest, *source;
 	uint32_t val;
 
+	if (req->type != BNEP_CONTROL || req->ctrl != BNEP_SETUP_CONN_REQ)
+		return BNEP_CONN_NOT_ALLOWED;
+
 	dest = req->service;
 	source = req->service + req->uuid_size;
 
 	switch (req->uuid_size) {
 	case 2: /* UUID16 */
 		*dst = get_be16(dest);
-		*src = get_be16(source);
+		src = get_be16(source);
 		break;
 	case 16: /* UUID128 */
 		/* Check that the bytes in the UUID, except the service ID
@@ -629,7 +591,7 @@ uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
 		if (val > 0xffff)
 			return BNEP_CONN_INVALID_SRC;
 
-		*src = val;
+		src = val;
 		break;
 	default:
 		return BNEP_CONN_INVALID_SVC;
@@ -639,12 +601,13 @@ uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
 	switch (*dst) {
 	case BNEP_SVC_NAP:
 	case BNEP_SVC_GN:
-		if (*src == BNEP_SVC_PANU)
+		if (src == BNEP_SVC_PANU)
 			return BNEP_SUCCESS;
+
 		return BNEP_CONN_INVALID_SRC;
 	case BNEP_SVC_PANU:
-		if (*src == BNEP_SVC_PANU || *src == BNEP_SVC_GN ||
-							*src == BNEP_SVC_NAP)
+		if (src == BNEP_SVC_PANU || src == BNEP_SVC_GN ||
+							src == BNEP_SVC_NAP)
 			return BNEP_SUCCESS;
 
 		return BNEP_CONN_INVALID_SRC;
@@ -652,3 +615,69 @@ uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
 
 	return BNEP_CONN_INVALID_DST;
 }
+
+int bnep_server_add(int sk, char *bridge, char *iface, const bdaddr_t *addr,
+							void *setup_data)
+{
+	int err;
+	uint16_t dst = NULL;
+	struct bnep_setup_conn_req *req = setup_data;
+
+	/* Highest known Control command ID
+	 * is BNEP_FILTER_MULT_ADDR_RSP = 0x06 */
+	if (req->type == BNEP_CONTROL &&
+				req->ctrl > BNEP_FILTER_MULT_ADDR_RSP) {
+		uint8_t pkt[3];
+
+		pkt[0] = BNEP_CONTROL;
+		pkt[1] = BNEP_CMD_NOT_UNDERSTOOD;
+		pkt[2] = req->ctrl;
+
+		send(sk, pkt, sizeof(pkt), 0);
+
+		return -EINVAL;
+	}
+
+	/* Processing BNEP_SETUP_CONNECTION_REQUEST_MSG */
+	err = bnep_setup_decode(sk, setup_data, &dst);
+	if (err < 0) {
+		error("error while decoding setup connection request: %d", err);
+		return -EINVAL;
+	}
+
+	if (!bridge || !iface || !addr || !dst)
+		return -EINVAL;
+
+	err = bnep_connadd(sk, dst, iface);
+	if (err < 0)
+		return err;
+
+	err = bnep_add_to_bridge(iface, bridge);
+	if (err < 0) {
+		bnep_conndel(addr);
+		return err;
+	}
+
+	return bnep_if_up(iface);
+}
+
+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;
+
+	rsp.type = type;
+	rsp.ctrl = ctrl;
+	rsp.resp = htons(resp);
+
+	return send(sk, &rsp, sizeof(rsp), 0);
+}
diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
index 5aedf38..7a3efb6 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -41,10 +41,8 @@ void bnep_set_disconnect(struct bnep *session, bnep_disconnect_cb disconn_cb,
 								void *data);
 void bnep_disconnect(struct bnep *session);
 
-int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
-							const bdaddr_t *addr);
+int bnep_server_add(int sk, char *bridge, char *iface, const bdaddr_t *addr,
+							void *setup_data);
 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_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
-								uint16_t *src);
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 12266c9..e0eb6ef 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -46,6 +46,7 @@
 #include "src/log.h"
 #include "src/error.h"
 #include "src/sdpd.h"
+#include "src/shared/util.h"
 
 #include "bnep.h"
 #include "server.h"
@@ -280,11 +281,14 @@ static void setup_destroy(void *user_data)
 static gboolean bnep_setup(GIOChannel *chan,
 			GIOCondition cond, gpointer user_data)
 {
+	const uint8_t bt_base[] = { 0x00, 0x00, 0x10, 0x00, 0x80, 0x00,
+					0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB };
 	struct network_adapter *na = user_data;
 	struct network_server *ns;
 	uint8_t packet[BNEP_MTU];
 	struct bnep_setup_conn_req *req = (void *) packet;
-	uint16_t src_role, dst_role, rsp = BNEP_CONN_NOT_ALLOWED;
+	uint16_t dst_role, rsp = BNEP_CONN_NOT_ALLOWED;
+	uint32_t val;
 	int n, sk;
 
 	if (cond & G_IO_NVAL)
@@ -304,30 +308,27 @@ static gboolean bnep_setup(GIOChannel *chan,
 		return FALSE;
 	}
 
-	/* Highest known Control command ID
-	 * is BNEP_FILTER_MULT_ADDR_RSP = 0x06 */
-	if (req->type == BNEP_CONTROL &&
-				req->ctrl > BNEP_FILTER_MULT_ADDR_RSP) {
-		uint8_t pkt[3];
+	switch (req->uuid_size) {
+	case 2:
+		dst_role = get_be16(req->service);
+		break;
+	case 16:
+		if (memcmp(&req->service[4], bt_base, sizeof(bt_base)) != 0)
+			return FALSE;
 
-		pkt[0] = BNEP_CONTROL;
-		pkt[1] = BNEP_CMD_NOT_UNDERSTOOD;
-		pkt[2] = req->ctrl;
+		/* Intentional no-brake */
 
-		send(sk, pkt, sizeof(pkt), 0);
+	case 4:
+		val = get_be32(req->service);
+		if (val > 0xffff)
+			return FALSE;
 
+		dst_role = val;
+		break;
+	default:
 		return FALSE;
 	}
 
-	if (req->type != BNEP_CONTROL || req->ctrl != BNEP_SETUP_CONN_REQ)
-		return FALSE;
-
-	rsp = bnep_setup_decode(req, &dst_role, &src_role);
-	if (rsp != BNEP_SUCCESS)
-		goto reply;
-
-	rsp = BNEP_CONN_NOT_ALLOWED;
-
 	ns = find_server(na->servers, dst_role);
 	if (!ns) {
 		error("Server unavailable: (0x%x)", dst_role);
@@ -347,8 +348,8 @@ static gboolean bnep_setup(GIOChannel *chan,
 	strncpy(na->setup->dev, BNEP_INTERFACE, 16);
 	na->setup->dev[15] = '\0';
 
-	if (bnep_server_add(sk, dst_role, ns->bridge, na->setup->dev,
-							&na->setup->dst) < 0)
+	if (bnep_server_add(sk, ns->bridge, na->setup->dev,
+					&na->setup->dst, (void *) packet) < 0)
 		goto reply;
 
 	na->setup = NULL;
-- 
2.1.0

Re: [PATCH 04/12] profiles/network: Move bnep connection setup logic to bnep

[Szymon Janc]

Hi Grzegorz,

On Thursday 26 of February 2015 12:53:26 Grzegorz Kolodziejczyk wrote:
> BNEP connection set up logic which was added before bnep_server_add,
> can be private method of bnep. Moved logic was almost doubled in two
> cases: NAP role in PAN, server listening. Now set up and connect
> scenario check of bnep connection is only handled in bnep part for
> listen connections.
> ---
>  android/pan.c             |  28 ++--------
>  profiles/network/bnep.c   | 127 ++++++++++++++++++++++++++++------------------
>  profiles/network/bnep.h   |   6 +--
>  profiles/network/server.c |  43 ++++++++--------
>  4 files changed, 105 insertions(+), 99 deletions(-)
> 
> diff --git a/android/pan.c b/android/pan.c
> index 0bf5f71..8bafcd0 100644
> --- a/android/pan.c
> +++ b/android/pan.c
> @@ -463,8 +463,7 @@ static gboolean nap_setup_cb(GIOChannel *chan, GIOCondition cond,
>  {
>  	struct pan_device *dev = user_data;
>  	uint8_t packet[BNEP_MTU];
> -	struct bnep_setup_conn_req *req = (void *) packet;
> -	uint16_t src_role, dst_role, rsp = BNEP_CONN_NOT_ALLOWED;
> +	uint16_t rsp = BNEP_CONN_NOT_ALLOWED;
>  	int sk, n, err;
>  
>  	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
> @@ -481,27 +480,6 @@ static gboolean nap_setup_cb(GIOChannel *chan, GIOCondition cond,
>  		goto failed;
>  	}
>  
> -	/* Highest known control command id BNEP_FILTER_MULT_ADDR_RSP 0x06 */
> -	if (req->type == BNEP_CONTROL &&
> -					req->ctrl > BNEP_FILTER_MULT_ADDR_RSP) {
> -		error("cmd not understood");
> -		bnep_send_ctrl_rsp(sk, BNEP_CONTROL, BNEP_CMD_NOT_UNDERSTOOD,
> -								req->ctrl);
> -		goto failed;
> -	}
> -
> -	if (req->type != BNEP_CONTROL || req->ctrl != BNEP_SETUP_CONN_REQ) {
> -		error("cmd is not BNEP_SETUP_CONN_REQ %02X %02X", req->type,
> -								req->ctrl);
> -		goto failed;
> -	}
> -
> -	rsp = bnep_setup_decode(req, &dst_role, &src_role);
> -	if (rsp != BNEP_SUCCESS) {
> -		error("bnep_setup_decode failed");
> -		goto failed;
> -	}
> -
>  	err = nap_create_bridge();
>  	if (err < 0) {
>  		error("pan: Failed to create bridge: %s (%d)", strerror(-err),
> @@ -509,8 +487,8 @@ static gboolean nap_setup_cb(GIOChannel *chan, GIOCondition cond,
>  		goto failed;
>  	}
>  
> -	if (bnep_server_add(sk, dst_role, BNEP_BRIDGE, dev->iface,
> -							&dev->dst) < 0) {
> +	if (bnep_server_add(sk, BNEP_BRIDGE, dev->iface, &dev->dst,
> +							(void *) packet) < 0) {


I'd move request reading to bnep_server_add(). Also don't forget to verify
if enough bytes was read before accessing this via struct bnep_setup_conn_req
pointer.

>  		nap_remove_bridge();
>  		error("server_connadd failed");
>  		rsp = BNEP_CONN_NOT_ALLOWED;
> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> index fe13a5b..7177972 100644
> --- a/profiles/network/bnep.c
> +++ b/profiles/network/bnep.c
> @@ -549,63 +549,25 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
>  	return err;
>  }
>  
> -int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
> -						const bdaddr_t *addr)
> -{
> -	int err;
> -
> -	if (!bridge || !iface || !addr)
> -		return -EINVAL;
> -
> -	err = bnep_connadd(sk, dst, iface);
> -	if (err < 0)
> -		return err;
> -
> -	err = bnep_add_to_bridge(iface, bridge);
> -	if (err < 0) {
> -		bnep_conndel(addr);
> -		return err;
> -	}
> -
> -	return bnep_if_up(iface);
> -}
> -
> -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;
> -
> -	rsp.type = type;
> -	rsp.ctrl = ctrl;
> -	rsp.resp = htons(resp);
> -
> -	return send(sk, &rsp, sizeof(rsp), 0);
> -}
> -
> -uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
> -								uint16_t *src)
> +static uint16_t bnep_setup_decode(int sk, struct bnep_setup_conn_req *req,
> +								uint16_t *dst)
>  {
>  	const uint8_t bt_base[] = { 0x00, 0x00, 0x10, 0x00, 0x80, 0x00,
>  					0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB };
> +	uint16_t src;
>  	uint8_t *dest, *source;
>  	uint32_t val;
>  
> +	if (req->type != BNEP_CONTROL || req->ctrl != BNEP_SETUP_CONN_REQ)
> +		return BNEP_CONN_NOT_ALLOWED;
> +
>  	dest = req->service;
>  	source = req->service + req->uuid_size;
>  
>  	switch (req->uuid_size) {
>  	case 2: /* UUID16 */
>  		*dst = get_be16(dest);
> -		*src = get_be16(source);
> +		src = get_be16(source);
>  		break;
>  	case 16: /* UUID128 */
>  		/* Check that the bytes in the UUID, except the service ID
> @@ -629,7 +591,7 @@ uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
>  		if (val > 0xffff)
>  			return BNEP_CONN_INVALID_SRC;
>  
> -		*src = val;
> +		src = val;
>  		break;
>  	default:
>  		return BNEP_CONN_INVALID_SVC;
> @@ -639,12 +601,13 @@ uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
>  	switch (*dst) {
>  	case BNEP_SVC_NAP:
>  	case BNEP_SVC_GN:
> -		if (*src == BNEP_SVC_PANU)
> +		if (src == BNEP_SVC_PANU)
>  			return BNEP_SUCCESS;
> +
>  		return BNEP_CONN_INVALID_SRC;
>  	case BNEP_SVC_PANU:
> -		if (*src == BNEP_SVC_PANU || *src == BNEP_SVC_GN ||
> -							*src == BNEP_SVC_NAP)
> +		if (src == BNEP_SVC_PANU || src == BNEP_SVC_GN ||
> +							src == BNEP_SVC_NAP)
>  			return BNEP_SUCCESS;
>  
>  		return BNEP_CONN_INVALID_SRC;
> @@ -652,3 +615,69 @@ uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
>  
>  	return BNEP_CONN_INVALID_DST;
>  }
> +
> +int bnep_server_add(int sk, char *bridge, char *iface, const bdaddr_t *addr,
> +							void *setup_data)
> +{
> +	int err;
> +	uint16_t dst = NULL;
> +	struct bnep_setup_conn_req *req = setup_data;
> +
> +	/* Highest known Control command ID
> +	 * is BNEP_FILTER_MULT_ADDR_RSP = 0x06 */
> +	if (req->type == BNEP_CONTROL &&
> +				req->ctrl > BNEP_FILTER_MULT_ADDR_RSP) {
> +		uint8_t pkt[3];
> +
> +		pkt[0] = BNEP_CONTROL;
> +		pkt[1] = BNEP_CMD_NOT_UNDERSTOOD;
> +		pkt[2] = req->ctrl;
> +
> +		send(sk, pkt, sizeof(pkt), 0);
> +
> +		return -EINVAL;
> +	}
> +
> +	/* Processing BNEP_SETUP_CONNECTION_REQUEST_MSG */
> +	err = bnep_setup_decode(sk, setup_data, &dst);
> +	if (err < 0) {
> +		error("error while decoding setup connection request: %d", err);
> +		return -EINVAL;
> +	}
> +
> +	if (!bridge || !iface || !addr || !dst)
> +		return -EINVAL;
> +
> +	err = bnep_connadd(sk, dst, iface);
> +	if (err < 0)
> +		return err;
> +
> +	err = bnep_add_to_bridge(iface, bridge);
> +	if (err < 0) {
> +		bnep_conndel(addr);
> +		return err;
> +	}
> +
> +	return bnep_if_up(iface);
> +}
> +
> +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;
> +
> +	rsp.type = type;
> +	rsp.ctrl = ctrl;
> +	rsp.resp = htons(resp);
> +
> +	return send(sk, &rsp, sizeof(rsp), 0);
> +}
> diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
> index 5aedf38..7a3efb6 100644
> --- a/profiles/network/bnep.h
> +++ b/profiles/network/bnep.h
> @@ -41,10 +41,8 @@ void bnep_set_disconnect(struct bnep *session, bnep_disconnect_cb disconn_cb,
>  								void *data);
>  void bnep_disconnect(struct bnep *session);
>  
> -int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
> -							const bdaddr_t *addr);
> +int bnep_server_add(int sk, char *bridge, char *iface, const bdaddr_t *addr,
> +							void *setup_data);
>  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_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
> -								uint16_t *src);
> diff --git a/profiles/network/server.c b/profiles/network/server.c
> index 12266c9..e0eb6ef 100644
> --- a/profiles/network/server.c
> +++ b/profiles/network/server.c
> @@ -46,6 +46,7 @@
>  #include "src/log.h"
>  #include "src/error.h"
>  #include "src/sdpd.h"
> +#include "src/shared/util.h"
>  
>  #include "bnep.h"
>  #include "server.h"
> @@ -280,11 +281,14 @@ static void setup_destroy(void *user_data)
>  static gboolean bnep_setup(GIOChannel *chan,
>  			GIOCondition cond, gpointer user_data)
>  {
> +	const uint8_t bt_base[] = { 0x00, 0x00, 0x10, 0x00, 0x80, 0x00,
> +					0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB };
>  	struct network_adapter *na = user_data;
>  	struct network_server *ns;
>  	uint8_t packet[BNEP_MTU];
>  	struct bnep_setup_conn_req *req = (void *) packet;
> -	uint16_t src_role, dst_role, rsp = BNEP_CONN_NOT_ALLOWED;
> +	uint16_t dst_role, rsp = BNEP_CONN_NOT_ALLOWED;
> +	uint32_t val;
>  	int n, sk;
>  
>  	if (cond & G_IO_NVAL)
> @@ -304,30 +308,27 @@ static gboolean bnep_setup(GIOChannel *chan,
>  		return FALSE;
>  	}
>  
> -	/* Highest known Control command ID
> -	 * is BNEP_FILTER_MULT_ADDR_RSP = 0x06 */
> -	if (req->type == BNEP_CONTROL &&
> -				req->ctrl > BNEP_FILTER_MULT_ADDR_RSP) {
> -		uint8_t pkt[3];
> +	switch (req->uuid_size) {
> +	case 2:
> +		dst_role = get_be16(req->service);
> +		break;
> +	case 16:
> +		if (memcmp(&req->service[4], bt_base, sizeof(bt_base)) != 0)
> +			return FALSE;
>  
> -		pkt[0] = BNEP_CONTROL;
> -		pkt[1] = BNEP_CMD_NOT_UNDERSTOOD;
> -		pkt[2] = req->ctrl;
> +		/* Intentional no-brake */
>  
> -		send(sk, pkt, sizeof(pkt), 0);
> +	case 4:
> +		val = get_be32(req->service);
> +		if (val > 0xffff)
> +			return FALSE;
>  
> +		dst_role = val;
> +		break;
> +	default:
>  		return FALSE;
>  	}
>  
> -	if (req->type != BNEP_CONTROL || req->ctrl != BNEP_SETUP_CONN_REQ)
> -		return FALSE;
> -
> -	rsp = bnep_setup_decode(req, &dst_role, &src_role);
> -	if (rsp != BNEP_SUCCESS)
> -		goto reply;
> -
> -	rsp = BNEP_CONN_NOT_ALLOWED;
> -
>  	ns = find_server(na->servers, dst_role);
>  	if (!ns) {
>  		error("Server unavailable: (0x%x)", dst_role);
> @@ -347,8 +348,8 @@ static gboolean bnep_setup(GIOChannel *chan,
>  	strncpy(na->setup->dev, BNEP_INTERFACE, 16);
>  	na->setup->dev[15] = '\0';
>  
> -	if (bnep_server_add(sk, dst_role, ns->bridge, na->setup->dev,
> -							&na->setup->dst) < 0)
> +	if (bnep_server_add(sk, ns->bridge, na->setup->dev,
> +					&na->setup->dst, (void *) packet) < 0)
>  		goto reply;
>  
>  	na->setup = NULL;
> 

-- 
Best regards, 
Szymon Janc

[PATCH 05/12] profiles/network: Handle ctrl rsp after conn setup by bnep

[Grzegorz Kolodziejczyk]

This patch moves setup control response to bnep and makes it private.
---
 android/pan.c             | 14 +++--------
 profiles/network/bnep.c   | 60 +++++++++++++++++++++++++++++------------------
 profiles/network/bnep.h   |  2 --
 profiles/network/server.c | 36 +++++++++-------------------
 4 files changed, 51 insertions(+), 61 deletions(-)

diff --git a/android/pan.c b/android/pan.c
index 8bafcd0..08c8134 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -463,7 +463,6 @@ static gboolean nap_setup_cb(GIOChannel *chan, GIOCondition cond,
 {
 	struct pan_device *dev = user_data;
 	uint8_t packet[BNEP_MTU];
-	uint16_t rsp = BNEP_CONN_NOT_ALLOWED;
 	int sk, n, err;
 
 	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
@@ -481,23 +480,17 @@ static gboolean nap_setup_cb(GIOChannel *chan, GIOCondition cond,
 	}
 
 	err = nap_create_bridge();
-	if (err < 0) {
+	if (err < 0)
 		error("pan: Failed to create bridge: %s (%d)", strerror(-err),
 									-err);
-		goto failed;
-	}
 
-	if (bnep_server_add(sk, BNEP_BRIDGE, dev->iface, &dev->dst,
-							(void *) packet) < 0) {
+	if (bnep_server_add(sk, (err < 0) ? NULL : BNEP_BRIDGE, dev->iface,
+					&dev->dst, (void *) packet) < 0) {
 		nap_remove_bridge();
 		error("server_connadd failed");
-		rsp = BNEP_CONN_NOT_ALLOWED;
 		goto failed;
 	}
 
-	rsp = BNEP_SUCCESS;
-	bnep_send_ctrl_rsp(sk, BNEP_CONTROL, BNEP_SETUP_CONN_RSP, rsp);
-
 	dev->watch = g_io_add_watch(chan, G_IO_HUP | G_IO_ERR | G_IO_NVAL,
 							nap_watchdog_cb, dev);
 	g_io_channel_unref(dev->io);
@@ -509,7 +502,6 @@ static gboolean nap_setup_cb(GIOChannel *chan, GIOCondition cond,
 	return FALSE;
 
 failed:
-	bnep_send_ctrl_rsp(sk, BNEP_CONTROL, BNEP_SETUP_CONN_RSP, rsp);
 	pan_device_remove(dev);
 
 	return FALSE;
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 7177972..bef9b60 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -549,6 +549,18 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
 	return err;
 }
 
+static ssize_t bnep_send_ctrl_rsp(int sk, uint8_t type, uint8_t ctrl,
+								uint16_t resp)
+{
+	struct bnep_control_rsp rsp;
+
+	rsp.type = type;
+	rsp.ctrl = ctrl;
+	rsp.resp = htons(resp);
+
+	return send(sk, &rsp, sizeof(rsp), 0);
+}
+
 static uint16_t bnep_setup_decode(int sk, struct bnep_setup_conn_req *req,
 								uint16_t *dst)
 {
@@ -603,7 +615,6 @@ static uint16_t bnep_setup_decode(int sk, struct bnep_setup_conn_req *req,
 	case BNEP_SVC_GN:
 		if (src == BNEP_SVC_PANU)
 			return BNEP_SUCCESS;
-
 		return BNEP_CONN_INVALID_SRC;
 	case BNEP_SVC_PANU:
 		if (src == BNEP_SVC_PANU || src == BNEP_SVC_GN ||
@@ -620,7 +631,7 @@ int bnep_server_add(int sk, char *bridge, char *iface, const bdaddr_t *addr,
 							void *setup_data)
 {
 	int err;
-	uint16_t dst = NULL;
+	uint16_t rsp, dst = NULL;
 	struct bnep_setup_conn_req *req = setup_data;
 
 	/* Highest known Control command ID
@@ -639,26 +650,40 @@ int bnep_server_add(int sk, char *bridge, char *iface, const bdaddr_t *addr,
 	}
 
 	/* Processing BNEP_SETUP_CONNECTION_REQUEST_MSG */
-	err = bnep_setup_decode(sk, setup_data, &dst);
-	if (err < 0) {
-		error("error while decoding setup connection request: %d", err);
-		return -EINVAL;
+	rsp = bnep_setup_decode(sk, setup_data, &dst);
+	if (rsp != BNEP_SUCCESS || !dst) {
+		err = -rsp;
+		error("error while decoding setup connection request: %d", rsp);
+		goto reply;
 	}
 
-	if (!bridge || !iface || !addr || !dst)
-		return -EINVAL;
+	if (!dst) {
+		error("cannot decode proper destination service UUID");
+		rsp = BNEP_CONN_INVALID_DST;
+		goto reply;
+	}
 
 	err = bnep_connadd(sk, dst, iface);
-	if (err < 0)
-		return err;
+	if (err < 0) {
+		rsp = BNEP_CONN_NOT_ALLOWED;
+		goto reply;
+	}
 
 	err = bnep_add_to_bridge(iface, bridge);
 	if (err < 0) {
 		bnep_conndel(addr);
-		return err;
+		rsp = BNEP_CONN_NOT_ALLOWED;
+		goto reply;
 	}
 
-	return bnep_if_up(iface);
+	err = bnep_if_up(iface);
+	if (err < 0)
+		rsp = BNEP_CONN_NOT_ALLOWED;
+
+reply:
+	bnep_send_ctrl_rsp(sk, BNEP_CONTROL, BNEP_SETUP_CONN_RSP, rsp);
+
+	return err;
 }
 
 void bnep_server_delete(char *bridge, char *iface, const bdaddr_t *addr)
@@ -670,14 +695,3 @@ void bnep_server_delete(char *bridge, char *iface, const bdaddr_t *addr)
 	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;
-
-	rsp.type = type;
-	rsp.ctrl = ctrl;
-	rsp.resp = htons(resp);
-
-	return send(sk, &rsp, sizeof(rsp), 0);
-}
diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
index 7a3efb6..a9d9f98 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -44,5 +44,3 @@ void bnep_disconnect(struct bnep *session);
 int bnep_server_add(int sk, char *bridge, char *iface, const bdaddr_t *addr,
 							void *setup_data);
 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);
diff --git a/profiles/network/server.c b/profiles/network/server.c
index e0eb6ef..8e9afbc 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -287,9 +287,10 @@ static gboolean bnep_setup(GIOChannel *chan,
 	struct network_server *ns;
 	uint8_t packet[BNEP_MTU];
 	struct bnep_setup_conn_req *req = (void *) packet;
-	uint16_t dst_role, rsp = BNEP_CONN_NOT_ALLOWED;
+	uint16_t dst_role = 0;
 	uint32_t val;
 	int n, sk;
+	char *bridge = NULL;
 
 	if (cond & G_IO_NVAL)
 		return FALSE;
@@ -314,51 +315,36 @@ static gboolean bnep_setup(GIOChannel *chan,
 		break;
 	case 16:
 		if (memcmp(&req->service[4], bt_base, sizeof(bt_base)) != 0)
-			return FALSE;
+			break;
 
 		/* Intentional no-brake */
 
 	case 4:
 		val = get_be32(req->service);
 		if (val > 0xffff)
-			return FALSE;
+			break;
 
 		dst_role = val;
 		break;
 	default:
-		return FALSE;
+		break;
 	}
 
 	ns = find_server(na->servers, dst_role);
-	if (!ns) {
-		error("Server unavailable: (0x%x)", dst_role);
-		goto reply;
-	}
-
-	if (!ns->record_id) {
-		error("Service record not available");
-		goto reply;
-	}
+	if (!ns || !ns->record_id || !ns->bridge)
+		error("Server error, bridge not initialized: (0x%x)", dst_role);
 
-	if (!ns->bridge) {
-		error("Bridge interface not configured");
-		goto reply;
-	}
+	bridge = ns->bridge;
 
 	strncpy(na->setup->dev, BNEP_INTERFACE, 16);
 	na->setup->dev[15] = '\0';
 
-	if (bnep_server_add(sk, ns->bridge, na->setup->dev,
-					&na->setup->dst, (void *) packet) < 0)
-		goto reply;
+	if (bnep_server_add(sk, bridge, na->setup->dev, &na->setup->dst,
+							(void *) packet) < 0)
+		error("BNEP server cannot be added");
 
 	na->setup = NULL;
 
-	rsp = BNEP_SUCCESS;
-
-reply:
-	bnep_send_ctrl_rsp(sk, BNEP_CONTROL, BNEP_SETUP_CONN_RSP, rsp);
-
 	return FALSE;
 }
 
-- 
2.1.0

Re: [PATCH 05/12] profiles/network: Handle ctrl rsp after conn setup by bnep

[Szymon Janc]

Hi Grzegorz,

On Thursday 26 of February 2015 12:53:27 Grzegorz Kolodziejczyk wrote:
> This patch moves setup control response to bnep and makes it private.
> ---
>  android/pan.c             | 14 +++--------
>  profiles/network/bnep.c   | 60 +++++++++++++++++++++++++++++------------------
>  profiles/network/bnep.h   |  2 --
>  profiles/network/server.c | 36 +++++++++-------------------
>  4 files changed, 51 insertions(+), 61 deletions(-)
> 
> diff --git a/android/pan.c b/android/pan.c
> index 8bafcd0..08c8134 100644
> --- a/android/pan.c
> +++ b/android/pan.c
> @@ -463,7 +463,6 @@ static gboolean nap_setup_cb(GIOChannel *chan, GIOCondition cond,
>  {
>  	struct pan_device *dev = user_data;
>  	uint8_t packet[BNEP_MTU];
> -	uint16_t rsp = BNEP_CONN_NOT_ALLOWED;
>  	int sk, n, err;
>  
>  	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL)) {
> @@ -481,23 +480,17 @@ static gboolean nap_setup_cb(GIOChannel *chan, GIOCondition cond,
>  	}
>  
>  	err = nap_create_bridge();
> -	if (err < 0) {
> +	if (err < 0)
>  		error("pan: Failed to create bridge: %s (%d)", strerror(-err),
>  									-err);
> -		goto failed;

Please add some comment here why just error is printed eg (about error response
being send from bnep_server_add()).

> -	}
>  
> -	if (bnep_server_add(sk, BNEP_BRIDGE, dev->iface, &dev->dst,
> -							(void *) packet) < 0) {
> +	if (bnep_server_add(sk, (err < 0) ? NULL : BNEP_BRIDGE, dev->iface,
> +					&dev->dst, (void *) packet) < 0) {
>  		nap_remove_bridge();

You need to cleanly handle error path ie. here bridge might not be created.
But since we might have more devices connected I'd leave nap_remove_bridge()
to be handled in pan_device_remove().

>  		error("server_connadd failed");
> -		rsp = BNEP_CONN_NOT_ALLOWED;
>  		goto failed;
>  	}
>  
> -	rsp = BNEP_SUCCESS;
> -	bnep_send_ctrl_rsp(sk, BNEP_CONTROL, BNEP_SETUP_CONN_RSP, rsp);
> -
>  	dev->watch = g_io_add_watch(chan, G_IO_HUP | G_IO_ERR | G_IO_NVAL,
>  							nap_watchdog_cb, dev);
>  	g_io_channel_unref(dev->io);
> @@ -509,7 +502,6 @@ static gboolean nap_setup_cb(GIOChannel *chan, GIOCondition cond,
>  	return FALSE;
>  
>  failed:
> -	bnep_send_ctrl_rsp(sk, BNEP_CONTROL, BNEP_SETUP_CONN_RSP, rsp);
>  	pan_device_remove(dev);
>  
>  	return FALSE;
> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> index 7177972..bef9b60 100644
> --- a/profiles/network/bnep.c
> +++ b/profiles/network/bnep.c
> @@ -549,6 +549,18 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
>  	return err;
>  }
>  
> +static ssize_t bnep_send_ctrl_rsp(int sk, uint8_t type, uint8_t ctrl,
> +								uint16_t resp)
> +{
> +	struct bnep_control_rsp rsp;
> +
> +	rsp.type = type;
> +	rsp.ctrl = ctrl;
> +	rsp.resp = htons(resp);
> +
> +	return send(sk, &rsp, sizeof(rsp), 0);
> +}
> +
>  static uint16_t bnep_setup_decode(int sk, struct bnep_setup_conn_req *req,
>  								uint16_t *dst)
>  {
> @@ -603,7 +615,6 @@ static uint16_t bnep_setup_decode(int sk, struct bnep_setup_conn_req *req,
>  	case BNEP_SVC_GN:
>  		if (src == BNEP_SVC_PANU)
>  			return BNEP_SUCCESS;
> -
>  		return BNEP_CONN_INVALID_SRC;
>  	case BNEP_SVC_PANU:
>  		if (src == BNEP_SVC_PANU || src == BNEP_SVC_GN ||
> @@ -620,7 +631,7 @@ int bnep_server_add(int sk, char *bridge, char *iface, const bdaddr_t *addr,
>  							void *setup_data)
>  {
>  	int err;
> -	uint16_t dst = NULL;
> +	uint16_t rsp, dst = NULL;
>  	struct bnep_setup_conn_req *req = setup_data;
>  
>  	/* Highest known Control command ID
> @@ -639,26 +650,40 @@ int bnep_server_add(int sk, char *bridge, char *iface, const bdaddr_t *addr,
>  	}
>  
>  	/* Processing BNEP_SETUP_CONNECTION_REQUEST_MSG */
> -	err = bnep_setup_decode(sk, setup_data, &dst);
> -	if (err < 0) {
> -		error("error while decoding setup connection request: %d", err);
> -		return -EINVAL;
> +	rsp = bnep_setup_decode(sk, setup_data, &dst);
> +	if (rsp != BNEP_SUCCESS || !dst) {
> +		err = -rsp;
> +		error("error while decoding setup connection request: %d", rsp);
> +		goto reply;
>  	}
>  
> -	if (!bridge || !iface || !addr || !dst)
> -		return -EINVAL;
> +	if (!dst) {
> +		error("cannot decode proper destination service UUID");
> +		rsp = BNEP_CONN_INVALID_DST;
> +		goto reply;
> +	}
>  
>  	err = bnep_connadd(sk, dst, iface);
> -	if (err < 0)
> -		return err;
> +	if (err < 0) {
> +		rsp = BNEP_CONN_NOT_ALLOWED;
> +		goto reply;
> +	}
>  
>  	err = bnep_add_to_bridge(iface, bridge);
>  	if (err < 0) {
>  		bnep_conndel(addr);
> -		return err;
> +		rsp = BNEP_CONN_NOT_ALLOWED;
> +		goto reply;
>  	}
>  
> -	return bnep_if_up(iface);
> +	err = bnep_if_up(iface);
> +	if (err < 0)
> +		rsp = BNEP_CONN_NOT_ALLOWED;
> +
> +reply:
> +	bnep_send_ctrl_rsp(sk, BNEP_CONTROL, BNEP_SETUP_CONN_RSP, rsp);
> +
> +	return err;
>  }
>  
>  void bnep_server_delete(char *bridge, char *iface, const bdaddr_t *addr)
> @@ -670,14 +695,3 @@ void bnep_server_delete(char *bridge, char *iface, const bdaddr_t *addr)
>  	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;
> -
> -	rsp.type = type;
> -	rsp.ctrl = ctrl;
> -	rsp.resp = htons(resp);
> -
> -	return send(sk, &rsp, sizeof(rsp), 0);
> -}
> diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
> index 7a3efb6..a9d9f98 100644
> --- a/profiles/network/bnep.h
> +++ b/profiles/network/bnep.h
> @@ -44,5 +44,3 @@ void bnep_disconnect(struct bnep *session);
>  int bnep_server_add(int sk, char *bridge, char *iface, const bdaddr_t *addr,
>  							void *setup_data);
>  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);
> diff --git a/profiles/network/server.c b/profiles/network/server.c
> index e0eb6ef..8e9afbc 100644
> --- a/profiles/network/server.c
> +++ b/profiles/network/server.c
> @@ -287,9 +287,10 @@ static gboolean bnep_setup(GIOChannel *chan,
>  	struct network_server *ns;
>  	uint8_t packet[BNEP_MTU];
>  	struct bnep_setup_conn_req *req = (void *) packet;
> -	uint16_t dst_role, rsp = BNEP_CONN_NOT_ALLOWED;
> +	uint16_t dst_role = 0;
>  	uint32_t val;
>  	int n, sk;
> +	char *bridge = NULL;
>  
>  	if (cond & G_IO_NVAL)
>  		return FALSE;
> @@ -314,51 +315,36 @@ static gboolean bnep_setup(GIOChannel *chan,
>  		break;
>  	case 16:
>  		if (memcmp(&req->service[4], bt_base, sizeof(bt_base)) != 0)
> -			return FALSE;
> +			break;
>  
>  		/* Intentional no-brake */
>  
>  	case 4:
>  		val = get_be32(req->service);
>  		if (val > 0xffff)
> -			return FALSE;
> +			break;
>  
>  		dst_role = val;
>  		break;
>  	default:
> -		return FALSE;
> +		break;
>  	}
>  
>  	ns = find_server(na->servers, dst_role);
> -	if (!ns) {
> -		error("Server unavailable: (0x%x)", dst_role);
> -		goto reply;
> -	}
> -
> -	if (!ns->record_id) {
> -		error("Service record not available");
> -		goto reply;
> -	}
> +	if (!ns || !ns->record_id || !ns->bridge)
> +		error("Server error, bridge not initialized: (0x%x)", dst_role);
>  
> -	if (!ns->bridge) {
> -		error("Bridge interface not configured");
> -		goto reply;
> -	}
> +	bridge = ns->bridge;
>  
>  	strncpy(na->setup->dev, BNEP_INTERFACE, 16);
>  	na->setup->dev[15] = '\0';
>  
> -	if (bnep_server_add(sk, ns->bridge, na->setup->dev,
> -					&na->setup->dst, (void *) packet) < 0)
> -		goto reply;
> +	if (bnep_server_add(sk, bridge, na->setup->dev, &na->setup->dst,
> +							(void *) packet) < 0)
> +		error("BNEP server cannot be added");
>  
>  	na->setup = NULL;
>  
> -	rsp = BNEP_SUCCESS;
> -
> -reply:
> -	bnep_send_ctrl_rsp(sk, BNEP_CONTROL, BNEP_SETUP_CONN_RSP, rsp);
> -
>  	return FALSE;
>  }
>  
> 

-- 
Best regards, 
Szymon Janc

[PATCH 06/12] profiles/network: Fix sending command responses

[Grzegorz Kolodziejczyk]

Command response can be three bytes long (in case if command is not
understood) 1 byte(packet type) + 1 byte(control type) + 1 byte(unknown
control type). Command response can be also four bytes long if it's
response for setup connection, filter net type and filter multi addr,
1 byte(packet type) + 1 byte(control type) + 2 byte(response message).
---
 lib/bnep.h              |  6 +++++
 profiles/network/bnep.c | 59 +++++++++++++++++++++++++++++++++++--------------
 2 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/lib/bnep.h b/lib/bnep.h
index 2bbfb17..aa46852 100644
--- a/lib/bnep.h
+++ b/lib/bnep.h
@@ -103,6 +103,12 @@ struct bnep_set_filter_req {
 	uint8_t  list[0];
 } __attribute__((packed));
 
+struct bnep_ctrl_cmd_not_understood_cmd {
+	uint8_t type;
+	uint8_t ctrl;
+	uint8_t unkn_ctrl;
+} __attribute__((packed));
+
 struct bnep_control_rsp {
 	uint8_t  type;
 	uint8_t  ctrl;
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index bef9b60..132dbab 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -549,16 +549,40 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
 	return err;
 }
 
-static ssize_t bnep_send_ctrl_rsp(int sk, uint8_t type, uint8_t ctrl,
-								uint16_t resp)
+static ssize_t bnep_send_ctrl_rsp(int sk, uint8_t ctrl, uint16_t resp)
 {
-	struct bnep_control_rsp rsp;
+	ssize_t sent;
 
-	rsp.type = type;
-	rsp.ctrl = ctrl;
-	rsp.resp = htons(resp);
+	switch (ctrl) {
+	case BNEP_CMD_NOT_UNDERSTOOD: {
+		struct bnep_ctrl_cmd_not_understood_cmd rsp;
 
-	return send(sk, &rsp, sizeof(rsp), 0);
+		rsp.type = BNEP_CONTROL;
+		rsp.ctrl = ctrl;
+		rsp.unkn_ctrl = (uint8_t) resp;
+
+		sent = send(sk, &rsp, sizeof(rsp), 0);
+		break;
+	}
+	case BNEP_FILTER_MULT_ADDR_RSP:
+	case BNEP_FILTER_NET_TYPE_RSP:
+	case BNEP_SETUP_CONN_RSP: {
+		struct bnep_control_rsp rsp;
+
+		rsp.type = BNEP_CONTROL;
+		rsp.ctrl = ctrl;
+		rsp.resp = htons(resp);
+
+		sent = send(sk, &rsp, sizeof(rsp), 0);
+		break;
+	}
+	default:
+		error("wrong bnep response type");
+		sent = -1;
+		break;
+	}
+
+	return sent;
 }
 
 static uint16_t bnep_setup_decode(int sk, struct bnep_setup_conn_req *req,
@@ -637,16 +661,15 @@ int bnep_server_add(int sk, char *bridge, char *iface, const bdaddr_t *addr,
 	/* Highest known Control command ID
 	 * is BNEP_FILTER_MULT_ADDR_RSP = 0x06 */
 	if (req->type == BNEP_CONTROL &&
-				req->ctrl > BNEP_FILTER_MULT_ADDR_RSP) {
-		uint8_t pkt[3];
-
-		pkt[0] = BNEP_CONTROL;
-		pkt[1] = BNEP_CMD_NOT_UNDERSTOOD;
-		pkt[2] = req->ctrl;
+					req->ctrl > BNEP_FILTER_MULT_ADDR_RSP) {
+		error("cmd not understood");
+		err = bnep_send_ctrl_rsp(sk, BNEP_CMD_NOT_UNDERSTOOD,
+								req->ctrl);
+		if (err < 0)
+			error("send not understood ctrl rsp error: %s (%d)",
+							strerror(errno), errno);
 
-		send(sk, pkt, sizeof(pkt), 0);
-
-		return -EINVAL;
+		return err;
 	}
 
 	/* Processing BNEP_SETUP_CONNECTION_REQUEST_MSG */
@@ -681,7 +704,9 @@ int bnep_server_add(int sk, char *bridge, char *iface, const bdaddr_t *addr,
 		rsp = BNEP_CONN_NOT_ALLOWED;
 
 reply:
-	bnep_send_ctrl_rsp(sk, BNEP_CONTROL, BNEP_SETUP_CONN_RSP, rsp);
+	err = bnep_send_ctrl_rsp(sk, BNEP_SETUP_CONN_RSP, rsp);
+	if (err < 0)
+		error("send ctrl rsp error: %s (%d)", strerror(errno), errno);
 
 	return err;
 }
-- 
2.1.0

Re: [PATCH 06/12] profiles/network: Fix sending command responses

[Szymon Janc]

Hi Grzegorz,

On Thursday 26 of February 2015 12:53:28 Grzegorz Kolodziejczyk wrote:
> Command response can be three bytes long (in case if command is not
> understood) 1 byte(packet type) + 1 byte(control type) + 1 byte(unknown
> control type). Command response can be also four bytes long if it's
> response for setup connection, filter net type and filter multi addr,
> 1 byte(packet type) + 1 byte(control type) + 2 byte(response message).
> ---
>  lib/bnep.h              |  6 +++++
>  profiles/network/bnep.c | 59 +++++++++++++++++++++++++++++++++++--------------
>  2 files changed, 48 insertions(+), 17 deletions(-)
> 
> diff --git a/lib/bnep.h b/lib/bnep.h
> index 2bbfb17..aa46852 100644
> --- a/lib/bnep.h
> +++ b/lib/bnep.h
> @@ -103,6 +103,12 @@ struct bnep_set_filter_req {
>  	uint8_t  list[0];
>  } __attribute__((packed));
>  
> +struct bnep_ctrl_cmd_not_understood_cmd {
> +	uint8_t type;
> +	uint8_t ctrl;
> +	uint8_t unkn_ctrl;
> +} __attribute__((packed));
> +
>  struct bnep_control_rsp {
>  	uint8_t  type;
>  	uint8_t  ctrl;
> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> index bef9b60..132dbab 100644
> --- a/profiles/network/bnep.c
> +++ b/profiles/network/bnep.c
> @@ -549,16 +549,40 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
>  	return err;
>  }
>  
> -static ssize_t bnep_send_ctrl_rsp(int sk, uint8_t type, uint8_t ctrl,
> -								uint16_t resp)
> +static ssize_t bnep_send_ctrl_rsp(int sk, uint8_t ctrl, uint16_t resp)
>  {
> -	struct bnep_control_rsp rsp;
> +	ssize_t sent;
>  
> -	rsp.type = type;
> -	rsp.ctrl = ctrl;
> -	rsp.resp = htons(resp);
> +	switch (ctrl) {
> +	case BNEP_CMD_NOT_UNDERSTOOD: {
> +		struct bnep_ctrl_cmd_not_understood_cmd rsp;
>  
> -	return send(sk, &rsp, sizeof(rsp), 0);
> +		rsp.type = BNEP_CONTROL;
> +		rsp.ctrl = ctrl;
> +		rsp.unkn_ctrl = (uint8_t) resp;
> +
> +		sent = send(sk, &rsp, sizeof(rsp), 0);
> +		break;
> +	}
> +	case BNEP_FILTER_MULT_ADDR_RSP:
> +	case BNEP_FILTER_NET_TYPE_RSP:
> +	case BNEP_SETUP_CONN_RSP: {
> +		struct bnep_control_rsp rsp;
> +
> +		rsp.type = BNEP_CONTROL;
> +		rsp.ctrl = ctrl;
> +		rsp.resp = htons(resp);
> +
> +		sent = send(sk, &rsp, sizeof(rsp), 0);
> +		break;
> +	}
> +	default:
> +		error("wrong bnep response type");
> +		sent = -1;
> +		break;
> +	}
> +
> +	return sent;
>  }
>  
>  static uint16_t bnep_setup_decode(int sk, struct bnep_setup_conn_req *req,
> @@ -637,16 +661,15 @@ int bnep_server_add(int sk, char *bridge, char *iface, const bdaddr_t *addr,
>  	/* Highest known Control command ID
>  	 * is BNEP_FILTER_MULT_ADDR_RSP = 0x06 */
>  	if (req->type == BNEP_CONTROL &&
> -				req->ctrl > BNEP_FILTER_MULT_ADDR_RSP) {
> -		uint8_t pkt[3];
> -
> -		pkt[0] = BNEP_CONTROL;
> -		pkt[1] = BNEP_CMD_NOT_UNDERSTOOD;
> -		pkt[2] = req->ctrl;
> +					req->ctrl > BNEP_FILTER_MULT_ADDR_RSP) {
> +		error("cmd not understood");

Please prefix error/info with "bnep: "

> +		err = bnep_send_ctrl_rsp(sk, BNEP_CMD_NOT_UNDERSTOOD,
> +								req->ctrl);
> +		if (err < 0)
> +			error("send not understood ctrl rsp error: %s (%d)",
> +							strerror(errno), errno);
>  
> -		send(sk, pkt, sizeof(pkt), 0);
> -
> -		return -EINVAL;
> +		return err;
>  	}
>  
>  	/* Processing BNEP_SETUP_CONNECTION_REQUEST_MSG */
> @@ -681,7 +704,9 @@ int bnep_server_add(int sk, char *bridge, char *iface, const bdaddr_t *addr,
>  		rsp = BNEP_CONN_NOT_ALLOWED;
>  
>  reply:
> -	bnep_send_ctrl_rsp(sk, BNEP_CONTROL, BNEP_SETUP_CONN_RSP, rsp);
> +	err = bnep_send_ctrl_rsp(sk, BNEP_SETUP_CONN_RSP, rsp);
> +	if (err < 0)
> +		error("send ctrl rsp error: %s (%d)", strerror(errno), errno);
>  
>  	return err;
>  }
> 

-- 
Best regards, 
Szymon Janc

[PATCH 07/12] profiles/network: Keep interface arguments naming consistent

[Grzegorz Kolodziejczyk]

Arguments shall keep consistent naming.
---
 profiles/network/bnep.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
index a9d9f98..36ad904 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -35,7 +35,7 @@ struct bnep *bnep_new(int sk, uint16_t local_role, uint16_t remote_role,
 void bnep_free(struct bnep *session);
 
 typedef void (*bnep_connect_cb) (char *iface, int err, void *data);
-int bnep_connect(struct bnep *b, bnep_connect_cb conn_cb, void *data);
+int bnep_connect(struct bnep *session, bnep_connect_cb conn_cb, void *data);
 typedef void (*bnep_disconnect_cb) (void *data);
 void bnep_set_disconnect(struct bnep *session, bnep_disconnect_cb disconn_cb,
 								void *data);
-- 
2.1.0

[PATCH 08/12] profiles/network: Shortening service name argument to srv

[Grzegorz Kolodziejczyk]

Current argument naming is long and it can be simplified to srv.
---
 profiles/network/connection.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/profiles/network/connection.c b/profiles/network/connection.c
index cc73989..19a0d8d 100644
--- a/profiles/network/connection.c
+++ b/profiles/network/connection.c
@@ -78,9 +78,9 @@ struct network_conn {
 
 static GSList *peers = NULL;
 
-static uint16_t get_service_id(struct btd_service *service)
+static uint16_t get_service_id(struct btd_service *srv)
 {
-	return bnep_service_id(btd_service_get_profile(service)->remote_uuid);
+	return bnep_service_id(btd_service_get_profile(srv)->remote_uuid);
 }
 
 static struct network_peer *find_peer(GSList *list, struct btd_device *device)
@@ -308,11 +308,11 @@ static DBusMessage *local_connect(DBusConnection *conn,
 }
 
 /* Connect and initiate BNEP session */
-int connection_connect(struct btd_service *service)
+int connection_connect(struct btd_service *srv)
 {
-	struct network_conn *nc = btd_service_get_user_data(service);
+	struct network_conn *nc = btd_service_get_user_data(srv);
 	struct network_peer *peer = nc->peer;
-	uint16_t id = get_service_id(service);
+	uint16_t id = get_service_id(srv);
 	GError *err = NULL;
 	const bdaddr_t *src;
 	const bdaddr_t *dst;
@@ -342,9 +342,9 @@ int connection_connect(struct btd_service *service)
 	return 0;
 }
 
-int connection_disconnect(struct btd_service *service)
+int connection_disconnect(struct btd_service *srv)
 {
-	struct network_conn *nc = btd_service_get_user_data(service);
+	struct network_conn *nc = btd_service_get_user_data(srv);
 
 	if (nc->state == DISCONNECTED)
 		return 0;
@@ -496,12 +496,12 @@ static const GDBusPropertyTable connection_properties[] = {
 	{ }
 };
 
-void connection_unregister(struct btd_service *service)
+void connection_unregister(struct btd_service *srv)
 {
-	struct btd_device *device = btd_service_get_device(service);
-	struct network_conn *conn = btd_service_get_user_data(service);
+	struct btd_device *device = btd_service_get_device(srv);
+	struct network_conn *conn = btd_service_get_user_data(srv);
 	struct network_peer *peer = conn->peer;
-	uint16_t id = get_service_id(service);
+	uint16_t id = get_service_id(srv);
 
 	DBG("%s id %u", device_get_path(device), id);
 
@@ -543,12 +543,12 @@ static struct network_peer *create_peer(struct btd_device *device)
 	return peer;
 }
 
-int connection_register(struct btd_service *service)
+int connection_register(struct btd_service *srv)
 {
-	struct btd_device *device = btd_service_get_device(service);
+	struct btd_device *device = btd_service_get_device(srv);
 	struct network_peer *peer;
 	struct network_conn *nc;
-	uint16_t id = get_service_id(service);
+	uint16_t id = get_service_id(srv);
 
 	DBG("%s id %u", device_get_path(device), id);
 
@@ -562,11 +562,11 @@ int connection_register(struct btd_service *service)
 
 	nc = g_new0(struct network_conn, 1);
 	nc->id = id;
-	nc->service = btd_service_ref(service);
+	nc->service = btd_service_ref(srv);
 	nc->state = DISCONNECTED;
 	nc->peer = peer;
 
-	btd_service_set_user_data(service, nc);
+	btd_service_set_user_data(srv, nc);
 
 	DBG("id %u registered", id);
 
-- 
2.1.0

[PATCH 09/12] profiles/network: Make get_service_id connections private method

[Grzegorz Kolodziejczyk]

get_service_id method is used only within connection code to get id (16
bit value) of received string format 128bit uuid or service named e.g.
panu, gn or nap. There is no need to make it public as bnep.c only
related code.
---
 profiles/network/bnep.c       | 25 -------------------------
 profiles/network/bnep.h       |  1 -
 profiles/network/connection.c | 20 ++++++++++++++------
 3 files changed, 14 insertions(+), 32 deletions(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 132dbab..d784427 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -85,31 +85,6 @@ struct bnep {
 	void	*disconn_data;
 };
 
-uint16_t bnep_service_id(const char *svc)
-{
-	int i;
-	uint16_t id;
-
-	/* Friendly service name */
-	for (i = 0; __svc[i].name; i++) {
-		if (!strcasecmp(svc, __svc[i].name))
-			return __svc[i].id;
-	}
-
-	/* UUID 128 string */
-	for (i = 0; __svc[i].uuid128; i++) {
-		if (!strcasecmp(svc, __svc[i].uuid128))
-			return __svc[i].id;
-	}
-
-	/* Try convert to HEX */
-	id = strtol(svc, NULL, 16);
-	if ((id < BNEP_SVC_PANU) || (id > BNEP_SVC_GN))
-		return 0;
-
-	return id;
-}
-
 const char *bnep_uuid(uint16_t id)
 {
 	int i;
diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
index 36ad904..0c1669c 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -26,7 +26,6 @@ struct bnep;
 int bnep_init(void);
 int bnep_cleanup(void);
 
-uint16_t bnep_service_id(const char *svc);
 const char *bnep_uuid(uint16_t id);
 const char *bnep_name(uint16_t id);
 
diff --git a/profiles/network/connection.c b/profiles/network/connection.c
index 19a0d8d..cbcf39b 100644
--- a/profiles/network/connection.c
+++ b/profiles/network/connection.c
@@ -46,6 +46,7 @@
 #include "src/profile.h"
 #include "src/service.h"
 #include "src/error.h"
+#include "lib/uuid.h"
 
 #include "bnep.h"
 #include "connection.h"
@@ -78,9 +79,16 @@ struct network_conn {
 
 static GSList *peers = NULL;
 
-static uint16_t get_service_id(struct btd_service *srv)
+static uint16_t get_pan_srv_id(const char *svc)
 {
-	return bnep_service_id(btd_service_get_profile(srv)->remote_uuid);
+	if (!strcasecmp(svc, "panu") || !strcasecmp(svc, PANU_UUID))
+		return BNEP_SVC_PANU;
+	if (!strcasecmp(svc, "nap") || !strcasecmp(svc, NAP_UUID))
+		return BNEP_SVC_NAP;
+	if (!strcasecmp(svc, "gn") || !strcasecmp(svc, GN_UUID))
+		return BNEP_SVC_GN;
+
+	return 0;
 }
 
 static struct network_peer *find_peer(GSList *list, struct btd_device *device)
@@ -283,7 +291,7 @@ static DBusMessage *local_connect(DBusConnection *conn,
 						DBUS_TYPE_INVALID) == FALSE)
 		return btd_error_invalid_args(msg);
 
-	id = bnep_service_id(svc);
+	id = get_pan_srv_id(svc);
 	uuid = bnep_uuid(id);
 
 	if (uuid == NULL)
@@ -312,7 +320,7 @@ int connection_connect(struct btd_service *srv)
 {
 	struct network_conn *nc = btd_service_get_user_data(srv);
 	struct network_peer *peer = nc->peer;
-	uint16_t id = get_service_id(srv);
+	uint16_t id = get_pan_srv_id(btd_service_get_profile(srv)->remote_uuid);
 	GError *err = NULL;
 	const bdaddr_t *src;
 	const bdaddr_t *dst;
@@ -501,7 +509,7 @@ void connection_unregister(struct btd_service *srv)
 	struct btd_device *device = btd_service_get_device(srv);
 	struct network_conn *conn = btd_service_get_user_data(srv);
 	struct network_peer *peer = conn->peer;
-	uint16_t id = get_service_id(srv);
+	uint16_t id = get_pan_srv_id(btd_service_get_profile(srv)->remote_uuid);
 
 	DBG("%s id %u", device_get_path(device), id);
 
@@ -548,7 +556,7 @@ int connection_register(struct btd_service *srv)
 	struct btd_device *device = btd_service_get_device(srv);
 	struct network_peer *peer;
 	struct network_conn *nc;
-	uint16_t id = get_service_id(srv);
+	uint16_t id = get_pan_srv_id(btd_service_get_profile(srv)->remote_uuid);
 
 	DBG("%s id %u", device_get_path(device), id);
 
-- 
2.1.0

[PATCH 10/12] profiles/network: Remove unneded bnep_uuid function from bnep code

[Grzegorz Kolodziejczyk]

This funcion is no longer needed since connection and server can handle
this funcionality by itself.
---
 profiles/network/bnep.c       | 10 ----------
 profiles/network/bnep.h       |  1 -
 profiles/network/connection.c | 19 ++++++++++++-------
 profiles/network/server.c     | 24 +++++++++++++++++++++---
 4 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index d784427..0a73e54 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -85,16 +85,6 @@ struct bnep {
 	void	*disconn_data;
 };
 
-const char *bnep_uuid(uint16_t id)
-{
-	int i;
-
-	for (i = 0; __svc[i].uuid128; i++)
-		if (__svc[i].id == id)
-			return __svc[i].uuid128;
-	return NULL;
-}
-
 const char *bnep_name(uint16_t id)
 {
 	int i;
diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
index 0c1669c..21a09f6 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -26,7 +26,6 @@ struct bnep;
 int bnep_init(void);
 int bnep_cleanup(void);
 
-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,
diff --git a/profiles/network/connection.c b/profiles/network/connection.c
index cbcf39b..1cd3293 100644
--- a/profiles/network/connection.c
+++ b/profiles/network/connection.c
@@ -283,21 +283,23 @@ static DBusMessage *local_connect(DBusConnection *conn,
 	struct btd_service *service;
 	struct network_conn *nc;
 	const char *svc;
-	const char *uuid;
 	uint16_t id;
 	int err;
+	char uuid_str[MAX_LEN_UUID_STR];
+	bt_uuid_t uuid;
 
 	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &svc,
 						DBUS_TYPE_INVALID) == FALSE)
 		return btd_error_invalid_args(msg);
 
 	id = get_pan_srv_id(svc);
-	uuid = bnep_uuid(id);
+	bt_uuid16_create(&uuid, id);
+	bt_uuid_to_uuid128(&uuid, &uuid);
 
-	if (uuid == NULL)
+	if (bt_uuid_to_string(&uuid, uuid_str, MAX_LEN_UUID_STR) < 0)
 		return btd_error_invalid_args(msg);
 
-	service = btd_device_get_service(peer->device, uuid);
+	service = btd_device_get_service(peer->device, uuid_str);
 	if (service == NULL)
 		return btd_error_not_supported(msg);
 
@@ -438,15 +440,18 @@ static gboolean network_property_get_uuid(const GDBusPropertyTable *property,
 {
 	struct network_peer *peer = data;
 	struct network_conn *nc;
-	const char *uuid;
+	char uuid_str[MAX_LEN_UUID_STR];
+	bt_uuid_t uuid;
 
 	nc = find_connection_by_state(peer->connections, CONNECTED);
 	if (nc == NULL)
 		return FALSE;
 
-	uuid = bnep_uuid(nc->id);
+	bt_uuid16_create(&uuid, nc->id);
+	bt_uuid_to_uuid128(&uuid, &uuid);
+	bt_uuid_to_string(&uuid, uuid_str, MAX_LEN_UUID_STR);
 
-	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &uuid);
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &uuid_str);
 
 	return TRUE;
 }
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 8e9afbc..21eac5d 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -113,14 +113,32 @@ static struct network_server *find_server(GSList *list, uint16_t id)
 static struct network_server *find_server_by_uuid(GSList *list,
 							const char *uuid)
 {
+	bt_uuid_t srv_uuid, bnep_uuid;
+
 	for (; list; list = list->next) {
 		struct network_server *ns = list->data;
+		bt_uuid16_create(&bnep_uuid, ns->id);
 
-		if (strcasecmp(uuid, bnep_uuid(ns->id)) == 0)
+		/* UUID value compare */
+		if (!bt_string_to_uuid(&srv_uuid, uuid) &&
+					!bt_uuid_cmp(&srv_uuid, &bnep_uuid))
 			return ns;
 
-		if (strcasecmp(uuid, bnep_name(ns->id)) == 0)
-			return ns;
+		/* String value compare */
+		switch (ns->id) {
+		case BNEP_SVC_PANU:
+			if (!strcasecmp(uuid, "panu"))
+				return ns;
+			break;
+		case BNEP_SVC_NAP:
+			if (!strcasecmp(uuid, "nap"))
+				return ns;
+			break;
+		case BNEP_SVC_GN:
+			if (!strcasecmp(uuid, "gn"))
+				return ns;
+			break;
+		}
 	}
 
 	return NULL;
-- 
2.1.0

[PATCH 11/12] profiles/network: Remove not needed get name by bnep id function

[Grzegorz Kolodziejczyk]

This function and service structure declaration is no longer needed
since related code was distracted.
---
 profiles/network/bnep.c | 21 ---------------------
 profiles/network/bnep.h |  2 --
 2 files changed, 23 deletions(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 0a73e54..7ec166c 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -54,17 +54,6 @@
 
 static int ctl;
 
-static struct {
-	const char	*name;		/* Friendly name */
-	const char	*uuid128;	/* UUID 128 */
-	uint16_t	id;		/* Service class identifier */
-} __svc[] = {
-	{ "panu",	PANU_UUID,	BNEP_SVC_PANU	},
-	{ "gn",		GN_UUID,	BNEP_SVC_GN	},
-	{ "nap",	NAP_UUID,	BNEP_SVC_NAP	},
-	{ NULL }
-};
-
 struct __service_16 {
 	uint16_t dst;
 	uint16_t src;
@@ -85,16 +74,6 @@ struct bnep {
 	void	*disconn_data;
 };
 
-const char *bnep_name(uint16_t id)
-{
-	int i;
-
-	for (i = 0; __svc[i].name; i++)
-		if (__svc[i].id == id)
-			return __svc[i].name;
-	return NULL;
-}
-
 int bnep_init(void)
 {
 	ctl = socket(PF_BLUETOOTH, SOCK_RAW, BTPROTO_BNEP);
diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
index 21a09f6..31f9b67 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -26,8 +26,6 @@ struct bnep;
 int bnep_init(void);
 int bnep_cleanup(void);
 
-const char *bnep_name(uint16_t id);
-
 struct bnep *bnep_new(int sk, uint16_t local_role, uint16_t remote_role,
 								char *iface);
 void bnep_free(struct bnep *session);
-- 
2.1.0

[PATCH 12/12] profiles/network: Move disconn cb setting to bnep connect method

[Grzegorz Kolodziejczyk]

Disconnect callback can be set while connececting bnep. In previous
implementation there was seperated method to setting up the disconnect
callback and it was always called immediately after calling connect -
this method was redundand.
---
 android/pan.c                 |  5 ++---
 profiles/network/bnep.c       | 22 +++++++---------------
 profiles/network/bnep.h       |  6 +++---
 profiles/network/connection.c |  4 +---
 4 files changed, 13 insertions(+), 24 deletions(-)

diff --git a/android/pan.c b/android/pan.c
index 08c8134..4c4851f 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -324,14 +324,13 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
 	if (!dev->session)
 		goto fail;
 
-	perr = bnep_connect(dev->session, bnep_conn_cb, dev);
+	perr = bnep_connect(dev->session, bnep_conn_cb, bnep_disconn_cb, dev,
+									dev);
 	if (perr < 0) {
 		error("bnep connect req failed: %s", strerror(-perr));
 		goto fail;
 	}
 
-	bnep_set_disconnect(dev->session, bnep_disconn_cb, dev);
-
 	if (dev->io) {
 		g_io_channel_unref(dev->io);
 		dev->io = NULL;
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 7ec166c..2c10572 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -369,17 +369,21 @@ void bnep_free(struct bnep *session)
 	g_free(session);
 }
 
-int bnep_connect(struct bnep *session, bnep_connect_cb conn_cb, void *data)
+int bnep_connect(struct bnep *session, bnep_connect_cb conn_cb,
+					bnep_disconnect_cb disconn_cb,
+					void *conn_data, void *disconn_data)
 {
 	GError *gerr = NULL;
 	int err;
 
-	if (!session || !conn_cb)
+	if (!session || !conn_cb || !disconn_cb)
 		return -EINVAL;
 
 	session->attempts = 0;
 	session->conn_cb = conn_cb;
-	session->conn_data = data;
+	session->disconn_cb = disconn_cb;
+	session->conn_data = conn_data;
+	session->disconn_data = disconn_data;
 
 	bt_io_get(session->io, &gerr, BT_IO_OPT_DEST_BDADDR, &session->dst_addr,
 							BT_IO_OPT_INVALID);
@@ -417,18 +421,6 @@ void bnep_disconnect(struct bnep *session)
 	bnep_conndel(&session->dst_addr);
 }
 
-void bnep_set_disconnect(struct bnep *session, bnep_disconnect_cb disconn_cb,
-								void *data)
-{
-	if (!session || !disconn_cb)
-		return;
-
-	if (!session->disconn_cb && !session->disconn_data) {
-		session->disconn_cb = disconn_cb;
-		session->disconn_data = data;
-	}
-}
-
 static 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 31f9b67..b284699 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -31,10 +31,10 @@ struct bnep *bnep_new(int sk, uint16_t local_role, uint16_t remote_role,
 void bnep_free(struct bnep *session);
 
 typedef void (*bnep_connect_cb) (char *iface, int err, void *data);
-int bnep_connect(struct bnep *session, bnep_connect_cb conn_cb, void *data);
 typedef void (*bnep_disconnect_cb) (void *data);
-void bnep_set_disconnect(struct bnep *session, bnep_disconnect_cb disconn_cb,
-								void *data);
+int bnep_connect(struct bnep *session, bnep_connect_cb conn_cb,
+					bnep_disconnect_cb disconn_cb,
+					void *conn_data, void *disconn_data);
 void bnep_disconnect(struct bnep *session);
 
 int bnep_server_add(int sk, char *bridge, char *iface, const bdaddr_t *addr,
diff --git a/profiles/network/connection.c b/profiles/network/connection.c
index 1cd3293..b12da6e 100644
--- a/profiles/network/connection.c
+++ b/profiles/network/connection.c
@@ -257,14 +257,12 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
 	if (!nc->session)
 		goto failed;
 
-	perr = bnep_connect(nc->session, bnep_conn_cb, nc);
+	perr = bnep_connect(nc->session, bnep_conn_cb, bnep_disconn_cb, nc, nc);
 	if (perr < 0) {
 		error("bnep connect(): %s (%d)", strerror(-perr), -perr);
 		goto failed;
 	}
 
-	bnep_set_disconnect(nc->session, bnep_disconn_cb, nc);
-
 	if (nc->io) {
 		g_io_channel_unref(nc->io);
 		nc->io = NULL;
-- 
2.1.0

Re: [PATCH 01/12] profiles/network: Minor style fix

[Szymon Janc]

Hi Grzegorz,

On Thursday 26 of February 2015 12:53:23 Grzegorz Kolodziejczyk wrote:
> This patch fixes style issues e.g. line aligns, braces.
> ---
>  profiles/network/bnep.c   |  6 ++++--
>  profiles/network/server.c | 11 +++++------
>  2 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> index 09d4b65..e3a2b89 100644
> --- a/profiles/network/bnep.c
> +++ b/profiles/network/bnep.c
> @@ -508,8 +508,9 @@ static int bnep_add_to_bridge(const char *devname, const char *bridge)
>  		err = -errno;
>  		error("bnep: Can't add %s to the bridge %s: %s(%d)",
>  					devname, bridge, strerror(-err), -err);
> -	} else
> +	} else {
>  		info("bridge %s: interface %s added", bridge, devname);
> +	}
>  
>  	close(sk);
>  
> @@ -539,8 +540,9 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
>  		err = -errno;
>  		error("bnep: Can't delete %s from the bridge %s: %s(%d)",
>  					devname, bridge, strerror(-err), -err);
> -	} else
> +	} else {
>  		info("bridge %s: interface %s removed", bridge, devname);
> +	}
>  
>  	close(sk);
>  
> diff --git a/profiles/network/server.c b/profiles/network/server.c
> index 3fb031f..ebbe056 100644
> --- a/profiles/network/server.c
> +++ b/profiles/network/server.c
> @@ -650,8 +650,7 @@ static struct network_adapter *create_adapter(struct btd_adapter *adapter)
>  	na = g_new0(struct network_adapter, 1);
>  	na->adapter = btd_adapter_ref(adapter);
>  
> -	na->io = bt_io_listen(NULL, confirm_event, na,
> -				NULL, &err,
> +	na->io = bt_io_listen(NULL, confirm_event, na, NULL, &err,
>  				BT_IO_OPT_SOURCE_BDADDR,
>  				btd_adapter_get_address(adapter),
>  				BT_IO_OPT_PSM, BNEP_PSM,
> @@ -697,10 +696,10 @@ int server_register(struct btd_adapter *adapter, uint16_t id)
>  	if (g_slist_length(na->servers) > 0)
>  		goto done;
>  
> -	if (!g_dbus_register_interface(btd_get_dbus_connection(),
> -					path, NETWORK_SERVER_INTERFACE,
> -					server_methods, NULL, NULL,
> -					na, path_unregister)) {
> +	if (!g_dbus_register_interface(btd_get_dbus_connection(), path,
> +						NETWORK_SERVER_INTERFACE,
> +						server_methods, NULL, NULL, na,
> +						path_unregister)) {
>  		error("D-Bus failed to register %s interface",
>  						NETWORK_SERVER_INTERFACE);
>  		server_free(ns);

Patches 1 and 2 are now applied, thanks. 

-- 
Best regards, 
Szymon Janc