Renaming function name because it does more than freeing memory.
Also moving disconnect notification call to destory_pan_device
reduce redundancy.
---
android/pan.c | 51 +++++++++++++++++++++++----------------------------
1 file changed, 23 insertions(+), 28 deletions(-)
diff --git a/android/pan.c b/android/pan.c
index e410f54..ec589cf 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -65,24 +65,6 @@ static int device_cmp(gconstpointer s, gconstpointer user_data)
return bacmp(&dev->dst, dst);
}
-static void pan_device_free(struct pan_device *dev)
-{
- local_role = HAL_PAN_ROLE_NONE;
-
- if (dev->watch > 0) {
- g_source_remove(dev->watch);
- dev->watch = 0;
- }
-
- if (dev->io) {
- g_io_channel_unref(dev->io);
- dev->io = NULL;
- }
-
- devices = g_slist_remove(devices, dev);
- g_free(dev);
-}
-
static void bt_pan_notify_conn_state(struct pan_device *dev, uint8_t state)
{
struct hal_ev_pan_conn_state ev;
@@ -121,6 +103,25 @@ static void bt_pan_notify_ctrl_state(struct pan_device *dev, uint8_t state)
&ev);
}
+static void destroy_pan_device(struct pan_device *dev)
+{
+ local_role = HAL_PAN_ROLE_NONE;
+
+ if (dev->watch > 0) {
+ g_source_remove(dev->watch);
+ dev->watch = 0;
+ }
+
+ if (dev->io) {
+ g_io_channel_unref(dev->io);
+ dev->io = NULL;
+ }
+
+ bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
+ devices = g_slist_remove(devices, dev);
+ g_free(dev);
+}
+
static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
gpointer data)
{
@@ -130,8 +131,7 @@ static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
bnep_if_down(dev->iface);
bnep_conndel(&dev->dst);
- bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
- pan_device_free(dev);
+ destroy_pan_device(dev);
return FALSE;
}
@@ -145,8 +145,7 @@ 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);
- pan_device_free(dev);
+ destroy_pan_device(dev);
return;
}
@@ -189,8 +188,7 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
return;
fail:
- bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
- pan_device_free(dev);
+ destroy_pan_device(dev);
}
static void bt_pan_connect(const void *buf, uint16_t len)
@@ -292,10 +290,7 @@ static void bt_pan_disconnect(const void *buf, uint16_t len)
bnep_if_down(dev->iface);
bnep_conndel(&dst);
-
- bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
- pan_device_free(dev);
-
+ destroy_pan_device(dev);
status = HAL_STATUS_SUCCESS;
failed:
--
1.8.3.2
Hi Luiz,
On 17.12.2013 11:19, Luiz Augusto von Dentz wrote:
> Hi Ravi,
>
> On Tue, Dec 17, 2013 at 12:05 AM, Ravi kumar Veeramally
> <[email protected]> wrote:
>> 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 | 37 +++++++++++++++++++++++++++++++++++++
>> profiles/network/bnep.h | 5 +++++
>> 2 files changed, 42 insertions(+)
>>
>> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
>> index 08037e6..d7d8832 100644
>> --- a/profiles/network/bnep.c
>> +++ b/profiles/network/bnep.c
>> @@ -71,6 +71,7 @@ struct bnep_conn {
>> GIOChannel *io;
>> uint16_t src;
>> uint16_t dst;
>> + bdaddr_t dst_addr;
>> guint attempts;
>> guint setup_to;
>> void *data;
>> @@ -246,6 +247,42 @@ int bnep_if_down(const char *devname)
>> return 0;
>> }
>>
>> +struct bnep_conn *bnep_new(uint16_t src, uint16_t dst,
>> + const bdaddr_t *dst_addr)
> I would change the order of the parameters, also do not use bdaddr_t
> otherwise you wont be able to do unit tests with it, something like
> bnep_new(int fd, uint16_t local_role, uint16_t remote_role) looks
> better, but perhaps you gonna need the MTU as well.
bdaddr_t is required incase if it unable to up the interface, connection
needs to be deleted and it is difficult to track on which error we have to
delete the connection. And one more thing I am moving many of these
apis to local to the bnep.c.
Regarding MTU at least now it is not required by connection.c or
android/pan.c.
>> +{
>> + struct bnep_conn *bc;
>> +
>> + DBG("");
>> +
>> + if (!dst_addr)
>> + return NULL;
>> +
>> + bc = g_new0(struct bnep_conn, 1);
>> + if (!bc)
>> + return NULL;
> No need to check the return of g_new0, if it fails it will exit so
> this code will never be triggered.
Ok.
Thanks,
Ravi.
Hi Johan,
On 17.12.2013 11:27, Johan Hedberg wrote:
> Hi Ravi,
>
> On Tue, Dec 17, 2013, Ravi kumar Veeramally wrote:
>> Renaming for easy readability.
>> ---
>> android/pan.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/android/pan.c b/android/pan.c
>> index ec589cf..03db350 100644
>> --- a/android/pan.c
>> +++ b/android/pan.c
>> @@ -162,7 +162,7 @@ static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
>> dev->io = NULL;
>> }
>>
>> -static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
>> +static void bt_io_connect_cb(GIOChannel *chan, GError *err, gpointer data)
>> {
>> struct pan_device *dev = data;
>> uint16_t src, dst;
>> @@ -238,7 +238,7 @@ static void bt_pan_connect(const void *buf, uint16_t len)
>> ba2str(&dev->dst, addr);
>> DBG("connecting to %s %s", addr, dev->iface);
>>
>> - dev->io = bt_io_connect(connect_cb, dev, NULL, &gerr,
>> + dev->io = bt_io_connect(bt_io_connect_cb, dev, NULL, &gerr,
>> BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
>> BT_IO_OPT_DEST_BDADDR, &dev->dst,
>> BT_IO_OPT_PSM, BNEP_PSM,
> This might actually cause more confusion since the assumption is that
> bt_io_* prefixed symbols are part of the btio code. The current naming doesn't
> bother me, but if you wanna suggest something else like pan_connected
> I'm fine with that too).
Ok, ignore this patch, let the same cb name continue.
Ravi.
Hi Ravi,
On Tue, Dec 17, 2013, Ravi kumar Veeramally wrote:
> Renaming for easy readability.
> ---
> android/pan.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/android/pan.c b/android/pan.c
> index ec589cf..03db350 100644
> --- a/android/pan.c
> +++ b/android/pan.c
> @@ -162,7 +162,7 @@ static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
> dev->io = NULL;
> }
>
> -static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
> +static void bt_io_connect_cb(GIOChannel *chan, GError *err, gpointer data)
> {
> struct pan_device *dev = data;
> uint16_t src, dst;
> @@ -238,7 +238,7 @@ static void bt_pan_connect(const void *buf, uint16_t len)
> ba2str(&dev->dst, addr);
> DBG("connecting to %s %s", addr, dev->iface);
>
> - dev->io = bt_io_connect(connect_cb, dev, NULL, &gerr,
> + dev->io = bt_io_connect(bt_io_connect_cb, dev, NULL, &gerr,
> BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
> BT_IO_OPT_DEST_BDADDR, &dev->dst,
> BT_IO_OPT_PSM, BNEP_PSM,
This might actually cause more confusion since the assumption is that
bt_io_* prefixed symbols are part of the btio code. The current naming doesn't
bother me, but if you wanna suggest something else like pan_connected
I'm fine with that too).
Johan
Hi Ravi,
On Tue, Dec 17, 2013 at 12:05 AM, Ravi kumar Veeramally
<[email protected]> wrote:
> 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 | 37 +++++++++++++++++++++++++++++++++++++
> profiles/network/bnep.h | 5 +++++
> 2 files changed, 42 insertions(+)
>
> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> index 08037e6..d7d8832 100644
> --- a/profiles/network/bnep.c
> +++ b/profiles/network/bnep.c
> @@ -71,6 +71,7 @@ struct bnep_conn {
> GIOChannel *io;
> uint16_t src;
> uint16_t dst;
> + bdaddr_t dst_addr;
> guint attempts;
> guint setup_to;
> void *data;
> @@ -246,6 +247,42 @@ int bnep_if_down(const char *devname)
> return 0;
> }
>
> +struct bnep_conn *bnep_new(uint16_t src, uint16_t dst,
> + const bdaddr_t *dst_addr)
I would change the order of the parameters, also do not use bdaddr_t
otherwise you wont be able to do unit tests with it, something like
bnep_new(int fd, uint16_t local_role, uint16_t remote_role) looks
better, but perhaps you gonna need the MTU as well.
> +{
> + struct bnep_conn *bc;
> +
> + DBG("");
> +
> + if (!dst_addr)
> + return NULL;
> +
> + bc = g_new0(struct bnep_conn, 1);
> + if (!bc)
> + return NULL;
No need to check the return of g_new0, if it fails it will exit so
this code will never be triggered.
Hi Ravi,
On Tue, Dec 17, 2013 at 12:05 AM, Ravi kumar Veeramally
<[email protected]> wrote:
> Renaming function name because it does more than freeing memory.
> Also moving disconnect notification call to destory_pan_device
> reduce redundancy.
> ---
> android/pan.c | 51 +++++++++++++++++++++++----------------------------
> 1 file changed, 23 insertions(+), 28 deletions(-)
>
> diff --git a/android/pan.c b/android/pan.c
> index e410f54..ec589cf 100644
> --- a/android/pan.c
> +++ b/android/pan.c
> @@ -65,24 +65,6 @@ static int device_cmp(gconstpointer s, gconstpointer user_data)
> return bacmp(&dev->dst, dst);
> }
>
> -static void pan_device_free(struct pan_device *dev)
> -{
> - local_role = HAL_PAN_ROLE_NONE;
> -
> - if (dev->watch > 0) {
> - g_source_remove(dev->watch);
> - dev->watch = 0;
> - }
> -
> - if (dev->io) {
> - g_io_channel_unref(dev->io);
> - dev->io = NULL;
> - }
> -
> - devices = g_slist_remove(devices, dev);
> - g_free(dev);
> -}
> -
> static void bt_pan_notify_conn_state(struct pan_device *dev, uint8_t state)
> {
> struct hal_ev_pan_conn_state ev;
> @@ -121,6 +103,25 @@ static void bt_pan_notify_ctrl_state(struct pan_device *dev, uint8_t state)
> &ev);
> }
>
> +static void destroy_pan_device(struct pan_device *dev)
> +{
> + local_role = HAL_PAN_ROLE_NONE;
> +
> + if (dev->watch > 0) {
> + g_source_remove(dev->watch);
> + dev->watch = 0;
> + }
> +
> + if (dev->io) {
> + g_io_channel_unref(dev->io);
> + dev->io = NULL;
> + }
> +
> + bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
> + devices = g_slist_remove(devices, dev);
> + g_free(dev);
> +}
> +
> static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
> gpointer data)
> {
> @@ -130,8 +131,7 @@ static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
>
> bnep_if_down(dev->iface);
> bnep_conndel(&dev->dst);
> - bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
> - pan_device_free(dev);
> + destroy_pan_device(dev);
>
> return FALSE;
> }
> @@ -145,8 +145,7 @@ 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);
> - pan_device_free(dev);
> + destroy_pan_device(dev);
> return;
> }
>
> @@ -189,8 +188,7 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
> return;
>
> fail:
> - bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
> - pan_device_free(dev);
> + destroy_pan_device(dev);
> }
>
> static void bt_pan_connect(const void *buf, uint16_t len)
> @@ -292,10 +290,7 @@ static void bt_pan_disconnect(const void *buf, uint16_t len)
>
> bnep_if_down(dev->iface);
> bnep_conndel(&dst);
> -
> - bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
> - pan_device_free(dev);
> -
> + destroy_pan_device(dev);
> status = HAL_STATUS_SUCCESS;
>
> failed:
> --
> 1.8.3.2
Have a look at android/a2dp.c, after sending disconnected you can
probably free the device since there is no use for it while
disconnected.
--
Luiz Augusto von Dentz
Providing single api to validate bnep setup resp and hide
other functions.
---
profiles/network/bnep.c | 49 ++++++++++++++++++++++++++++++++++++++++++-----
profiles/network/bnep.h | 4 +---
profiles/network/server.c | 46 +++++++++-----------------------------------
3 files changed, 54 insertions(+), 45 deletions(-)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 5deac81..6e1af74 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -535,19 +535,19 @@ 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)
+static 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_SUCCESS;
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_SUCCESS;
return BNEP_CONN_INVALID_SRC;
}
@@ -555,8 +555,8 @@ uint16_t bnep_setup_chk(uint16_t dst, uint16_t src)
return BNEP_CONN_INVALID_DST;
}
-uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
- uint16_t *src)
+static uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req,
+ uint16_t *dst, uint16_t *src)
{
const uint8_t bt_base[] = { 0x00, 0x00, 0x10, 0x00, 0x80, 0x00,
0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB };
@@ -602,6 +602,45 @@ uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
return BNEP_SUCCESS;
}
+int bnep_validate_setup_rsp(int sk, uint16_t *dst)
+{
+ uint8_t packet[BNEP_MTU];
+ struct bnep_setup_conn_req *req = (void *) packet;
+ uint16_t src;
+ uint8_t pkt[3];
+ int n, rsp = BNEP_CONN_NOT_ALLOWED;
+
+ /* Reading BNEP_SETUP_CONNECTION_REQUEST_MSG */
+ n = read(sk, packet, sizeof(packet));
+ if (n < 0) {
+ error("read(): %s(%d)", strerror(errno), errno);
+ return n;
+ }
+
+ /* Highest known Control command ID
+ * is BNEP_FILTER_MULT_ADDR_RSP = 0x06 */
+ if (req->type == BNEP_CONTROL &&
+ req->ctrl > BNEP_FILTER_MULT_ADDR_RSP) {
+ pkt[0] = BNEP_CONTROL;
+ pkt[1] = BNEP_CMD_NOT_UNDERSTOOD;
+ pkt[2] = req->ctrl;
+
+ send(sk, pkt, sizeof(pkt), 0);
+ return -EINVAL;
+ }
+
+ if (req->type != BNEP_CONTROL || req->ctrl != BNEP_SETUP_CONN_REQ)
+ return -EINVAL;
+
+ rsp = bnep_setup_decode(req, dst, &src);
+ if (rsp)
+ return rsp;
+
+ rsp = bnep_setup_chk(*dst, src);
+
+ return rsp;
+}
+
int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
const bdaddr_t *addr)
{
diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
index 734055d..36cfee0 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -43,7 +43,5 @@ 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);
+int bnep_validate_setup_rsp(int sk, uint16_t *dst);
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 7cb5a1e..432b6d5 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -282,10 +282,8 @@ static gboolean bnep_setup(GIOChannel *chan,
{
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;
- int n, sk;
+ uint16_t dst;
+ int sk, rsp = BNEP_CONN_NOT_ALLOWED;
if (cond & G_IO_NVAL)
return FALSE;
@@ -296,45 +294,18 @@ static gboolean bnep_setup(GIOChannel *chan,
}
sk = g_io_channel_unix_get_fd(chan);
-
- /* Reading BNEP_SETUP_CONNECTION_REQUEST_MSG */
- n = read(sk, packet, sizeof(packet));
- if (n < 0) {
- error("read(): %s(%d)", strerror(errno), errno);
- 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];
-
- pkt[0] = BNEP_CONTROL;
- pkt[1] = BNEP_CMD_NOT_UNDERSTOOD;
- pkt[2] = req->ctrl;
-
- send(sk, pkt, sizeof(pkt), 0);
-
- return FALSE;
- }
-
- if (req->type != BNEP_CONTROL || req->ctrl != BNEP_SETUP_CONN_REQ)
+ rsp = bnep_validate_setup_rsp(sk, &dst);
+ if (rsp < 0)
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 > 0)
goto reply;
rsp = BNEP_CONN_NOT_ALLOWED;
- ns = find_server(na->servers, dst_role);
+ ns = find_server(na->servers, dst);
if (!ns) {
- error("Server unavailable: (0x%x)", dst_role);
+ error("Server unavailable: (0x%x)", dst);
goto reply;
}
@@ -348,10 +319,11 @@ static gboolean bnep_setup(GIOChannel *chan,
goto reply;
}
- if (bnep_server_add(sk, dst_role, ns->bridge, na->setup->dev,
+ if (bnep_server_add(sk, dst, ns->bridge, na->setup->dev,
&na->setup->dst) < 0)
goto reply;
+ ns->sessions = g_slist_append(ns->sessions, na->setup);
na->setup = NULL;
rsp = BNEP_SUCCESS;
--
1.8.3.2
Refactored bnep connect and disconnect calls to simply and
keeping bnep related functionality behind curtains.
Provided bnep_conn struct globally. bnep_connect calls takes
care of bnep_setup until interface up then connect callback
will be called, disconnect_cb is registered as watchdog and it
will triggered by remote device on disconnect or any other
I/O error. bnep_disconnect should be called only when iface is
up/connected.
---
android/pan.c | 56 +++++++--------------
profiles/network/bnep.c | 112 +++++++++++++++++++++++++-----------------
profiles/network/bnep.h | 10 ++--
profiles/network/connection.c | 50 ++++++++++++-------
4 files changed, 122 insertions(+), 106 deletions(-)
diff --git a/android/pan.c b/android/pan.c
index 03db350..0cae6ac 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -54,7 +54,7 @@ struct pan_device {
uint8_t conn_state;
uint8_t role;
GIOChannel *io;
- guint watch;
+ struct bnep_conn *bnep;
};
static int device_cmp(gconstpointer s, gconstpointer user_data)
@@ -105,38 +105,31 @@ static void bt_pan_notify_ctrl_state(struct pan_device *dev, uint8_t state)
static void destroy_pan_device(struct pan_device *dev)
{
- local_role = HAL_PAN_ROLE_NONE;
-
- if (dev->watch > 0) {
- g_source_remove(dev->watch);
- dev->watch = 0;
- }
-
if (dev->io) {
+ g_io_channel_shutdown(dev->io, TRUE, NULL);
g_io_channel_unref(dev->io);
dev->io = NULL;
}
+ if (dev->conn_state == HAL_PAN_STATE_CONNECTED)
+ bnep_disconnect(dev->bnep);
+
bt_pan_notify_conn_state(dev, HAL_PAN_STATE_DISCONNECTED);
+ bnep_free(dev->bnep);
devices = g_slist_remove(devices, dev);
g_free(dev);
}
-static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
- gpointer data)
+static void bnep_disconn_cb(void *data)
{
struct pan_device *dev = data;
DBG("%s disconnected", dev->iface);
- bnep_if_down(dev->iface);
- bnep_conndel(&dev->dst);
destroy_pan_device(dev);
-
- return FALSE;
}
-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 pan_device *dev = data;
@@ -144,22 +137,15 @@ 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);
destroy_pan_device(dev);
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 bt_io_connect_cb(GIOChannel *chan, GError *err, gpointer data)
@@ -179,7 +165,13 @@ static void bt_io_connect_cb(GIOChannel *chan, GError *err, gpointer data)
dst = (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->bnep = bnep_new(src, dst, &dev->dst);
+ if (!dev->bnep) {
+ error("unable to allocate memory for dev->bnep");
+ goto fail;
+ }
+
+ perr = bnep_connect(dev->bnep, sk, bnep_conn_cb, bnep_disconn_cb, dev);
if (perr < 0) {
error("bnep connect req failed: %s", strerror(-perr));
goto fail;
@@ -267,7 +259,7 @@ static void bt_pan_disconnect(const void *buf, uint16_t len)
{
const struct hal_cmd_pan_disconnect *cmd = buf;
struct pan_device *dev;
- uint8_t status;
+ uint8_t status = HAL_STATUS_FAILED;
GSList *l;
bdaddr_t dst;
@@ -276,20 +268,10 @@ static void bt_pan_disconnect(const void *buf, uint16_t len)
android2bdaddr(&cmd->bdaddr, &dst);
l = g_slist_find_custom(devices, &dst, device_cmp);
- if (!l) {
- status = HAL_STATUS_FAILED;
+ if (!l)
goto failed;
- }
dev = l->data;
-
- if (dev->watch) {
- g_source_remove(dev->watch);
- dev->watch = 0;
- }
-
- bnep_if_down(dev->iface);
- bnep_conndel(&dst);
destroy_pan_device(dev);
status = HAL_STATUS_SUCCESS;
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index d7d8832..752e00e 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -72,25 +72,15 @@ struct bnep_conn {
uint16_t src;
uint16_t dst;
bdaddr_t dst_addr;
+ char iface[16];
guint attempts;
guint setup_to;
void *data;
- bnep_connect_cb conn_cb;
+ bnep_connect_cb conn_cb;
+ bnep_disconnect_cb disconn_cb;
+ int watch;
};
-static void free_bnep_connect(struct bnep_conn *bc)
-{
- if (!bc)
- return;
-
- if (bc->io) {
- g_io_channel_unref(bc->io);
- bc->io = NULL;
- }
-
- g_free(bc);
-}
-
uint16_t bnep_service_id(const char *svc)
{
int i;
@@ -268,19 +258,16 @@ struct bnep_conn *bnep_new(uint16_t src, uint16_t dst,
return bc;
}
-void bnep_free(struct bnep_conn *bc)
+static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
+ gpointer data)
{
- DBG("");
+ struct bnep_conn *bc = data;
- if (!bc)
- return;
+ DBG("");
- if (bc->io) {
- g_io_channel_unref(bc->io);
- bc->io = NULL;
- }
+ bc->disconn_cb(bc->data);
- g_free(bc);
+ return FALSE;
}
static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
@@ -290,12 +277,11 @@ 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;
if (cond & G_IO_NVAL)
- goto failed;
+ return FALSE;
if (bc->setup_to > 0) {
g_source_remove(bc->setup_to);
@@ -347,24 +333,28 @@ 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(bc->io);
- if (bnep_connadd(sk, bc->src, iface)) {
+ if (bnep_connadd(sk, bc->src, bc->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(bc->iface)) {
+ error("could not up %s", bc->iface);
+ bnep_conndel(&bc->dst_addr);
goto failed;
}
- bc->conn_cb(chan, iface, 0, bc->data);
- free_bnep_connect(bc);
+ bc->watch = g_io_add_watch(bc->io, G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+ bnep_watchdog_cb, bc);
+ g_io_channel_unref(bc->io);
+ bc->io = NULL;
+
+ bc->conn_cb(bc->iface, 0, bc->data);
return FALSE;
failed:
- bc->conn_cb(NULL, NULL, -EIO, bc->data);
- free_bnep_connect(bc);
+ bc->conn_cb(NULL, -EIO, bc->data);
return FALSE;
}
@@ -408,40 +398,72 @@ static gboolean bnep_conn_req_to(gpointer user_data)
return TRUE;
}
- bc->conn_cb(NULL, NULL, -ETIMEDOUT, bc->data);
- free_bnep_connect(bc);
+ bc->conn_cb(NULL, -ETIMEDOUT, bc->data);
return FALSE;
}
-int bnep_connect(int sk, uint16_t src, uint16_t dst, bnep_connect_cb conn_cb,
- void *data)
+int bnep_connect(struct bnep_conn *bc, int sk, bnep_connect_cb conn_cb,
+ bnep_disconnect_cb disconn_cb, void *data)
{
- struct bnep_conn *bc;
int err;
- if (!conn_cb)
+ if (!bc || !conn_cb || !disconn_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;
+ bc->conn_cb = conn_cb;
+ bc->disconn_cb = disconn_cb;
err = bnep_setup_conn_req(bc);
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_conn_req_to, bc);
+ bc->watch = g_io_add_watch(bc->io,
+ G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
bnep_setup_cb, bc);
return 0;
}
+void bnep_disconnect(struct bnep_conn *bc)
+{
+ DBG("");
+
+ if (!bc)
+ return;
+
+ if (bc->io) {
+ g_io_channel_unref(bc->io);
+ bc->io = NULL;
+ }
+
+ if (bc->watch) {
+ g_source_remove(bc->watch);
+ bc->watch = 0;
+ }
+
+ bnep_if_down(bc->iface);
+ bnep_conndel(&bc->dst_addr);
+}
+
+void bnep_free(struct bnep_conn *bc)
+{
+ DBG("");
+
+ if (!bc)
+ return;
+
+ if (bc->io) {
+ g_io_channel_unref(bc->io);
+ bc->io = NULL;
+ }
+
+ g_free(bc);
+}
+
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 9c28899..51b09f1 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -33,6 +33,11 @@ const char *bnep_name(uint16_t id);
struct bnep_conn *bnep_new(uint16_t src, uint16_t dst,
const bdaddr_t *dst_addr);
void bnep_free(struct bnep_conn *bnep);
+typedef void (*bnep_connect_cb) (char *iface, int err, void *data);
+typedef void (*bnep_disconnect_cb) (void *data);
+int bnep_connect(struct bnep_conn *bnep, int sk, bnep_connect_cb conn_cb,
+ bnep_disconnect_cb disconn_cb, void *data);
+void bnep_disconnect(struct bnep_conn *bnep);
int bnep_connadd(int sk, uint16_t role, char *dev);
int bnep_conndel(const bdaddr_t *dst);
int bnep_if_up(const char *devname);
@@ -40,11 +45,6 @@ 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,
- void *data);
-
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/connection.c b/profiles/network/connection.c
index fb3e1ce..c7cc99f 100644
--- a/profiles/network/connection.c
+++ b/profiles/network/connection.c
@@ -72,6 +72,7 @@ struct network_conn {
guint dc_id;
struct network_peer *peer;
DBusMessage *connect;
+ struct bnep_conn *bnep;
};
static GSList *peers = NULL;
@@ -106,8 +107,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(void *data)
{
struct network_conn *nc = data;
DBusConnection *conn = btd_get_dbus_connection();
@@ -126,12 +126,18 @@ 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");
- return FALSE;
+ bnep_free(nc->bnep);
+ nc->bnep = NULL;
+
+ if (nc->io) {
+ g_io_channel_shutdown(nc->io, TRUE, NULL);
+ g_io_channel_unref(nc->io);
+ nc->io = NULL;
+ }
}
static void local_connect_cb(struct network_conn *nc, int err)
@@ -158,9 +164,17 @@ static void cancel_connection(struct network_conn *nc, int 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->io) {
+ g_io_channel_shutdown(nc->io, TRUE, NULL);
+ g_io_channel_unref(nc->io);
+ nc->io = NULL;
+ }
+
+ if (nc->state == CONNECTED)
+ bnep_disconnect(nc->bnep);
+
+ bnep_free(nc->bnep);
+ nc->bnep = NULL;
nc->state = DISCONNECTED;
}
@@ -169,11 +183,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 +196,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 +230,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,7 +247,14 @@ 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->bnep = bnep_new(BNEP_SVC_PANU, nc->id,
+ device_get_address(nc->peer->device));
+ if (!nc->bnep) {
+ error("unable to allocate memory for nc->bnep");
+ goto failed;
+ }
+
+ perr = bnep_connect(nc->bnep, sk, bnep_conn_cb, bnep_disconn_cb, nc);
if (perr < 0) {
error("bnep connect(): %s (%d)", strerror(-perr), -perr);
goto failed;
--
1.8.3.2
To simplify bnep server realted bridge creation and deletion calls
provided extra apis and moved related apis to static.
---
profiles/network/bnep.c | 50 +++++++++++++++++++++++++++++++++++++++++------
profiles/network/bnep.h | 10 ++++------
profiles/network/server.c | 37 +++--------------------------------
3 files changed, 51 insertions(+), 46 deletions(-)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 752e00e..5deac81 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -151,7 +151,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;
@@ -167,7 +167,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;
@@ -187,7 +187,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;
@@ -212,7 +212,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;
@@ -464,7 +464,7 @@ void bnep_free(struct bnep_conn *bc)
g_free(bc);
}
-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;
@@ -495,7 +495,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;
@@ -601,3 +601,41 @@ uint16_t bnep_setup_decode(struct bnep_setup_conn_req *req, uint16_t *dst,
return BNEP_SUCCESS;
}
+
+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);
+}
diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
index 51b09f1..734055d 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -38,12 +38,10 @@ typedef void (*bnep_disconnect_cb) (void *data);
int bnep_connect(struct bnep_conn *bnep, int sk, bnep_connect_cb conn_cb,
bnep_disconnect_cb disconn_cb, void *data);
void bnep_disconnect(struct bnep_conn *bnep);
-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);
+
+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);
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
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 | 37 +++++++++++++++++++++++++++++++++++++
profiles/network/bnep.h | 5 +++++
2 files changed, 42 insertions(+)
diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 08037e6..d7d8832 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -71,6 +71,7 @@ struct bnep_conn {
GIOChannel *io;
uint16_t src;
uint16_t dst;
+ bdaddr_t dst_addr;
guint attempts;
guint setup_to;
void *data;
@@ -246,6 +247,42 @@ int bnep_if_down(const char *devname)
return 0;
}
+struct bnep_conn *bnep_new(uint16_t src, uint16_t dst,
+ const bdaddr_t *dst_addr)
+{
+ struct bnep_conn *bc;
+
+ DBG("");
+
+ if (!dst_addr)
+ return NULL;
+
+ bc = g_new0(struct bnep_conn, 1);
+ if (!bc)
+ return NULL;
+
+ bc->src = src;
+ bc->dst = dst;
+ bacpy(&bc->dst_addr, dst_addr);
+
+ return bc;
+}
+
+void bnep_free(struct bnep_conn *bc)
+{
+ DBG("");
+
+ if (!bc)
+ return;
+
+ if (bc->io) {
+ g_io_channel_unref(bc->io);
+ bc->io = NULL;
+ }
+
+ g_free(bc);
+}
+
static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
gpointer data)
{
diff --git a/profiles/network/bnep.h b/profiles/network/bnep.h
index dd22c40..9c28899 100644
--- a/profiles/network/bnep.h
+++ b/profiles/network/bnep.h
@@ -21,6 +21,8 @@
*
*/
+struct bnep_conn;
+
int bnep_init(void);
int bnep_cleanup(void);
@@ -28,6 +30,9 @@ uint16_t bnep_service_id(const char *svc);
const char *bnep_uuid(uint16_t id);
const char *bnep_name(uint16_t id);
+struct bnep_conn *bnep_new(uint16_t src, uint16_t dst,
+ const bdaddr_t *dst_addr);
+void bnep_free(struct bnep_conn *bnep);
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
Renaming for easy readability.
---
android/pan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/android/pan.c b/android/pan.c
index ec589cf..03db350 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -162,7 +162,7 @@ static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
dev->io = NULL;
}
-static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
+static void bt_io_connect_cb(GIOChannel *chan, GError *err, gpointer data)
{
struct pan_device *dev = data;
uint16_t src, dst;
@@ -238,7 +238,7 @@ static void bt_pan_connect(const void *buf, uint16_t len)
ba2str(&dev->dst, addr);
DBG("connecting to %s %s", addr, dev->iface);
- dev->io = bt_io_connect(connect_cb, dev, NULL, &gerr,
+ dev->io = bt_io_connect(bt_io_connect_cb, dev, NULL, &gerr,
BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
BT_IO_OPT_DEST_BDADDR, &dev->dst,
BT_IO_OPT_PSM, BNEP_PSM,
--
1.8.3.2