Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH_v2 3/7] profiles/network: Rename common.c|h to bnep.c|h
From: Ravi kumar Veeramally @ 2013-11-28 19:52 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZJVZUjsTpgbAWx7J+gg4OYd4SkzvH+2nLAqaMnH-rm49Q@mail.gmail.com>

Hi Luiz,

On 28.11.2013 18:19, Luiz Augusto von Dentz wrote:
> Hi Ravi,
>
> On Thu, Nov 28, 2013 at 4:45 PM, Ravi kumar Veeramally
> <ravikumar.veeramally@linux.intel.com> wrote:
>> Files common.c|h contains only bnep related code, it makes
>> more sence with bnep.c|h.
>> ---
>>   Makefile.plugins              |   2 +-
>>   profiles/network/bnep.c       | 453 ++++++++++++++++++++++++++++++++++++++++++
>>   profiles/network/bnep.h       |  42 ++++
>>   profiles/network/common.c     | 453 ------------------------------------------
>>   profiles/network/common.h     |  42 ----
>>   profiles/network/connection.c |   2 +-
>>   profiles/network/manager.c    |   2 +-
>>   profiles/network/server.c     |   2 +-
>>   8 files changed, 499 insertions(+), 499 deletions(-)
>>   create mode 100644 profiles/network/bnep.c
>>   create mode 100644 profiles/network/bnep.h
>>   delete mode 100644 profiles/network/common.c
>>   delete mode 100644 profiles/network/common.h
> Did you use git mv? I would expect something like the following if did:
  Yes, I did git mv.
   reason might be bnep.c has it's own include file bnep.h.
   So bnpe.c is 99% bnep.h 100%.

> ---
>   profiles/network/{common.c => bnep.c} | 0
>   profiles/network/{common.h => bnep.h} | 0
>   2 files changed, 0 insertions(+), 0 deletions(-)
>   rename profiles/network/{common.c => bnep.c} (100%)
>   rename profiles/network/{common.h => bnep.h} (100%)
>
> diff --git a/profiles/network/common.c b/profiles/network/bnep.c
> similarity index 100%
> rename from profiles/network/common.c
> rename to profiles/network/bnep.c
> diff --git a/profiles/network/common.h b/profiles/network/bnep.h
> similarity index 100%
> rename from profiles/network/common.h
> rename to profiles/network/bnep.h
> --
  Have you tried to compile after it?

Thanks,
Ravi.

^ permalink raw reply

* Re: [PATCH_v2 1/7] profiles/network: Remove redundant code for bnep interface name
From: Ravi kumar Veeramally @ 2013-11-28 19:55 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: BlueZ development
In-Reply-To: <CAJdJm_Marb2+Q4e=zLeHq9Qf4a=o4z3-Q55mpL0bDWq3h1-s_Q@mail.gmail.com>

Hi,

On 28.11.2013 17:52, Anderson Lizardo wrote:
> Hi Ravi,
>
> On Thu, Nov 28, 2013 at 10:45 AM, Ravi kumar Veeramally
> <ravikumar.veeramally@linux.intel.com> wrote:
>> Remove redundant code for copying bnepX interface name from connection
>> and server files. It is better to place on actual function call.
>> ---
>>   profiles/network/common.c     | 3 +--
>>   profiles/network/connection.c | 2 --
>>   profiles/network/server.c     | 3 ---
>>   3 files changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/profiles/network/common.c b/profiles/network/common.c
>> index 0b291bd..b94b3a5 100644
>> --- a/profiles/network/common.c
>> +++ b/profiles/network/common.c
>> @@ -150,8 +150,7 @@ int bnep_connadd(int sk, uint16_t role, char *dev)
>>          struct bnep_connadd_req req;
>>
>>          memset(&req, 0, sizeof(req));
>> -       strncpy(req.device, dev, 16);
>> -       req.device[15] = '\0';
>> +       strcpy(req.device, "bnep%d");
>>          req.sock = sk;
>>          req.role = role;
>>          if (ioctl(ctl, BNEPCONNADD, &req) < 0) {
>> diff --git a/profiles/network/connection.c b/profiles/network/connection.c
>> index 5966268..301f66d 100644
>> --- a/profiles/network/connection.c
>> +++ b/profiles/network/connection.c
>> @@ -692,8 +692,6 @@ int connection_register(struct btd_service *service)
>>
>>          nc = g_new0(struct network_conn, 1);
>>          nc->id = id;
>> -       memset(nc->dev, 0, sizeof(nc->dev));
>> -       strcpy(nc->dev, "bnep%d");
>>          nc->service = btd_service_ref(service);
>>          nc->state = DISCONNECTED;
>>          nc->peer = peer;
>> diff --git a/profiles/network/server.c b/profiles/network/server.c
>> index 0050b30..3a7e52a 100644
>> --- a/profiles/network/server.c
>> +++ b/profiles/network/server.c
>> @@ -269,9 +269,6 @@ static int server_connadd(struct network_server *ns,
>>          char devname[16];
>>          int err, nsk;
>>
>> -       memset(devname, 0, sizeof(devname));
>> -       strcpy(devname, "bnep%d");
>> -
>>          nsk = g_io_channel_unix_get_fd(session->io);
>>          err = bnep_connadd(nsk, dst_role, devname);
> You are still passing a (now uninitialized) devname pointer to
> bnep_connadd(). Is this still necessary?
  You are right, it is missing in server.c but in connection.c struct is
created with g_new0.

Thanks,
Ravi.

^ permalink raw reply

* [RFC] Fix eir parsing function.
From: Andrei Emeltchenko @ 2013-11-29  8:00 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Currently eir_parse always return 0 but it is checked throughout the
code (in android/bluetooth code as well in src/adapteri, etc) for
return value (err < 0) which never happens. Return -1 if there is
nothing to parse. Send as RFC as I do not know how current code supposed
to be working.
---
 src/eir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/eir.c b/src/eir.c
index 084884e..f7450c9 100644
--- a/src/eir.c
+++ b/src/eir.c
@@ -138,7 +138,7 @@ int eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
 
 	/* No EIR data to parse */
 	if (eir_data == NULL)
-		return 0;
+		return -1;
 
 	while (len < eir_len - 1) {
 		uint8_t field_len = eir_data[0];
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 1/2] android/hal-bluetooth: Rename create_enum_prop to enum_prop_to_hal
From: Szymon Janc @ 2013-11-29  8:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This better describes purpose of this macro.
---
 android/hal-bluetooth.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index a879583..f232afd 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -28,7 +28,7 @@
 
 static const bt_callbacks_t *bt_hal_cbacks = NULL;
 
-#define create_enum_prop(prop, hal_prop, type) do { \
+#define enum_prop_to_hal(prop, hal_prop, type) do { \
 	static type e; \
 	prop.val = &e; \
 	prop.len = sizeof(e); \
@@ -63,11 +63,11 @@ static void adapter_props_to_hal(bt_property_t *send_props,
 
 		switch (prop->type) {
 		case HAL_PROP_ADAPTER_TYPE:
-			create_enum_prop(send_props[i], prop,
+			enum_prop_to_hal(send_props[i], prop,
 							bt_device_type_t);
 			break;
 		case HAL_PROP_ADAPTER_SCAN_MODE:
-			create_enum_prop(send_props[i], prop,
+			enum_prop_to_hal(send_props[i], prop,
 							bt_scan_mode_t);
 			break;
 		case HAL_PROP_ADAPTER_SERVICE_REC:
@@ -109,7 +109,7 @@ static void device_props_to_hal(bt_property_t *send_props,
 
 		switch (prop->type) {
 		case HAL_PROP_DEVICE_TYPE:
-			create_enum_prop(send_props[i], prop,
+			enum_prop_to_hal(send_props[i], prop,
 							bt_device_type_t);
 			break;
 		case HAL_PROP_DEVICE_SERVICE_REC:
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH 2/2] android/hal-bluetooth: Fix sending invalid adapter property
From: Szymon Janc @ 2013-11-29  8:05 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1385712358-8121-1-git-send-email-szymon.janc@tieto.com>

If property to be set is of enum type it should be first converted to
byte value as size of enum might varry depending on architecture.

To keep code simple command buffer uses len received from framework
as this is more or equal to HAL property size.
---
 android/hal-bluetooth.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index f232afd..87d6fc7 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -35,6 +35,18 @@ static const bt_callbacks_t *bt_hal_cbacks = NULL;
 	e = *((uint8_t *) (hal_prop->val)); \
 } while (0)
 
+#define enum_prop_from_hal(prop, hal_len, hal_val, enum_type) do { \
+	enum_type e; \
+	if (prop->len != sizeof(e)) { \
+		error("invalid HAL property %u (%u vs %zu), aborting ", \
+					prop->type, prop->len, sizeof(e)); \
+		exit(EXIT_FAILURE); \
+	} \
+	memcpy(&e, prop->val, sizeof(e)); \
+	*((uint8_t *) hal_val) = e; /* enums are mapped to 1 byte */ \
+	*hal_len = 1; \
+} while (0)
+
 static void handle_adapter_state_changed(void *buf, uint16_t len)
 {
 	struct hal_ev_adapter_state_changed *ev = buf;
@@ -91,6 +103,23 @@ static void adapter_props_to_hal(bt_property_t *send_props,
 	exit(EXIT_FAILURE);
 }
 
+static void adapter_prop_from_hal(const bt_property_t *property, uint8_t *type,
+						uint16_t *len, void *val)
+{
+	/* type match IPC type */
+	*type = property->type;
+
+	switch(property->type) {
+	case HAL_PROP_ADAPTER_SCAN_MODE:
+		enum_prop_from_hal(property, len, val, bt_scan_mode_t);
+		break;
+	default:
+		*len = property->len;
+		memcpy(val, property->val, property->len);
+		break;
+	}
+}
+
 static void device_props_to_hal(bt_property_t *send_props,
 				struct hal_property *prop, uint8_t num_props,
 				uint16_t len)
@@ -458,13 +487,10 @@ static int set_adapter_property(const bt_property_t *property)
 	if (!interface_ready())
 		return BT_STATUS_NOT_READY;
 
-	/* type match IPC type */
-	cmd->type = property->type;
-	cmd->len = property->len;
-	memcpy(cmd->val, property->val, property->len);
+	adapter_prop_from_hal(property, &cmd->type, &cmd->len, cmd->val);
 
 	return hal_ipc_cmd(HAL_SERVICE_ID_BLUETOOTH, HAL_OP_SET_ADAPTER_PROP,
-					sizeof(buf), cmd, 0, NULL, NULL);
+				sizeof(*cmd) + cmd->len, cmd, 0, NULL, NULL);
 }
 
 static int get_remote_device_properties(bt_bdaddr_t *remote_addr)
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH] android/main: Call unregister for all registered services on exit
From: Andrei Emeltchenko @ 2013-11-29  8:08 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 android/main.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/android/main.c b/android/main.c
index 2879bbf..2a9348b 100644
--- a/android/main.c
+++ b/android/main.c
@@ -493,6 +493,38 @@ static void cleanup_hal_connection(void)
 	ipc_cleanup();
 }
 
+static void cleanup_services(void)
+{
+	int i;
+
+	DBG("");
+
+	for (i=HAL_SERVICE_ID_BLUETOOTH; i < HAL_SERVICE_ID_MAX; i++) {
+		if (!services[i])
+			continue;
+
+		switch (i) {
+		case HAL_SERVICE_ID_BLUETOOTH:
+			bt_bluetooth_unregister();
+			break;
+		case HAL_SERVICE_ID_SOCK:
+			bt_socket_unregister();
+			break;
+		case HAL_SERVICE_ID_HIDHOST:
+			bt_hid_unregister();
+			break;
+		case HAL_SERVICE_ID_A2DP:
+			bt_a2dp_unregister();
+			break;
+		case HAL_SERVICE_ID_PAN:
+			bt_pan_unregister();
+			break;
+		}
+
+		services[i] = false;
+	}
+}
+
 static bool set_capabilities(void)
 {
 #if defined(ANDROID)
@@ -598,6 +630,8 @@ int main(int argc, char *argv[])
 	if (bluetooth_start_timeout > 0)
 		g_source_remove(bluetooth_start_timeout);
 
+	cleanup_services();
+
 	cleanup_hal_connection();
 	stop_sdp_server();
 	bt_bluetooth_cleanup();
-- 
1.8.3.2


^ permalink raw reply related

* Re: [RFC] Fix eir parsing function.
From: Johan Hedberg @ 2013-11-29  8:22 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1385712049-24635-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Fri, Nov 29, 2013, Andrei Emeltchenko wrote:
> Currently eir_parse always return 0 but it is checked throughout the
> code (in android/bluetooth code as well in src/adapteri, etc) for
> return value (err < 0) which never happens. Return -1 if there is
> nothing to parse. Send as RFC as I do not know how current code supposed
> to be working.
> ---
>  src/eir.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/eir.c b/src/eir.c
> index 084884e..f7450c9 100644
> --- a/src/eir.c
> +++ b/src/eir.c
> @@ -138,7 +138,7 @@ int eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
>  
>  	/* No EIR data to parse */
>  	if (eir_data == NULL)
> -		return 0;
> +		return -1;
>  
>  	while (len < eir_len - 1) {
>  		uint8_t field_len = eir_data[0];

I think the only concern here is "when is it safe to call
eir_data_free()?". If it would not be safe to call that in case the
eir_parse function "fails" then we'd need to be able to clearly
distinguish failure though a -1 return value. However, as it now stands
it is always safe to call eir_data_free() on a struct that was passed to
eir_parse(), so I'd go for simply changing this function to return void
instead of int.

Johan

^ permalink raw reply

* Re: [PATCH 05/10] android/socket: Cleanup sockets on unregister
From: Johan Hedberg @ 2013-11-29  8:26 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1385649486-19978-5-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Thu, Nov 28, 2013, Andrei Emeltchenko wrote:
> This cleans up rfsock structures closing all sockets and making general cleanup
> for servers and for connections. This will be called form socket unregister.
> ---
>  android/socket.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/android/socket.c b/android/socket.c
> index 1fb154d..d55db54 100644
> --- a/android/socket.c
> +++ b/android/socket.c
> @@ -943,7 +943,27 @@ bool bt_socket_register(int sk, const bdaddr_t *addr)
>  	return true;
>  }
>  
> +static void free_connection(gpointer data, gpointer user_data)
> +{
> +	struct rfcomm_sock *rfsock = data;
> +
> +	connections = g_list_remove(connections, rfsock);
> +	cleanup_rfsock(rfsock);
> +}
> +
> +static void free_server(gpointer data, gpointer user_data)
> +{
> +	struct rfcomm_sock *rfsock = data;
> +
> +	servers = g_list_remove(servers, rfsock);
> +	cleanup_rfsock(rfsock);
> +}
> +
>  void bt_socket_unregister(void)
>  {
>  	DBG("");
> +
> +	g_list_foreach(connections, free_connection, NULL);
> +
> +	g_list_foreach(servers, free_server, NULL);
>  }

I think what you're looking for here is g_slist_free_full instead of
g_slist_foreach. That way you won't need to have any helper functions at
all but you can directly pass cleanup_rfsock to it.

Johan

^ permalink raw reply

* Re: [PATCH 04/10] android/main: Free enabled string on exit
From: Johan Hedberg @ 2013-11-29  8:29 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1385649486-19978-4-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Thu, Nov 28, 2013, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> ---
>  android/main.c | 3 +++
>  1 file changed, 3 insertions(+)

Patches 1-4 have been applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH 10/10] android/socket: Remove unneeded code
From: Johan Hedberg @ 2013-11-29  8:31 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1385649486-19978-10-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Thu, Nov 28, 2013, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> The flag is already set in bt_io_listen.
> ---
>  android/socket.c | 2 --
>  1 file changed, 2 deletions(-)

Patches 6-10 have been applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH 1/2] android/hal-bluetooth: Rename create_enum_prop to enum_prop_to_hal
From: Johan Hedberg @ 2013-11-29  8:32 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1385712358-8121-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Fri, Nov 29, 2013, Szymon Janc wrote:
> This better describes purpose of this macro.
> ---
>  android/hal-bluetooth.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Both patches have been applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] android/main: Call unregister for all registered services on exit
From: Johan Hedberg @ 2013-11-29  8:34 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1385712529-24757-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Fri, Nov 29, 2013, Andrei Emeltchenko wrote:
> ---
>  android/main.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/android/main.c b/android/main.c
> index 2879bbf..2a9348b 100644
> --- a/android/main.c
> +++ b/android/main.c
> @@ -493,6 +493,38 @@ static void cleanup_hal_connection(void)
>  	ipc_cleanup();
>  }
>  
> +static void cleanup_services(void)
> +{
> +	int i;
> +
> +	DBG("");
> +
> +	for (i=HAL_SERVICE_ID_BLUETOOTH; i < HAL_SERVICE_ID_MAX; i++) {

You've got coding style issues here: there should be a space before and
after '='. Since this trivial thing was the only issue I saw with the
patch I fixed it up myself and applied it.

Johan

^ permalink raw reply

* Re: [RFC] Fix eir parsing function.
From: Szymon Janc @ 2013-11-29  8:39 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: Andrei Emeltchenko, linux-bluetooth
In-Reply-To: <20131129082247.GA6800@x220.p-661hnu-f1>

Hi,

> Hi Andrei,
> 
> On Fri, Nov 29, 2013, Andrei Emeltchenko wrote:
> > Currently eir_parse always return 0 but it is checked throughout the
> > code (in android/bluetooth code as well in src/adapteri, etc) for
> > return value (err < 0) which never happens. Return -1 if there is
> > nothing to parse. Send as RFC as I do not know how current code supposed
> > to be working.
> > ---
> >  src/eir.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/eir.c b/src/eir.c
> > index 084884e..f7450c9 100644
> > --- a/src/eir.c
> > +++ b/src/eir.c
> > @@ -138,7 +138,7 @@ int eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
> >  
> >  	/* No EIR data to parse */
> >  	if (eir_data == NULL)
> > -		return 0;
> > +		return -1;
> >  
> >  	while (len < eir_len - 1) {
> >  		uint8_t field_len = eir_data[0];
> 
> I think the only concern here is "when is it safe to call
> eir_data_free()?". If it would not be safe to call that in case the
> eir_parse function "fails" then we'd need to be able to clearly
> distinguish failure though a -1 return value. However, as it now stands
> it is always safe to call eir_data_free() on a struct that was passed to
> eir_parse(), so I'd go for simply changing this function to return void
> instead of int.

I think it should be possible to check if parsing failed due to invalid EIR
but I think parsing empty EIR should not result in error.

Most callers do verify eir_len or buffer validity before calling eir_parse()
anyway so I would change this  check to:
if (!eir_data || !eir_len) return 0; 

and simplify callers code.

-- 
BR
Szymon Janc

^ permalink raw reply

* Re: [RFC] Fix eir parsing function.
From: Johan Hedberg @ 2013-11-29  8:54 UTC (permalink / raw)
  To: Szymon Janc; +Cc: Andrei Emeltchenko, linux-bluetooth
In-Reply-To: <1501205.ZvMV0NcIeM@uw000953>

Hi Szymon,

On Fri, Nov 29, 2013, Szymon Janc wrote:
> > Hi Andrei,
> > 
> > On Fri, Nov 29, 2013, Andrei Emeltchenko wrote:
> > > Currently eir_parse always return 0 but it is checked throughout the
> > > code (in android/bluetooth code as well in src/adapteri, etc) for
> > > return value (err < 0) which never happens. Return -1 if there is
> > > nothing to parse. Send as RFC as I do not know how current code supposed
> > > to be working.
> > > ---
> > >  src/eir.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/src/eir.c b/src/eir.c
> > > index 084884e..f7450c9 100644
> > > --- a/src/eir.c
> > > +++ b/src/eir.c
> > > @@ -138,7 +138,7 @@ int eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
> > >  
> > >  	/* No EIR data to parse */
> > >  	if (eir_data == NULL)
> > > -		return 0;
> > > +		return -1;
> > >  
> > >  	while (len < eir_len - 1) {
> > >  		uint8_t field_len = eir_data[0];
> > 
> > I think the only concern here is "when is it safe to call
> > eir_data_free()?". If it would not be safe to call that in case the
> > eir_parse function "fails" then we'd need to be able to clearly
> > distinguish failure though a -1 return value. However, as it now stands
> > it is always safe to call eir_data_free() on a struct that was passed to
> > eir_parse(), so I'd go for simply changing this function to return void
> > instead of int.
> 
> I think it should be possible to check if parsing failed due to invalid EIR
> but I think parsing empty EIR should not result in error.
> 
> Most callers do verify eir_len or buffer validity before calling eir_parse()
> anyway so I would change this  check to:
> if (!eir_data || !eir_len) return 0; 
> 
> and simplify callers code.

I'm fine with adding a check for !eir_len, but if you check the actual
implementation of eir_parse you'll see that it has no places where it'd
return an error in the case of invalid EIR. In such a case the function
just stops parsing at that point and returns. I think from a
interoperability/usability perspective this makes sense: we take the
best effort to parse what we can and use the data that we were able to
parse until the parsing error happened. If you'd return an error when
parsing fails it'd make the calling code disregard any of the valid data
that was parsed until the point where the invalid data began.

Johan

^ permalink raw reply

* Re: [RFC] Fix eir parsing function.
From: Szymon Janc @ 2013-11-29  9:03 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: Andrei Emeltchenko, linux-bluetooth
In-Reply-To: <20131129085407.GA10642@x220.p-661hnu-f1>

Hi Johan,

> Hi Szymon,
> 
> On Fri, Nov 29, 2013, Szymon Janc wrote:
> > > Hi Andrei,
> > > 
> > > On Fri, Nov 29, 2013, Andrei Emeltchenko wrote:
> > > > Currently eir_parse always return 0 but it is checked throughout the
> > > > code (in android/bluetooth code as well in src/adapteri, etc) for
> > > > return value (err < 0) which never happens. Return -1 if there is
> > > > nothing to parse. Send as RFC as I do not know how current code supposed
> > > > to be working.
> > > > ---
> > > >  src/eir.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/src/eir.c b/src/eir.c
> > > > index 084884e..f7450c9 100644
> > > > --- a/src/eir.c
> > > > +++ b/src/eir.c
> > > > @@ -138,7 +138,7 @@ int eir_parse(struct eir_data *eir, const uint8_t *eir_data, uint8_t eir_len)
> > > >  
> > > >  	/* No EIR data to parse */
> > > >  	if (eir_data == NULL)
> > > > -		return 0;
> > > > +		return -1;
> > > >  
> > > >  	while (len < eir_len - 1) {
> > > >  		uint8_t field_len = eir_data[0];
> > > 
> > > I think the only concern here is "when is it safe to call
> > > eir_data_free()?". If it would not be safe to call that in case the
> > > eir_parse function "fails" then we'd need to be able to clearly
> > > distinguish failure though a -1 return value. However, as it now stands
> > > it is always safe to call eir_data_free() on a struct that was passed to
> > > eir_parse(), so I'd go for simply changing this function to return void
> > > instead of int.
> > 
> > I think it should be possible to check if parsing failed due to invalid EIR
> > but I think parsing empty EIR should not result in error.
> > 
> > Most callers do verify eir_len or buffer validity before calling eir_parse()
> > anyway so I would change this  check to:
> > if (!eir_data || !eir_len) return 0; 
> > 
> > and simplify callers code.
> 
> I'm fine with adding a check for !eir_len, but if you check the actual
> implementation of eir_parse you'll see that it has no places where it'd
> return an error in the case of invalid EIR. In such a case the function
> just stops parsing at that point and returns. I think from a
> interoperability/usability perspective this makes sense: we take the
> best effort to parse what we can and use the data that we were able to
> parse until the parsing error happened. If you'd return an error when
> parsing fails it'd make the calling code disregard any of the valid data
> that was parsed until the point where the invalid data began.

Right, then making this return void makes sense as currently it is confusing
that it might return an error (eg. as used in eir_parse_oob()).

-- 
BR
Szymon Janc



^ permalink raw reply

* [PATCHv2] android/socket: Cleanup sockets on unregister
From: Andrei Emeltchenko @ 2013-11-29  9:33 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

This cleans up rfsock structures closing all sockets and making general cleanup
for servers and for connections. This will be called form socket unregister.
---
 android/socket.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/android/socket.c b/android/socket.c
index 4f47861..7d41756 100644
--- a/android/socket.c
+++ b/android/socket.c
@@ -93,8 +93,10 @@ static struct rfcomm_sock *create_rfsock(int sock, int *hal_fd)
 	return rfsock;
 }
 
-static void cleanup_rfsock(struct rfcomm_sock *rfsock)
+static void cleanup_rfsock(gpointer data)
 {
+	struct rfcomm_sock *rfsock = data;
+
 	DBG("rfsock: %p fd %d real_sock %d chan %u",
 		rfsock, rfsock->fd, rfsock->real_sock, rfsock->channel);
 
@@ -941,4 +943,7 @@ void bt_socket_register(const bdaddr_t *addr)
 void bt_socket_unregister(void)
 {
 	DBG("");
+
+	g_list_free_full(connections, cleanup_rfsock);
+	g_list_free_full(servers, cleanup_rfsock);
 }
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH_v3 0/7] Refactor bnep code and implement pan methods
From: Ravi kumar Veeramally @ 2013-11-29  9:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ravi kumar Veeramally

v3: Fixed Anderson and Luiz comments and solved rebased conflicts.

v2: Refactored profiles/network/common.* as per Johan's comments
    (renaming common.c|h to bnep.c|h and moving bnep related code to
     bnep.c ro reduce redundancy in profiles/netowrk/connection.c and
     android/pan.c)

v1: This patch set supports PANU role with a minor fix in android. Added
    CAP_NET_RAW capability for bnep services. Creates bnep connection and
    up the inreface on connect call and free the device on disconnect call.
    Interface name(bnepX) will be notified on control state cb. Android
    environment will create IP address with dhcp calls.

Ravi kumar Veeramally (7):
  profiles/network: Remove redundant code for bnep interface name
  profiles/network: Refactor bnep connection setup functionality
  profiles/network: Rename common.c|h to bnep.c|h
  android/pan: Implement pan connect method in daemon
  android/pan: Implement pan disconnect method in daemon
  android/pan: Implement the get local role method in daemon
  android: Add reasons for adding capabilites to process

 Makefile.plugins                      |   2 +-
 android/Android.mk                    |   2 +
 android/Makefile.am                   |   3 +-
 android/main.c                        |   3 +
 android/pan.c                         | 252 ++++++++++++++++++++++++++++++++--
 profiles/network/{common.c => bnep.c} | 184 ++++++++++++++++++++++++-
 profiles/network/{common.h => bnep.h} |   5 +
 profiles/network/connection.c         | 171 ++---------------------
 profiles/network/manager.c            |   2 +-
 profiles/network/server.c             |   5 +-
 10 files changed, 452 insertions(+), 177 deletions(-)
 rename profiles/network/{common.c => bnep.c} (59%)
 rename profiles/network/{common.h => bnep.h} (87%)

-- 
1.8.3.2


^ permalink raw reply

* [PATCH_v3 1/7] profiles/network: Remove redundant code for bnep interface name
From: Ravi kumar Veeramally @ 2013-11-29  9:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1385718846-19070-1-git-send-email-ravikumar.veeramally@linux.intel.com>

Remove redundant code for copying bnepX interface name from connection
and server files. It is better to place on actual function call.
---
 profiles/network/common.c     | 4 ++--
 profiles/network/connection.c | 2 --
 profiles/network/server.c     | 3 ---
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/profiles/network/common.c b/profiles/network/common.c
index 0b291bd..c2da925 100644
--- a/profiles/network/common.c
+++ b/profiles/network/common.c
@@ -149,9 +149,9 @@ int bnep_connadd(int sk, uint16_t role, char *dev)
 {
 	struct bnep_connadd_req req;
 
+	memset(dev, 0, 16);
 	memset(&req, 0, sizeof(req));
-	strncpy(req.device, dev, 16);
-	req.device[15] = '\0';
+	strcpy(req.device, "bnep%d");
 	req.sock = sk;
 	req.role = role;
 	if (ioctl(ctl, BNEPCONNADD, &req) < 0) {
diff --git a/profiles/network/connection.c b/profiles/network/connection.c
index 5966268..301f66d 100644
--- a/profiles/network/connection.c
+++ b/profiles/network/connection.c
@@ -692,8 +692,6 @@ int connection_register(struct btd_service *service)
 
 	nc = g_new0(struct network_conn, 1);
 	nc->id = id;
-	memset(nc->dev, 0, sizeof(nc->dev));
-	strcpy(nc->dev, "bnep%d");
 	nc->service = btd_service_ref(service);
 	nc->state = DISCONNECTED;
 	nc->peer = peer;
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 0050b30..3a7e52a 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -269,9 +269,6 @@ static int server_connadd(struct network_server *ns,
 	char devname[16];
 	int err, nsk;
 
-	memset(devname, 0, sizeof(devname));
-	strcpy(devname, "bnep%d");
-
 	nsk = g_io_channel_unix_get_fd(session->io);
 	err = bnep_connadd(nsk, dst_role, devname);
 	if (err < 0)
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH_v3 2/7] profiles/network: Refactor bnep connection setup functionality
From: Ravi kumar Veeramally @ 2013-11-29  9:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1385718846-19070-1-git-send-email-ravikumar.veeramally@linux.intel.com>

Moving bnep connection setup related functionality to common.c.
Provided bnep_connect call with bnep_connect_cb for status and
bnep interface name. It will be simple if someone want to utilize
this call otherwise they have to reimplement similar functionality
with minimal changes (e.g. android/pan).
---
 profiles/network/common.c     | 178 ++++++++++++++++++++++++++++++++++++++++++
 profiles/network/common.h     |   5 ++
 profiles/network/connection.c | 167 +++------------------------------------
 3 files changed, 194 insertions(+), 156 deletions(-)

diff --git a/profiles/network/common.c b/profiles/network/common.c
index c2da925..fd25d96 100644
--- a/profiles/network/common.c
+++ b/profiles/network/common.c
@@ -46,6 +46,9 @@
 #include "common.h"
 #include "lib/uuid.h"
 
+#define CON_SETUP_RETRIES      3
+#define CON_SETUP_TO           9
+
 static int ctl;
 
 static struct {
@@ -59,6 +62,21 @@ static struct {
 	{ NULL }
 };
 
+struct __service_16 {
+	uint16_t dst;
+	uint16_t src;
+} __attribute__ ((packed));
+
+struct bnep_conn {
+	GIOChannel	*io;
+	uint16_t	src;
+	uint16_t	dst;
+	guint	attempts;
+	guint	setup_to;
+	void	*data;
+	bnep_connect_cb	conn_cb;
+};
+
 uint16_t bnep_service_id(const char *svc)
 {
 	int i;
@@ -215,6 +233,166 @@ int bnep_if_down(const char *devname)
 	return 0;
 }
 
+static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
+								gpointer data)
+{
+	struct bnep_conn *bc = data;
+	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;
+
+	if (bc->setup_to > 0) {
+		g_source_remove(bc->setup_to);
+		bc->setup_to = 0;
+	}
+
+	if (cond & (G_IO_HUP | G_IO_ERR)) {
+		error("Hangup or error on l2cap server socket");
+		goto failed;
+	}
+
+	sk = g_io_channel_unix_get_fd(chan);
+	memset(pkt, 0, BNEP_MTU);
+	r = read(sk, pkt, sizeof(pkt) - 1);
+	if (r < 0) {
+		error("IO Channel read error");
+		goto failed;
+	}
+
+	if (r == 0) {
+		error("No packet received on l2cap socket");
+		goto failed;
+	}
+
+	errno = EPROTO;
+
+	if ((size_t) r < sizeof(*rsp)) {
+		error("Packet received is not bnep type");
+		goto failed;
+	}
+
+	rsp = (void *) pkt;
+	if (rsp->type != BNEP_CONTROL) {
+		error("Packet received is not bnep type");
+		goto failed;
+	}
+
+	if (rsp->ctrl != BNEP_SETUP_CONN_RSP)
+		return TRUE;
+
+	r = ntohs(rsp->resp);
+	if (r != BNEP_SUCCESS) {
+		error("bnep failed");
+		goto failed;
+	}
+
+	memset(&timeo, 0, sizeof(timeo));
+	timeo.tv_sec = 0;
+	setsockopt(sk, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo));
+
+	sk = g_io_channel_unix_get_fd(bc->io);
+	if (bnep_connadd(sk, bc->src, iface)) {
+		error("bnep conn could not be added");
+		goto failed;
+	}
+
+	if (bnep_if_up(iface)) {
+		error("could not up %s", iface);
+		goto failed;
+	}
+
+	bc->conn_cb(chan, iface, 0, bc->data);
+	g_free(bc);
+
+	return FALSE;
+
+failed:
+	bc->conn_cb(NULL, NULL, -EIO, bc->data);
+	g_free(bc);
+
+	return FALSE;
+}
+
+
+static int bnep_setup_conn_req(struct bnep_conn *bc)
+{
+	struct bnep_setup_conn_req *req;
+	struct __service_16 *s;
+	unsigned char pkt[BNEP_MTU];
+	int fd;
+
+	/* Send request */
+	req = (void *) pkt;
+	req->type = BNEP_CONTROL;
+	req->ctrl = BNEP_SETUP_CONN_REQ;
+	req->uuid_size = 2;     /* 16bit UUID */
+	s = (void *) req->service;
+	s->src = htons(bc->src);
+	s->dst = htons(bc->dst);
+
+	fd = g_io_channel_unix_get_fd(bc->io);
+	if (write(fd, pkt, sizeof(*req) + sizeof(*s)) < 0) {
+		error("bnep connection req send failed: %s", strerror(errno));
+		return -errno;
+	}
+
+	bc->attempts++;
+
+	return 0;
+}
+
+static gboolean bnep_conn_req_to(gpointer user_data)
+{
+	struct bnep_conn *bc = user_data;
+
+	if (bc->attempts == CON_SETUP_RETRIES) {
+		error("Too many bnep connection attempts");
+	} else {
+		error("bnep connection setup TO, retrying...");
+		if (bnep_setup_conn_req(bc) == 0)
+			return TRUE;
+	}
+
+	bc->conn_cb(NULL, NULL, -ETIMEDOUT, bc->data);
+	g_free(bc);
+
+	return FALSE;
+}
+
+int bnep_connect(GIOChannel *io, uint16_t src, uint16_t dst,
+					bnep_connect_cb conn_cb, void *data)
+{
+	struct bnep_conn *bc;
+	int err;
+
+	if (!io || !conn_cb)
+		return -EINVAL;
+
+	bc = g_new0(struct bnep_conn, 1);
+	bc->io = io;
+	bc->attempts = 0;
+	bc->src = src;
+	bc->dst = dst;
+	bc->conn_cb = conn_cb;
+	bc->data = data;
+
+	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_setup_cb, bc);
+	return 0;
+}
+
 int bnep_add_to_bridge(const char *devname, const char *bridge)
 {
 	int ifindex;
diff --git a/profiles/network/common.h b/profiles/network/common.h
index 9a8caac..11db828 100644
--- a/profiles/network/common.h
+++ b/profiles/network/common.h
@@ -35,3 +35,8 @@ int bnep_if_up(const char *devname);
 int bnep_if_down(const char *devname);
 int bnep_add_to_bridge(const char *devname, const char *bridge);
 int bnep_del_from_bridge(const char *devname, const char *bridge);
+
+typedef void (*bnep_connect_cb) (GIOChannel *chan, char *iface, int err,
+								void *data);
+int bnep_connect(GIOChannel *io, uint16_t src, uint16_t dst,
+					bnep_connect_cb conn_cb, void *data);
diff --git a/profiles/network/connection.c b/profiles/network/connection.c
index 301f66d..5723afd 100644
--- a/profiles/network/connection.c
+++ b/profiles/network/connection.c
@@ -51,8 +51,6 @@
 #include "connection.h"
 
 #define NETWORK_PEER_INTERFACE "org.bluez.Network1"
-#define CON_SETUP_RETRIES      3
-#define CON_SETUP_TO           9
 
 typedef enum {
 	CONNECTED,
@@ -73,16 +71,9 @@ struct network_conn {
 	GIOChannel	*io;
 	guint		dc_id;
 	struct network_peer *peer;
-	guint		attempt_cnt;
-	guint		timeout_source;
 	DBusMessage	*connect;
 };
 
-struct __service_16 {
-	uint16_t dst;
-	uint16_t src;
-} __attribute__ ((packed));
-
 static GSList *peers = NULL;
 
 static uint16_t get_service_id(struct btd_service *service)
@@ -163,11 +154,6 @@ static void local_connect_cb(struct network_conn *nc, int err)
 
 static void cancel_connection(struct network_conn *nc, int err)
 {
-	if (nc->timeout_source > 0) {
-		g_source_remove(nc->timeout_source);
-		nc->timeout_source = 0;
-	}
-
 	btd_service_connecting_complete(nc->service, err);
 	if (nc->connect)
 		local_connect_cb(nc, err);
@@ -200,83 +186,24 @@ static void disconnect_cb(struct btd_device *device, gboolean removal,
 	connection_destroy(NULL, user_data);
 }
 
-static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
-							gpointer data)
+static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
 {
 	struct network_conn *nc = data;
-	struct bnep_control_rsp *rsp;
-	struct timeval timeo;
-	char pkt[BNEP_MTU];
-	ssize_t r;
-	int sk;
 	const char *path;
 	DBusConnection *conn;
 
-	DBG("cond %u", cond);
-
-	if (cond & G_IO_NVAL)
-		return FALSE;
-
-	if (nc->timeout_source > 0) {
-		g_source_remove(nc->timeout_source);
-		nc->timeout_source = 0;
-	}
-
-	if (cond & (G_IO_HUP | G_IO_ERR)) {
-		error("Hangup or error on l2cap server socket");
-		goto failed;
-	}
-
-	sk = g_io_channel_unix_get_fd(chan);
-
-	memset(pkt, 0, BNEP_MTU);
-	r = read(sk, pkt, sizeof(pkt) -1);
-	if (r < 0) {
-		error("IO Channel read error");
-		goto failed;
-	}
-
-	if (r == 0) {
-		error("No packet received on l2cap socket");
-		goto failed;
-	}
-
-	errno = EPROTO;
-
-	if ((size_t) r < sizeof(*rsp)) {
-		error("Packet received is not bnep type");
-		goto failed;
-	}
-
-	rsp = (void *) pkt;
-	if (rsp->type != BNEP_CONTROL) {
-		error("Packet received is not bnep type");
-		goto failed;
-	}
-
-	if (rsp->ctrl != BNEP_SETUP_CONN_RSP)
-		return TRUE;
-
-	r = ntohs(rsp->resp);
-
-	if (r != BNEP_SUCCESS) {
-		error("bnep failed");
-		goto failed;
-	}
-
-	memset(&timeo, 0, sizeof(timeo));
-	timeo.tv_sec = 0;
-
-	setsockopt(sk, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo));
+	DBG("");
 
-	if (bnep_connadd(sk, BNEP_SVC_PANU, nc->dev)) {
-		error("%s could not be added", nc->dev);
+	if (err < 0) {
+		error("connect failed %s", strerror(-err));
 		goto failed;
 	}
 
-	bnep_if_up(nc->dev);
+	info("%s connected", nc->dev);
 
+	memcpy(nc->dev, iface, sizeof(nc->dev));
 	btd_service_connecting_complete(nc->service, 0);
+
 	if (nc->connect)
 		local_connect_cb(nc, 0);
 
@@ -292,88 +219,16 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
 
 	nc->state = CONNECTED;
 	nc->dc_id = device_add_disconnect_watch(nc->peer->device, disconnect_cb,
-						nc, NULL);
-
-	info("%s connected", nc->dev);
-	/* Start watchdog */
+								nc, NULL);
 	g_io_add_watch(chan, G_IO_ERR | G_IO_HUP | G_IO_NVAL,
-			(GIOFunc) bnep_watchdog_cb, nc);
+							bnep_watchdog_cb, nc);
 	g_io_channel_unref(nc->io);
 	nc->io = NULL;
 
-	return FALSE;
+	return;
 
 failed:
 	cancel_connection(nc, -EIO);
-
-	return FALSE;
-}
-
-static int bnep_send_conn_req(struct network_conn *nc)
-{
-	struct bnep_setup_conn_req *req;
-	struct __service_16 *s;
-	unsigned char pkt[BNEP_MTU];
-	int fd;
-
-	DBG("");
-
-	/* Send request */
-	req = (void *) pkt;
-	req->type = BNEP_CONTROL;
-	req->ctrl = BNEP_SETUP_CONN_REQ;
-	req->uuid_size = 2;	/* 16bit UUID */
-	s = (void *) req->service;
-	s->dst = htons(nc->id);
-	s->src = htons(BNEP_SVC_PANU);
-
-	fd = g_io_channel_unix_get_fd(nc->io);
-	if (write(fd, pkt, sizeof(*req) + sizeof(*s)) < 0) {
-		int err = -errno;
-		error("bnep connection req send failed: %s", strerror(errno));
-		return err;
-	}
-
-	nc->attempt_cnt++;
-
-	return 0;
-}
-
-static gboolean bnep_conn_req_to(gpointer user_data)
-{
-	struct network_conn *nc;
-
-	nc = user_data;
-	if (nc->attempt_cnt == CON_SETUP_RETRIES) {
-		error("Too many bnep connection attempts");
-	} else {
-		error("bnep connection setup TO, retrying...");
-		if (bnep_send_conn_req(nc) == 0)
-			return TRUE;
-	}
-
-	cancel_connection(nc, -ETIMEDOUT);
-
-	return FALSE;
-}
-
-static int bnep_connect(struct network_conn *nc)
-{
-	int err;
-
-	nc->attempt_cnt = 0;
-
-	err = bnep_send_conn_req(nc);
-	if (err < 0)
-		return err;
-
-	nc->timeout_source = g_timeout_add_seconds(CON_SETUP_TO,
-							bnep_conn_req_to, nc);
-
-	g_io_add_watch(nc->io, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
-							bnep_setup_cb, nc);
-
-	return 0;
 }
 
 static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
@@ -386,7 +241,7 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
 		goto failed;
 	}
 
-	perr = bnep_connect(nc);
+	perr = bnep_connect(nc->io, BNEP_SVC_PANU, nc->id, bnep_conn_cb, nc);
 	if (perr < 0) {
 		error("bnep connect(): %s (%d)", strerror(-perr), -perr);
 		goto failed;
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH_v3 3/7] profiles/network: Rename common.c|h to bnep.c|h
From: Ravi kumar Veeramally @ 2013-11-29  9:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1385718846-19070-1-git-send-email-ravikumar.veeramally@linux.intel.com>

Files common.c|h contains only bnep related code, it makes
more sence with bnep.c|h.
---
 Makefile.plugins                      | 2 +-
 profiles/network/{common.c => bnep.c} | 2 +-
 profiles/network/{common.h => bnep.h} | 0
 profiles/network/connection.c         | 2 +-
 profiles/network/manager.c            | 2 +-
 profiles/network/server.c             | 2 +-
 6 files changed, 5 insertions(+), 5 deletions(-)
 rename profiles/network/{common.c => bnep.c} (99%)
 rename profiles/network/{common.h => bnep.h} (100%)

diff --git a/Makefile.plugins b/Makefile.plugins
index f5025e9..6a1ddbf 100644
--- a/Makefile.plugins
+++ b/Makefile.plugins
@@ -47,7 +47,7 @@ builtin_sources += profiles/audio/control.h profiles/audio/control.c \
 
 builtin_modules += network
 builtin_sources += profiles/network/manager.c \
-			profiles/network/common.h profiles/network/common.c \
+			profiles/network/bnep.h profiles/network/bnep.c \
 			profiles/network/server.h profiles/network/server.c \
 			profiles/network/connection.h \
 			profiles/network/connection.c
diff --git a/profiles/network/common.c b/profiles/network/bnep.c
similarity index 99%
rename from profiles/network/common.c
rename to profiles/network/bnep.c
index fd25d96..03f9f57 100644
--- a/profiles/network/common.c
+++ b/profiles/network/bnep.c
@@ -43,7 +43,7 @@
 #include <glib.h>
 
 #include "log.h"
-#include "common.h"
+#include "bnep.h"
 #include "lib/uuid.h"
 
 #define CON_SETUP_RETRIES      3
diff --git a/profiles/network/common.h b/profiles/network/bnep.h
similarity index 100%
rename from profiles/network/common.h
rename to profiles/network/bnep.h
diff --git a/profiles/network/connection.c b/profiles/network/connection.c
index 5723afd..6ad0277 100644
--- a/profiles/network/connection.c
+++ b/profiles/network/connection.c
@@ -47,7 +47,7 @@
 #include "service.h"
 
 #include "error.h"
-#include "common.h"
+#include "bnep.h"
 #include "connection.h"
 
 #define NETWORK_PEER_INTERFACE "org.bluez.Network1"
diff --git a/profiles/network/manager.c b/profiles/network/manager.c
index ab4224d..8ac2dec 100644
--- a/profiles/network/manager.c
+++ b/profiles/network/manager.c
@@ -43,7 +43,7 @@
 #include "device.h"
 #include "profile.h"
 #include "service.h"
-#include "common.h"
+#include "bnep.h"
 #include "connection.h"
 #include "server.h"
 
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 3a7e52a..b3aab11 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -48,7 +48,7 @@
 #include "error.h"
 #include "sdpd.h"
 
-#include "common.h"
+#include "bnep.h"
 #include "server.h"
 
 #define NETWORK_SERVER_INTERFACE "org.bluez.NetworkServer1"
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH_v3 4/7] android/pan: Implement pan connect method in daemon
From: Ravi kumar Veeramally @ 2013-11-29  9:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1385718846-19070-1-git-send-email-ravikumar.veeramally@linux.intel.com>

Implements the PAN connect method in android daemon with PANU role
only. Setting up the bnep environment, adds connection and makes
bnep interface up are part of bnep_connect call. Notifies bnep
interface on control state call back and connection status on
connection state call back.
---
 android/Android.mk  |   2 +
 android/Makefile.am |   3 +-
 android/pan.c       | 218 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 215 insertions(+), 8 deletions(-)

diff --git a/android/Android.mk b/android/Android.mk
index c4d722d..549613c 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -42,6 +42,7 @@ LOCAL_SRC_FILES := \
 	../lib/hci.c \
 	../btio/btio.c \
 	../src/sdp-client.c \
+	../profiles/network/bnep.c \
 
 LOCAL_C_INCLUDES := \
 	$(call include-path-for, glib) \
@@ -66,6 +67,7 @@ lib_headers := \
 	sdp.h \
 	rfcomm.h \
 	sco.h \
+	bnep.h \
 
 $(shell mkdir -p $(LOCAL_PATH)/../lib/bluetooth)
 
diff --git a/android/Makefile.am b/android/Makefile.am
index 15ecf35..df04762 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -24,7 +24,8 @@ android_bluetoothd_SOURCES = android/main.c \
 				android/socket.h android/socket.c \
 				android/pan.h android/pan.c \
 				btio/btio.h btio/btio.c \
-				src/sdp-client.h src/sdp-client.c
+				src/sdp-client.h src/sdp-client.c \
+				profiles/network/bnep.h profiles/network/bnep.c
 
 android_bluetoothd_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
 
diff --git a/android/pan.c b/android/pan.c
index ea15637..7fe40da 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -29,35 +29,227 @@
 #include <fcntl.h>
 #include <glib.h>
 
+#include "btio/btio.h"
 #include "lib/bluetooth.h"
+#include "lib/bnep.h"
+#include "lib/sdp.h"
+#include "lib/sdp_lib.h"
+#include "src/glib-helper.h"
+#include "profiles/network/bnep.h"
+
 #include "log.h"
 #include "pan.h"
 #include "hal-msg.h"
 #include "ipc.h"
+#include "utils.h"
+#include "bluetooth.h"
 
-static uint8_t bt_pan_enable(struct hal_cmd_pan_enable *cmd, uint16_t len)
+static bdaddr_t adapter_addr;
+GSList *peers = NULL;
+uint8_t local_role = HAL_PAN_ROLE_NONE;
+
+struct network_peer {
+	char		dev[16];
+	bdaddr_t	dst;
+	uint8_t		conn_state;
+	uint8_t		role;
+	GIOChannel	*io;
+	guint		watch;
+};
+
+static int peer_cmp(gconstpointer s, gconstpointer user_data)
 {
-	DBG("Not Implemented");
+	const struct network_peer *np = s;
+	const bdaddr_t *dst = user_data;
 
-	return HAL_STATUS_FAILED;
+	return bacmp(&np->dst, dst);
 }
 
-static uint8_t bt_pan_get_role(void *cmd, uint16_t len)
+static void network_peer_free(struct network_peer *np)
+{
+	local_role = HAL_PAN_ROLE_NONE;
+
+	if (np->watch > 0) {
+		g_source_remove(np->watch);
+		np->watch = 0;
+	}
+
+	if (np->io) {
+		g_io_channel_unref(np->io);
+		np->io = NULL;
+	}
+
+	peers = g_slist_remove(peers, np);
+	g_free(np);
+	np = NULL;
+}
+
+static void bt_pan_notify_conn_state(struct network_peer *np, uint8_t state)
+{
+	struct hal_ev_pan_conn_state ev;
+	char addr[18];
+
+	if (np->conn_state == state)
+		return;
+
+	np->conn_state = state;
+	ba2str(&np->dst, addr);
+	DBG("device %s state %u", addr, state);
+
+	bdaddr2android(&np->dst, ev.bdaddr);
+	ev.state = state;
+	ev.local_role = local_role;
+	ev.remote_role = np->role;
+	ev.status = HAL_STATUS_SUCCESS;
+
+	ipc_send_notif(HAL_SERVICE_ID_PAN, HAL_EV_PAN_CONN_STATE, sizeof(ev),
+									&ev);
+}
+
+static void bt_pan_notify_ctrl_state(struct network_peer *np, uint8_t state)
+{
+	struct hal_ev_pan_ctrl_state ev;
+
+	DBG("");
+
+	ev.state = state;
+	ev.local_role = local_role;
+	ev.status = HAL_STATUS_SUCCESS;
+	memcpy(ev.name, np->dev, sizeof(np->dev));
+
+	ipc_send_notif(HAL_SERVICE_ID_PAN, HAL_EV_PAN_CTRL_STATE, sizeof(ev),
+									&ev);
+}
+
+static gboolean bnep_watchdog_cb(GIOChannel *chan, GIOCondition cond,
+								gpointer data)
+{
+	struct network_peer *np = data;
+
+	DBG("%s disconnected", np->dev);
+
+	bt_pan_notify_conn_state(np, HAL_PAN_STATE_DISCONNECTED);
+	network_peer_free(np);
+
+	return FALSE;
+}
+
+static void bnep_conn_cb(GIOChannel *chan, char *iface, int err, void *data)
+{
+	struct network_peer *np = data;
+
+	DBG("");
+
+	if (err < 0) {
+		error("bnep connect req failed: %s", strerror(-err));
+		bt_pan_notify_conn_state(np, HAL_PAN_STATE_DISCONNECTED);
+		network_peer_free(np);
+		return;
+	}
+
+	memcpy(np->dev, iface, sizeof(np->dev));
+
+	DBG("%s connected", np->dev);
+
+	bt_pan_notify_ctrl_state(np, HAL_PAN_CTRL_ENABLED);
+	bt_pan_notify_conn_state(np, HAL_PAN_STATE_CONNECTED);
+
+	np->watch = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+							bnep_watchdog_cb, np);
+	g_io_channel_unref(np->io);
+	np->io = NULL;
+}
+
+static void connect_cb(GIOChannel *chan, GError *err, gpointer data)
+{
+	struct network_peer *np = data;
+	uint16_t src, dst;
+	int perr;
+
+	DBG("");
+
+	if (err) {
+		error("%s", err->message);
+		bt_pan_notify_conn_state(np, HAL_PAN_STATE_DISCONNECTED);
+		network_peer_free(np);
+	}
+
+	src = (local_role == HAL_PAN_ROLE_NAP) ? BNEP_SVC_NAP : BNEP_SVC_PANU;
+	dst = (np->role == HAL_PAN_ROLE_NAP) ? BNEP_SVC_NAP : BNEP_SVC_PANU;
+
+	perr = bnep_connect(np->io, src, dst, bnep_conn_cb, np);
+	if (perr < 0) {
+		error("bnep connect req failed: %s", strerror(-perr));
+		bt_pan_notify_conn_state(np, HAL_PAN_STATE_DISCONNECTED);
+		network_peer_free(np);
+		return;
+	}
+}
+
+static uint8_t bt_pan_connect(struct hal_cmd_pan_connect *cmd, uint16_t len)
+{
+	struct network_peer *np;
+	bdaddr_t dst;
+	char addr[18];
+	GSList *l;
+	GError *gerr = NULL;
+
+	DBG("");
+
+	if (len < sizeof(*cmd))
+		return HAL_STATUS_INVALID;
+
+	android2bdaddr(&cmd->bdaddr, &dst);
+
+	l = g_slist_find_custom(peers, &dst, peer_cmp);
+	if (l)
+		return HAL_STATUS_FAILED;
+
+	np = g_new0(struct network_peer, 1);
+	bacpy(&np->dst, &dst);
+	local_role = cmd->local_role;
+	np->role = cmd->remote_role;
+
+	ba2str(&np->dst, addr);
+	DBG("connecting to %s %s", addr, np->dev);
+
+	np->io = bt_io_connect(connect_cb, np, NULL, &gerr,
+					BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
+					BT_IO_OPT_DEST_BDADDR, &np->dst,
+					BT_IO_OPT_PSM, BNEP_PSM,
+					BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
+					BT_IO_OPT_OMTU, BNEP_MTU,
+					BT_IO_OPT_IMTU, BNEP_MTU,
+					BT_IO_OPT_INVALID);
+	if (!np->io) {
+		error("%s", gerr->message);
+		g_error_free(gerr);
+		g_free(np);
+		return HAL_STATUS_FAILED;
+	}
+
+	peers = g_slist_append(peers, np);
+	bt_pan_notify_conn_state(np, HAL_PAN_STATE_CONNECTING);
+
+	return HAL_STATUS_SUCCESS;
+}
+
+static uint8_t bt_pan_disconnect(struct hal_cmd_pan_disconnect *cmd,
+								uint16_t len)
 {
 	DBG("Not Implemented");
 
 	return HAL_STATUS_FAILED;
 }
 
-static uint8_t bt_pan_connect(struct hal_cmd_pan_connect *cmd, uint16_t len)
+static uint8_t bt_pan_enable(struct hal_cmd_pan_enable *cmd, uint16_t len)
 {
 	DBG("Not Implemented");
 
 	return HAL_STATUS_FAILED;
 }
 
-static uint8_t bt_pan_disconnect(struct hal_cmd_pan_disconnect *cmd,
-								uint16_t len)
+static uint8_t bt_pan_get_role(void *cmd, uint16_t len)
 {
 	DBG("Not Implemented");
 
@@ -91,12 +283,24 @@ void bt_pan_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len)
 
 bool bt_pan_register(const bdaddr_t *addr)
 {
+	int err;
+
 	DBG("");
 
+	bacpy(&adapter_addr, addr);
+
+	err = bnep_init();
+	if (err) {
+		error("bnep init failed");
+		return false;
+	}
+
 	return true;
 }
 
 void bt_pan_unregister(void)
 {
 	DBG("");
+
+	bnep_cleanup();
 }
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH_v3 5/7] android/pan: Implement pan disconnect method in daemon
From: Ravi kumar Veeramally @ 2013-11-29  9:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1385718846-19070-1-git-send-email-ravikumar.veeramally@linux.intel.com>

Disconnect ongoing PANU role connection betweek devices, free
the device and notify the connection state.
---
 android/pan.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/android/pan.c b/android/pan.c
index 7fe40da..cb22fea 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -237,9 +237,35 @@ static uint8_t bt_pan_connect(struct hal_cmd_pan_connect *cmd, uint16_t len)
 static uint8_t bt_pan_disconnect(struct hal_cmd_pan_disconnect *cmd,
 								uint16_t len)
 {
-	DBG("Not Implemented");
+	struct network_peer *np;
+	GSList *l;
+	bdaddr_t dst;
 
-	return HAL_STATUS_FAILED;
+	DBG("");
+
+	if (len < sizeof(*cmd))
+		return HAL_STATUS_INVALID;
+
+	android2bdaddr(&cmd->bdaddr, &dst);
+
+	l = g_slist_find_custom(peers, &dst, peer_cmp);
+	if (!l)
+		return HAL_STATUS_FAILED;
+
+	np = l->data;
+
+	if (np->watch) {
+		g_source_remove(np->watch);
+		np->watch = 0;
+	}
+
+	bnep_if_down(np->dev);
+	bnep_kill_connection(&dst);
+
+	bt_pan_notify_conn_state(np, HAL_PAN_STATE_DISCONNECTED);
+	network_peer_free(np);
+
+	return HAL_STATUS_SUCCESS;
 }
 
 static uint8_t bt_pan_enable(struct hal_cmd_pan_enable *cmd, uint16_t len)
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH_v3 6/7] android/pan: Implement the get local role method in daemon
From: Ravi kumar Veeramally @ 2013-11-29  9:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1385718846-19070-1-git-send-email-ravikumar.veeramally@linux.intel.com>

Returns local role of the device (NONE, PANU or NAP).
---
 android/pan.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/android/pan.c b/android/pan.c
index cb22fea..87ae622 100644
--- a/android/pan.c
+++ b/android/pan.c
@@ -277,9 +277,15 @@ static uint8_t bt_pan_enable(struct hal_cmd_pan_enable *cmd, uint16_t len)
 
 static uint8_t bt_pan_get_role(void *cmd, uint16_t len)
 {
-	DBG("Not Implemented");
+	struct hal_rsp_pan_get_role rsp;
 
-	return HAL_STATUS_FAILED;
+	DBG("");
+
+	rsp.local_role = local_role;
+	ipc_send_rsp_full(HAL_SERVICE_ID_PAN, HAL_OP_PAN_GET_ROLE, sizeof(rsp),
+								&rsp, -1);
+
+	return HAL_STATUS_SUCCESS;
 }
 
 void bt_pan_handle_cmd(int sk, uint8_t opcode, void *buf, uint16_t len)
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH_v3 7/7] android: Add reasons for adding capabilites to process
From: Ravi kumar Veeramally @ 2013-11-29  9:54 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Ravi kumar Veeramally
In-Reply-To: <1385718846-19070-1-git-send-email-ravikumar.veeramally@linux.intel.com>

CAP_NET_ADMIN: Allow use of MGMT interface
CAP_NET_BIND_SERVICE: Allow use of privileged PSM
CAP_NET_RAW: Allow use of bnep ioctl calls
---
 android/main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/android/main.c b/android/main.c
index 3a14af5..eedca58 100644
--- a/android/main.c
+++ b/android/main.c
@@ -534,6 +534,9 @@ static bool set_capabilities(void)
 	header.version = _LINUX_CAPABILITY_VERSION;
 	header.pid = 0;
 
+	/* CAP_NET_ADMIN: Allow use of MGMT interface
+	 * CAP_NET_BIND_SERVICE: Allow use of privileged PSM
+	 * CAP_NET_RAW: Allow use of bnep ioctl calls */
 	cap.effective = cap.permitted =
 		CAP_TO_MASK(CAP_NET_RAW) |
 		CAP_TO_MASK(CAP_NET_ADMIN) |
-- 
1.8.3.2


^ permalink raw reply related

* [PATCH v2 0/9] android: IPC improvements - daemon part
From: Szymon Janc @ 2013-11-29 11:08 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

v2:
 - rebased against latest IPC helpers improvements
 - more compact command handlers table format
 - error handling path in command handlers improved according to Johan comments
 - randmon small fixes
 - patches not directly related to refactor removed from serie, will
   be send after this is merged

v1:
This serie implements IPC message handling iprovments in daemon similar
to what is already done in HAL part.

-- 
BR
Szymon Janc

Szymon Janc (9):
  android: Add initial code for IPC message handlers
  android/main: Use generic IPC message handling for core service
  android/main: Use common exit path in core service functions
  android/bluetooth: Use generic IPC msg handling for commands
  android/bluetooth: Make property handling function return HAL status
  android/hidhost: Use generic IPC message handling for commands
  android/pan: Use generic IPC message handling for commands
  android/a2dp: Use generic IPC message handling for commands
  android/socket: Use generic IPC message handling for commands

 android/a2dp.c      |  69 ++++----
 android/a2dp.h      |   2 -
 android/bluetooth.c | 477 ++++++++++++++++++++++++++++++++++------------------
 android/hidhost.c   | 309 ++++++++++++++++++++--------------
 android/hidhost.h   |   2 -
 android/ipc.c       |  78 +++++++++
 android/ipc.h       |  10 ++
 android/main.c      | 123 +++++---------
 android/pan.c       |  57 +++----
 android/pan.h       |   2 -
 android/socket.c    | 102 ++++++-----
 11 files changed, 736 insertions(+), 495 deletions(-)

-- 
1.8.3.2


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox