* [PATCH 1/2 v3] adapter: Add btd_request_authorization_cable_configured()
@ 2015-07-07 14:14 Bastien Nocera
2015-07-23 22:32 ` Szymon Janc
0 siblings, 1 reply; 3+ messages in thread
From: Bastien Nocera @ 2015-07-07 14:14 UTC (permalink / raw)
To: linux-bluetooth@vger.kernel.org
Add btd_request_authorization_cable_configured() function to allow
cable configured devices to ask the user straight away about whether the
device should be allowed to connect to the computer.
This allows us to ask the user at the time of the USB connection and
initial setup, rather than when the first Bluetooth connection is made.
---
src/adapter.c | 42 ++++++++++++++++++++++++++++++++++++------
src/adapter.h | 2 ++
2 files changed, 38 insertions(+), 6 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 95b5349..cf09d4b 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -262,6 +262,11 @@ struct btd_adapter {
bool is_default; /* true if adapter is default one */
};
+typedef enum {
+ ADAPTER_AUTHORIZE_DISCONNECTED = 0,
+ ADAPTER_AUTHORIZE_CHECK_CONNECTED
+} adapter_authorize_type;
+
static struct btd_adapter *btd_adapter_lookup(uint16_t index)
{
GList *list;
@@ -5814,8 +5819,9 @@ static void svc_complete(struct btd_device *dev, int err, void *user_data)
}
static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
- const char *uuid, service_auth_cb cb,
- void *user_data)
+ const char *uuid,
+ adapter_authorize_type check_for_connection,
+ service_auth_cb cb, void *user_data)
{
struct service_auth *auth;
struct btd_device *device;
@@ -5826,7 +5832,7 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
return 0;
/* Device connected? */
- if (!g_slist_find(adapter->connections, device))
+ if (check_for_connection && !g_slist_find(adapter->connections, device))
error("Authorization request for non-connected device!?");
auth = g_try_new0(struct service_auth, 1);
@@ -5839,7 +5845,12 @@ static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t *dst,
auth->device = device;
auth->adapter = adapter;
auth->id = ++id;
- auth->svc_id = device_wait_for_svc_complete(device, svc_complete, auth);
+ if (check_for_connection)
+ auth->svc_id = device_wait_for_svc_complete(device, svc_complete, auth);
+ else {
+ if (adapter->auth_idle_id == 0)
+ adapter->auth_idle_id = g_idle_add(process_auth_queue, adapter);
+ }
g_queue_push_tail(adapter->auths, auth);
@@ -5858,7 +5869,8 @@ guint btd_request_authorization(const bdaddr_t *src, const bdaddr_t *dst,
if (!adapter)
return 0;
- return adapter_authorize(adapter, dst, uuid, cb, user_data);
+ return adapter_authorize(adapter, dst, uuid,
+ ADAPTER_AUTHORIZE_CHECK_CONNECTED, cb, user_data);
}
for (l = adapters; l != NULL; l = g_slist_next(l)) {
@@ -5866,7 +5878,8 @@ guint btd_request_authorization(const bdaddr_t *src, const bdaddr_t *dst,
adapter = l->data;
- id = adapter_authorize(adapter, dst, uuid, cb, user_data);
+ id = adapter_authorize(adapter, dst, uuid,
+ ADAPTER_AUTHORIZE_CHECK_CONNECTED, cb, user_data);
if (id != 0)
return id;
}
@@ -5874,6 +5887,23 @@ guint btd_request_authorization(const bdaddr_t *src, const bdaddr_t *dst,
return 0;
}
+guint btd_request_authorization_cable_configured(const bdaddr_t *src, const bdaddr_t *dst,
+ const char *uuid, service_auth_cb cb,
+ void *user_data)
+{
+ struct btd_adapter *adapter;
+
+ if (bacmp(src, BDADDR_ANY) == 0)
+ return 0;
+
+ adapter = adapter_find(src);
+ if (!adapter)
+ return 0;
+
+ return adapter_authorize(adapter, dst, uuid,
+ ADAPTER_AUTHORIZE_DISCONNECTED, cb, user_data);
+}
+
static struct service_auth *find_authorization(guint id)
{
GSList *l;
diff --git a/src/adapter.h b/src/adapter.h
index 0c95f5d..b9da6cf 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -119,6 +119,8 @@ int btd_register_adapter_driver(struct btd_adapter_driver *driver);
void btd_unregister_adapter_driver(struct btd_adapter_driver *driver);
guint btd_request_authorization(const bdaddr_t *src, const bdaddr_t *dst,
const char *uuid, service_auth_cb cb, void *user_data);
+guint btd_request_authorization_cable_configured(const bdaddr_t *src, const bdaddr_t *dst,
+ const char *uuid, service_auth_cb cb, void *user_data);
int btd_cancel_authorization(guint id);
int btd_adapter_restore_powered(struct btd_adapter *adapter);
--
2.4.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/2 v3] adapter: Add btd_request_authorization_cable_configured()
2015-07-07 14:14 [PATCH 1/2 v3] adapter: Add btd_request_authorization_cable_configured() Bastien Nocera
@ 2015-07-23 22:32 ` Szymon Janc
2015-07-24 11:09 ` Bastien Nocera
0 siblings, 1 reply; 3+ messages in thread
From: Szymon Janc @ 2015-07-23 22:32 UTC (permalink / raw)
To: Bastien Nocera; +Cc: linux-bluetooth@vger.kernel.org
Hi Bastien,
On Tuesday 07 July 2015 16:14:00 Bastien Nocera wrote:
> Add btd_request_authorization_cable_configured() function to allow
> cable configured devices to ask the user straight away about whether the
> device should be allowed to connect to the computer.
>
> This allows us to ask the user at the time of the USB connection and
> initial setup, rather than when the first Bluetooth connection is made.
I'd also add a note to doc/agent-api.txt that this can be called also for not
connected devices under some circumstances (and see how it fits).
What bothers me is that now bluetoothd will send AuthorizeService request for
device that is not connected (and authorizing will not result in connection).
And this was explicitly checked against before this patch.
> ---
> src/adapter.c | 42 ++++++++++++++++++++++++++++++++++++------
> src/adapter.h | 2 ++
> 2 files changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 95b5349..cf09d4b 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -262,6 +262,11 @@ struct btd_adapter {
> bool is_default; /* true if adapter is default one */
> };
>
> +typedef enum {
> + ADAPTER_AUTHORIZE_DISCONNECTED = 0,
> + ADAPTER_AUTHORIZE_CHECK_CONNECTED
> +} adapter_authorize_type;
> +
> static struct btd_adapter *btd_adapter_lookup(uint16_t index)
> {
> GList *list;
> @@ -5814,8 +5819,9 @@ static void svc_complete(struct btd_device *dev, int
> err, void *user_data) }
>
> static int adapter_authorize(struct btd_adapter *adapter, const bdaddr_t
> *dst, - const char *uuid, service_auth_cb cb,
> - void *user_data)
> + const char *uuid,
> + adapter_authorize_type check_for_connection,
> + service_auth_cb cb, void *user_data)
> {
> struct service_auth *auth;
> struct btd_device *device;
> @@ -5826,7 +5832,7 @@ static int adapter_authorize(struct btd_adapter
> *adapter, const bdaddr_t *dst, return 0;
>
> /* Device connected? */
> - if (!g_slist_find(adapter->connections, device))
> + if (check_for_connection && !g_slist_find(adapter->connections, device))
> error("Authorization request for non-connected device!?");
Since check_for_connection is enum this should be compared to one of its
values.
>
> auth = g_try_new0(struct service_auth, 1);
> @@ -5839,7 +5845,12 @@ static int adapter_authorize(struct btd_adapter
> *adapter, const bdaddr_t *dst, auth->device = device;
> auth->adapter = adapter;
> auth->id = ++id;
> - auth->svc_id = device_wait_for_svc_complete(device, svc_complete, auth);
> + if (check_for_connection)
> + auth->svc_id = device_wait_for_svc_complete(device, svc_complete,
auth);
> + else {
> + if (adapter->auth_idle_id == 0)
> + adapter->auth_idle_id = g_idle_add(process_auth_queue, adapter);
> + }
>
> g_queue_push_tail(adapter->auths, auth);
>
> @@ -5858,7 +5869,8 @@ guint btd_request_authorization(const bdaddr_t *src,
> const bdaddr_t *dst, if (!adapter)
> return 0;
>
> - return adapter_authorize(adapter, dst, uuid, cb, user_data);
> + return adapter_authorize(adapter, dst, uuid,
> + ADAPTER_AUTHORIZE_CHECK_CONNECTED, cb, user_data);
> }
>
> for (l = adapters; l != NULL; l = g_slist_next(l)) {
> @@ -5866,7 +5878,8 @@ guint btd_request_authorization(const bdaddr_t *src,
> const bdaddr_t *dst,
>
> adapter = l->data;
>
> - id = adapter_authorize(adapter, dst, uuid, cb, user_data);
> + id = adapter_authorize(adapter, dst, uuid,
> + ADAPTER_AUTHORIZE_CHECK_CONNECTED, cb, user_data);
> if (id != 0)
> return id;
> }
> @@ -5874,6 +5887,23 @@ guint btd_request_authorization(const bdaddr_t *src,
> const bdaddr_t *dst, return 0;
> }
>
> +guint btd_request_authorization_cable_configured(const bdaddr_t *src, const
> bdaddr_t *dst, + const char *uuid, service_auth_cb cb,
> + void *user_data)
> +{
> + struct btd_adapter *adapter;
> +
> + if (bacmp(src, BDADDR_ANY) == 0)
> + return 0;
> +
> + adapter = adapter_find(src);
> + if (!adapter)
> + return 0;
> +
> + return adapter_authorize(adapter, dst, uuid,
> + ADAPTER_AUTHORIZE_DISCONNECTED, cb, user_data);
> +}
> +
> static struct service_auth *find_authorization(guint id)
> {
> GSList *l;
> diff --git a/src/adapter.h b/src/adapter.h
> index 0c95f5d..b9da6cf 100644
> --- a/src/adapter.h
> +++ b/src/adapter.h
> @@ -119,6 +119,8 @@ int btd_register_adapter_driver(struct
> btd_adapter_driver *driver); void btd_unregister_adapter_driver(struct
> btd_adapter_driver *driver); guint btd_request_authorization(const bdaddr_t
> *src, const bdaddr_t *dst, const char *uuid, service_auth_cb cb, void
> *user_data);
> +guint btd_request_authorization_cable_configured(const bdaddr_t *src, const
> bdaddr_t *dst, + const char *uuid, service_auth_cb cb, void
*user_data);
> int btd_cancel_authorization(guint id);
>
> int btd_adapter_restore_powered(struct btd_adapter *adapter);
Also in general I'd prefer to use sixaxis instead of cable in names. Mainly
for consistency with other core sixaxis specific code.
--
Szymon K. Janc
szymon.janc@gmail.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/2 v3] adapter: Add btd_request_authorization_cable_configured()
2015-07-23 22:32 ` Szymon Janc
@ 2015-07-24 11:09 ` Bastien Nocera
0 siblings, 0 replies; 3+ messages in thread
From: Bastien Nocera @ 2015-07-24 11:09 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
On Fri, 2015-07-24 at 00:32 +0200, Szymon Janc wrote:
> Hi Bastien,
>
> On Tuesday 07 July 2015 16:14:00 Bastien Nocera wrote:
> > Add btd_request_authorization_cable_configured() function to allow
> > cable configured devices to ask the user straight away about
> > whether the
> > device should be allowed to connect to the computer.
> >
> > This allows us to ask the user at the time of the USB connection
> > and
> > initial setup, rather than when the first Bluetooth connection is
> > made.
>
> I'd also add a note to doc/agent-api.txt that this can be called also
> for not
> connected devices under some circumstances (and see how it fits).
As I mentioned on IRC, I'm happy to do that if the rest of the patch is
deemed fine.
> What bothers me is that now bluetoothd will send AuthorizeService
> request for
> device that is not connected (and authorizing will not result in
> connection).
That's not what that patch does though, this patch just adds wiring to
do that...
> And this was explicitly checked against before this patch.
It's still checked, just not through the sixaxis plugin. That's the
only entry point that disables that check.
<snip>
>
> Also in general I'd prefer to use sixaxis instead of cable in names.
> Mainly
> for consistency with other core sixaxis specific code.
It's not sixaxis specific, and could be used by any cable pairing,
where the device is useful both when plugged in and then through BT.
Johan is the one that preferred cable rather than sixaxis, as the
method isn't specific to sixaxis but could be used for other devices
too.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-07-24 11:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-07 14:14 [PATCH 1/2 v3] adapter: Add btd_request_authorization_cable_configured() Bastien Nocera
2015-07-23 22:32 ` Szymon Janc
2015-07-24 11:09 ` Bastien Nocera
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).