[PATCH v2 0/4] bluetooth: Add bluetooth server support

[sim-auth PATCHv2 1/3] Add...

[PATCH 1/1] gisi: Updated...

Frédéric Danis

Friday, 28 January 2011 Fri, 28 Jan '11

2:05 a.m.

It watches Bluetooth adapter property changes and adds SDP record to listen client connection request. It supports multiple adapters, servers and client connections. Use of GSList instead of GHashTable to store adapter path and address as we need to get address from adapter or vice-versa (during authorization phase). Frédéric Danis (4): bluetooth: only use bluetooth_refcount in bluetooth_ref/bluetooth_unref bluetooth: use GSList instead of GHashTable to store adapter path/address bluetooth: Add bluetooth server support bluetooth: add Bluetooth service authorization support Makefile.am | 1 + plugins/bluetooth.c | 558 +++++++++++++++++++++++++++++++++++++++++++++++++-- plugins/bluetooth.h | 9 + 3 files changed, 551 insertions(+), 17 deletions(-)

Show replies by thread

Frédéric Danis

Friday, 28 January Fri, 28 Jan

2:05 a.m.

New subject: [PATCH 1/4] bluetooth: only use bluetooth_refcount in bluetooth_ref/bluetooth_unref

--- plugins/bluetooth.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-) diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c index 93dd7a1..e59bd31 100644 --- a/plugins/bluetooth.c +++ b/plugins/bluetooth.c @@ -505,7 +505,7 @@ static guint adapter_added_watch; static guint adapter_removed_watch; static guint property_watch; -static void bluetooth_ref(void) +static int bluetooth_ref(void) { if (bluetooth_refcount > 0) goto increment; @@ -544,13 +544,15 @@ static void bluetooth_ref(void) increment: g_atomic_int_inc(&bluetooth_refcount); - return; + return 0; remove: g_dbus_remove_watch(connection, bluetooth_watch); g_dbus_remove_watch(connection, adapter_added_watch); g_dbus_remove_watch(connection, adapter_removed_watch); g_dbus_remove_watch(connection, property_watch); + + return -EIO; } static void bluetooth_unref(void) @@ -569,10 +571,12 @@ static void bluetooth_unref(void) int bluetooth_register_uuid(const char *uuid, struct bluetooth_profile *profile) { - bluetooth_ref(); + int err; + + err = bluetooth_ref(); - if (bluetooth_refcount == 0) - return -EIO; + if (err != 0) + return err; g_hash_table_insert(uuid_hash, g_strdup(uuid), profile); -- 1.7.1

Frédéric Danis

2:05 a.m.

New subject: [PATCH 2/4] bluetooth: use GSList instead of GHashTable to store adapter path/address

--- plugins/bluetooth.c | 86 +++++++++++++++++++++++++++++++++++++++++++------- 1 files changed, 74 insertions(+), 12 deletions(-) diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c index e59bd31..4da662a 100644 --- a/plugins/bluetooth.c +++ b/plugins/bluetooth.c @@ -39,9 +39,14 @@ static DBusConnection *connection; static GHashTable *uuid_hash = NULL; -static GHashTable *adapter_address_hash = NULL; +static GSList *adapter_list = NULL; static gint bluetooth_refcount; +struct adapter_address { + char *adapter; + char *address; +}; + void bluetooth_create_path(const char *dev_addr, const char *adapter_addr, char *buf, int size) { @@ -235,6 +240,52 @@ static void parse_string(DBusMessageIter *iter, gpointer user_data) dbus_message_iter_get_basic(iter, str); } +static gint adapter_compare(gconstpointer a, gconstpointer b) +{ + const struct adapter_address *entry = a; + const char *key = b; + + return g_strcmp0(entry->adapter, key); +} + +static GSList* adapter_address_add(GSList *list, const char *path, + const char *address) +{ + GSList *l; + struct adapter_address *entry; + + l = g_slist_find_custom(adapter_list, path, adapter_compare); + if (l != NULL) { + entry = l->data; + + g_free(entry->address); + + entry->address = g_strdup(address); + + return list; + } + + entry = g_try_new0(struct adapter_address, 1); + if (entry == NULL) { + ofono_error("Unable to allocate adapter_address structure"); + return list; + } + + entry->adapter = g_strdup(path); + entry->address = g_strdup(address); + + return g_slist_prepend(list, entry); +} + +static GSList* adapter_address_remove(GSList *l, struct adapter_address *entry) +{ + g_free(entry->adapter); + g_free(entry->address); + g_free(entry); + + return g_slist_remove(l, entry); +} + static void device_properties_cb(DBusPendingCall *call, gpointer user_data) { DBusMessage *reply; @@ -245,6 +296,7 @@ static void device_properties_cb(DBusPendingCall *call, gpointer user_data) const char *device_addr = NULL; const char *alias = NULL; struct bluetooth_profile *profile; + GSList *entry; reply = dbus_pending_call_steal_reply(call); @@ -266,9 +318,15 @@ static void device_properties_cb(DBusPendingCall *call, gpointer user_data) "Address", parse_string, &device_addr, "Alias", parse_string, &alias, NULL); - if (adapter) - adapter_addr = g_hash_table_lookup(adapter_address_hash, - adapter); + if (adapter) { + entry = g_slist_find_custom(adapter_list, adapter, + adapter_compare); + if (entry != NULL) { + struct adapter_address *a = entry->data; + + adapter_addr = a->address; + } + } if ((have_uuid & HFP_AG) && device_addr && adapter_addr) { profile = g_hash_table_lookup(uuid_hash, HFP_AG_UUID); @@ -392,8 +450,7 @@ static void adapter_properties_cb(DBusPendingCall *call, gpointer user_data) NULL); DBG("Adapter Address: %s, Path: %s", addr, path); - g_hash_table_insert(adapter_address_hash, - g_strdup(path), g_strdup(addr)); + adapter_list = adapter_address_add(adapter_list, path, addr); for (l = device_list; l; l = l->next) { const char *device = l->data; @@ -429,10 +486,16 @@ static gboolean adapter_removed(DBusConnection *connection, DBusMessage *message, void *user_data) { const char *path; + GSList *l; if (dbus_message_get_args(message, NULL, DBUS_TYPE_OBJECT_PATH, &path, - DBUS_TYPE_INVALID) == TRUE) - g_hash_table_remove(adapter_address_hash, path); + DBUS_TYPE_INVALID) == TRUE) { + l = g_slist_find_custom(adapter_list, path, adapter_compare); + + if (l != NULL) + adapter_list = adapter_address_remove(adapter_list, + l->data); + } return TRUE; } @@ -538,9 +601,6 @@ static int bluetooth_ref(void) uuid_hash = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL); - adapter_address_hash = g_hash_table_new_full(g_str_hash, g_str_equal, - g_free, g_free); - increment: g_atomic_int_inc(&bluetooth_refcount); @@ -566,7 +626,9 @@ static void bluetooth_unref(void) g_dbus_remove_watch(connection, property_watch); g_hash_table_destroy(uuid_hash); - g_hash_table_destroy(adapter_address_hash); + while (adapter_list) + adapter_list = adapter_address_remove(adapter_list, + adapter_list->data); } int bluetooth_register_uuid(const char *uuid, struct bluetooth_profile *profile) -- 1.7.1

Frédéric Danis

2:05 a.m.

New subject: [PATCH 3/4] bluetooth: Add bluetooth server support

--- Makefile.am | 1 + plugins/bluetooth.c | 340 +++++++++++++++++++++++++++++++++++++++++++++++++++ plugins/bluetooth.h | 9 ++ 3 files changed, 350 insertions(+), 0 deletions(-) diff --git a/Makefile.am b/Makefile.am index 9b77e63..c0efe28 100644 --- a/Makefile.am +++ b/Makefile.am @@ -318,6 +318,7 @@ builtin_sources += plugins/bluetooth.c plugins/bluetooth.h builtin_modules += hfp builtin_sources += plugins/hfp.c plugins/bluetooth.h +builtin_sources += $(btio_sources) builtin_cflags += @BLUEZ_CFLAGS@ builtin_libadd += @BLUEZ_LIBS@ endif diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c index 4da662a..86d4c25 100644 --- a/plugins/bluetooth.c +++ b/plugins/bluetooth.c @@ -35,18 +35,36 @@ #include <ofono/dbus.h> +#include <btio.h> #include "bluetooth.h" static DBusConnection *connection; static GHashTable *uuid_hash = NULL; static GSList *adapter_list = NULL; static gint bluetooth_refcount; +static GSList *server_list = NULL; struct adapter_address { char *adapter; char *address; }; +struct server { + guint8 channel; + char *sdp_record; + GIOChannel *io; + GHashTable *adapter_hash; + ConnectFunc connect_cb; + gpointer user_data; + GSList *client_list; +}; + +struct cb_data { + struct server *server; + char *path; + guint source; +}; + void bluetooth_create_path(const char *dev_addr, const char *adapter_addr, char *buf, int size) { @@ -429,6 +447,277 @@ static gboolean property_changed(DBusConnection *connection, DBusMessage *msg, return TRUE; } +static void remove_record(char *path, guint handle, struct server *server) +{ + DBusMessage *msg; + + msg = dbus_message_new_method_call(BLUEZ_SERVICE, path, + BLUEZ_SERVICE_INTERFACE, + "RemoveRecord"); + if (msg == NULL) { + ofono_error("Unable to allocate D-Bus RemoveRecord message"); + return; + } + + dbus_message_append_args(msg, DBUS_TYPE_UINT32, &handle, + DBUS_TYPE_INVALID); + g_dbus_send_message(connection, msg); + + ofono_info("Unregistered handle for %s, channel %d: 0x%x", path, + server->channel, handle); +} + +static void server_stop(struct server *server) +{ + while (server->client_list) { + g_source_remove((guint) server->client_list->data); + server->client_list = g_slist_remove(server->client_list, + server->client_list->data); + } + + g_hash_table_foreach_remove(server->adapter_hash, + (GHRFunc) remove_record, server); + + if (server->io != NULL) { + g_io_channel_shutdown(server->io, TRUE, NULL); + g_io_channel_unref(server->io); + server->io = NULL; + } +} + +static void cb_data_destroy(gpointer data) +{ + struct cb_data *cb_data = data; + + if (cb_data->path != NULL) + g_free(cb_data->path); + g_free(cb_data); +} + +static gboolean client_event(GIOChannel *chan, GIOCondition cond, gpointer data) +{ + struct cb_data *cb_data = data; + struct server *server = cb_data->server; + + server->client_list = g_slist_remove(server->client_list, + (void *) cb_data->source); + + cb_data_destroy(cb_data); + + return FALSE; +} + +static void confirm_event(GIOChannel *io, gpointer user_data) +{ + struct server *server = user_data; + struct cb_data *client_data; + GError *err = NULL; + char laddress[18], raddress[18]; + guint8 channel; + + bt_io_get(io, BT_IO_RFCOMM, &err, BT_IO_OPT_SOURCE, laddress, + BT_IO_OPT_DEST, raddress, + BT_IO_OPT_CHANNEL, &channel, + BT_IO_OPT_INVALID); + if (err) { + ofono_error("%s", err->message); + g_error_free(err); + return; + } + + ofono_info("New connection for %s on channel %u from: %s,", laddress, + channel, raddress); + + if (!bt_io_accept(io, server->connect_cb, server->user_data, + NULL, &err)) { + ofono_error("%s", err->message); + g_error_free(err); + g_io_channel_unref(io); + return; + } + + client_data = g_try_new0(struct cb_data, 1); + if (client_data == NULL) { + ofono_error("Unable to allocate client cb_data structure"); + return; + } + + client_data->server = server; + client_data->source = g_io_add_watch(io, + G_IO_HUP | G_IO_ERR | G_IO_NVAL, + client_event, client_data); + server->client_list = g_slist_prepend(server->client_list, + (void *)client_data->source); +} + +static void add_record_cb(DBusPendingCall *call, gpointer user_data) +{ + struct cb_data *cb_data = user_data; + DBusMessage *reply = dbus_pending_call_steal_reply(call); + DBusError derr; + guint32 handle; + + dbus_error_init(&derr); + + if (dbus_set_error_from_message(&derr, reply)) { + ofono_error("Replied with an error: %s, %s", + derr.name, derr.message); + dbus_error_free(&derr); + g_free(cb_data->path); + goto done; + } + + dbus_message_get_args(reply, NULL, DBUS_TYPE_UINT32, &handle, + DBUS_TYPE_INVALID); + + g_hash_table_insert(cb_data->server->adapter_hash, cb_data->path, + (void *) handle); + + ofono_info("Registered handle for %s, channel %d: 0x%x", cb_data->path, + cb_data->server->channel, handle); + +done: + /* Do not free cb_data->path, it is used in adapter_hash */ + g_free(cb_data); + dbus_message_unref(reply); +} + +static void server_adapter_properties_cb(DBusPendingCall *call, + gpointer user_data) +{ + struct cb_data *cb_data = user_data; + const char *path = cb_data->path; + DBusMessage *reply; + const char *addr; + struct server *server = cb_data->server; + + reply = dbus_pending_call_steal_reply(call); + + if (dbus_message_is_error(reply, DBUS_ERROR_SERVICE_UNKNOWN)) { + DBG("Bluetooth daemon is apparently not available."); + goto done; + } + + bluetooth_parse_properties(reply, "Address", parse_string, &addr, NULL); + + DBG("Adapter Address: %s, Path: %s", addr, path); + adapter_list = adapter_address_add(adapter_list, path, addr); + + DBG("Add record on %s", path); + + bluetooth_send_with_reply(path, BLUEZ_SERVICE_INTERFACE, + "AddRecord", add_record_cb, + cb_data, NULL, -1, + DBUS_TYPE_STRING, &server->sdp_record, + DBUS_TYPE_INVALID); + +done: + dbus_message_unref(reply); +} + +static void server_parse_adapters(DBusMessageIter *array, gpointer user_data) +{ + struct server *server = user_data; + DBusMessageIter value; + struct cb_data *cb_data; + + if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY) + return; + + dbus_message_iter_recurse(array, &value); + + while (dbus_message_iter_get_arg_type(&value) + == DBUS_TYPE_OBJECT_PATH) { + const char *path; + + dbus_message_iter_get_basic(&value, &path); + + DBG("Calling GetProperties on %s", path); + + cb_data = g_try_new0(struct cb_data, 1); + if (cb_data == NULL) { + ofono_error("Unable to allocate cb_data structure"); + return; + } + + cb_data->server = server; + cb_data->path = g_strdup(path); + + bluetooth_send_with_reply(path, BLUEZ_ADAPTER_INTERFACE, + "GetProperties", server_adapter_properties_cb, + cb_data, NULL, -1, DBUS_TYPE_INVALID); + + dbus_message_iter_next(&value); + } +} + +static void server_manager_properties_cb(DBusPendingCall *call, + gpointer user_data) +{ + DBusMessage *reply; + + reply = dbus_pending_call_steal_reply(call); + + if (dbus_message_is_error(reply, DBUS_ERROR_SERVICE_UNKNOWN)) { + DBG("Bluetooth daemon is apparently not available."); + goto done; + } + + bluetooth_parse_properties(reply, "Adapters", server_parse_adapters, + user_data, NULL); + +done: + dbus_message_unref(reply); +} + +static void server_start(struct server *server, char *path) +{ + struct cb_data *cb_data; + GError *err = NULL; + + if (server->io != NULL) + goto out; + + server->io = bt_io_listen(BT_IO_RFCOMM, NULL, confirm_event, + server, NULL, &err, + BT_IO_OPT_CHANNEL, server->channel, + BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM, + BT_IO_OPT_INVALID); + if (server->io == NULL) { + ofono_error("Bluetooth channel %d register failed: %s", + server->channel, err->message); + g_error_free(err); + server_stop(server); + return; + } + +out: + if (server->sdp_record == NULL) + return; + + if (path != NULL) { + cb_data = g_try_new0(struct cb_data, 1); + if (cb_data == NULL) { + ofono_error("Unable to allocate cb_data structure"); + return; + } + + cb_data->server = server; + cb_data->path = g_strdup(path); + + bluetooth_send_with_reply(path, BLUEZ_SERVICE_INTERFACE, + "AddRecord", add_record_cb, + cb_data, NULL, -1, + DBUS_TYPE_STRING, &server->sdp_record, + DBUS_TYPE_INVALID); + } else + bluetooth_send_with_reply("/", BLUEZ_MANAGER_INTERFACE, + "GetProperties", + server_manager_properties_cb, server, + NULL, -1, DBUS_TYPE_INVALID); + +} + static void adapter_properties_cb(DBusPendingCall *call, gpointer user_data) { const char *path = user_data; @@ -479,6 +768,10 @@ static gboolean adapter_added(DBusConnection *connection, DBusMessage *message, "GetProperties", adapter_properties_cb, g_strdup(path), g_free, -1, DBUS_TYPE_INVALID); + if (server_list) + g_slist_foreach(server_list, (GFunc) server_start, + (gpointer) path); + return TRUE; } @@ -497,6 +790,13 @@ static gboolean adapter_removed(DBusConnection *connection, l->data); } + for (l = server_list; l; l = l->next) { + struct server *server = l->data; + + /* Handle have already been removed, so removing related path */ + g_hash_table_remove(server->adapter_hash, path); + } + return TRUE; } @@ -656,5 +956,45 @@ void bluetooth_unregister_uuid(const char *uuid) bluetooth_unref(); } +struct server *bluetooth_register_server(guint8 channel, const char *sdp_record, + ConnectFunc cb, gpointer user_data) +{ + struct server *server; + + server = g_try_new0(struct server, 1); + if (!server) + return NULL; + + if (bluetooth_ref() != 0) { + g_free(server); + return NULL; + } + + server->channel = channel; + if (sdp_record != NULL) + server->sdp_record = g_strdup(sdp_record); + server->connect_cb = cb; + server->user_data = user_data; + server->adapter_hash = g_hash_table_new_full(g_str_hash, g_str_equal, + g_free, NULL); + + server_start(server, NULL); + + server_list = g_slist_prepend(server_list, server); + + return server; +} + +void bluetooth_unregister_server(struct server *server) +{ + server_list = g_slist_remove(server_list, server); + server_stop(server); + g_hash_table_destroy(server->adapter_hash); + g_free(server->sdp_record); + g_free(server); + + bluetooth_unref(); +} + OFONO_PLUGIN_DEFINE(bluetooth, "Bluetooth Utils Plugins", VERSION, OFONO_PLUGIN_PRIORITY_DEFAULT, NULL, NULL) diff --git a/plugins/bluetooth.h b/plugins/bluetooth.h index 42b0d13..505d908 100644 --- a/plugins/bluetooth.h +++ b/plugins/bluetooth.h @@ -23,6 +23,7 @@ #define BLUEZ_MANAGER_INTERFACE BLUEZ_SERVICE ".Manager" #define BLUEZ_ADAPTER_INTERFACE BLUEZ_SERVICE ".Adapter" #define BLUEZ_DEVICE_INTERFACE BLUEZ_SERVICE ".Device" +#define BLUEZ_SERVICE_INTERFACE BLUEZ_SERVICE ".Service" #define DBUS_TIMEOUT 15 @@ -39,10 +40,18 @@ struct bluetooth_profile { void (*set_alias)(const char *device, const char *); }; +struct server; + +typedef void (*ConnectFunc)(GIOChannel *io, GError *err, gpointer user_data); + int bluetooth_register_uuid(const char *uuid, struct bluetooth_profile *profile); void bluetooth_unregister_uuid(const char *uuid); +struct server *bluetooth_register_server(guint8 channel, const char *sdp_record, + ConnectFunc cb, gpointer user_data); +void bluetooth_unregister_server(struct server *server); + void bluetooth_create_path(const char *dev_addr, const char *adapter_addr, char *buf, int size); -- 1.7.1

Frédéric Danis

2:05 a.m.

New subject: [PATCH 4/4] bluetooth: add Bluetooth service authorization support

--- plugins/bluetooth.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 127 insertions(+), 9 deletions(-) diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c index 86d4c25..7e4dbc1 100644 --- a/plugins/bluetooth.c +++ b/plugins/bluetooth.c @@ -44,6 +44,8 @@ static GSList *adapter_list = NULL; static gint bluetooth_refcount; static GSList *server_list = NULL; +#define TIMEOUT (60*1000) /* Timeout for user response (milliseconds) */ + struct adapter_address { char *adapter; char *address; @@ -63,6 +65,7 @@ struct cb_data { struct server *server; char *path; guint source; + GIOChannel *io; }; void bluetooth_create_path(const char *dev_addr, const char *adapter_addr, @@ -266,6 +269,14 @@ static gint adapter_compare(gconstpointer a, gconstpointer b) return g_strcmp0(entry->adapter, key); } +static gint address_compare(gconstpointer a, gconstpointer b) +{ + const struct adapter_address *entry = a; + const char *key = b; + + return g_strcmp0(entry->address, key); +} + static GSList* adapter_address_add(GSList *list, const char *path, const char *address) { @@ -507,13 +518,102 @@ static gboolean client_event(GIOChannel *chan, GIOCondition cond, gpointer data) return FALSE; } +static void cancel_authorization(struct cb_data *user_data) +{ + DBusMessage *msg; + + if (user_data->path == NULL) + return; + + msg = dbus_message_new_method_call(BLUEZ_SERVICE, user_data->path, + BLUEZ_SERVICE_INTERFACE, + "CancelAuthorization"); + + g_dbus_send_message(connection, msg); +} + +static void auth_cb(DBusPendingCall *call, gpointer user_data) +{ + struct cb_data *cb_data = user_data; + struct server *server = cb_data->server; + + DBusMessage *reply = dbus_pending_call_steal_reply(call); + DBusError derr; + GError *err = NULL; + + dbus_error_init(&derr); + + if (dbus_set_error_from_message(&derr, reply)) { + ofono_error("RequestAuthorization error: %s, %s", + derr.name, derr.message); + + if (dbus_error_has_name(&derr, DBUS_ERROR_NO_REPLY)) + cancel_authorization(cb_data); + + dbus_error_free(&derr); + + dbus_message_unref(reply); + + goto failed; + } + + dbus_message_unref(reply); + + ofono_info("RequestAuthorization succeeded"); + + if (!bt_io_accept(cb_data->io, server->connect_cb, server->user_data, + NULL, &err)) { + ofono_error("%s", err->message); + g_error_free(err); + goto failed; + } + + g_source_remove(cb_data->source); + server->client_list = g_slist_remove(server->client_list, + (void *) cb_data->source); + + cb_data->source = g_io_add_watch(cb_data->io, + G_IO_HUP | G_IO_ERR | G_IO_NVAL, + client_event, cb_data); + server->client_list = g_slist_prepend(server->client_list, + (void *)cb_data->source); + + return; + +failed: + g_source_remove(cb_data->source); + server->client_list = g_slist_remove(server->client_list, + (void *) cb_data->source); + + cb_data_destroy(cb_data); +} + +static gboolean auth_watch(GIOChannel *io, GIOCondition cond, + gpointer user_data) +{ + struct cb_data *cb_data = user_data; + struct server *server = cb_data->server; + + cancel_authorization(cb_data); + server->client_list = g_slist_remove(server->client_list, + (void *) cb_data->source); + + cb_data_destroy(cb_data); + + return FALSE; +} + static void confirm_event(GIOChannel *io, gpointer user_data) { struct server *server = user_data; struct cb_data *client_data; + guint handle; + const char *addr; + int ret; GError *err = NULL; char laddress[18], raddress[18]; guint8 channel; + GSList *entry; bt_io_get(io, BT_IO_RFCOMM, &err, BT_IO_OPT_SOURCE, laddress, BT_IO_OPT_DEST, raddress, @@ -528,14 +628,6 @@ static void confirm_event(GIOChannel *io, gpointer user_data) ofono_info("New connection for %s on channel %u from: %s,", laddress, channel, raddress); - if (!bt_io_accept(io, server->connect_cb, server->user_data, - NULL, &err)) { - ofono_error("%s", err->message); - g_error_free(err); - g_io_channel_unref(io); - return; - } - client_data = g_try_new0(struct cb_data, 1); if (client_data == NULL) { ofono_error("Unable to allocate client cb_data structure"); @@ -543,9 +635,35 @@ static void confirm_event(GIOChannel *io, gpointer user_data) } client_data->server = server; + entry = g_slist_find_custom(adapter_list, laddress, address_compare); + if (entry != NULL) { + struct adapter_address *a = entry->data; + + client_data->path = g_strdup(a->adapter); + } + + client_data->io = io; + + handle = (guint) g_hash_table_lookup(server->adapter_hash, + client_data->path); + addr = raddress; + ret = bluetooth_send_with_reply(client_data->path, + BLUEZ_SERVICE_INTERFACE, + "RequestAuthorization", + auth_cb, client_data, NULL, TIMEOUT, + DBUS_TYPE_STRING, &addr, + DBUS_TYPE_UINT32, &handle, + DBUS_TYPE_INVALID); + if (ret < 0) { + ofono_error("Request Bluetooth authorization failed"); + return; + } + + ofono_info("RequestAuthorization(%s, 0x%x)", raddress, handle); + client_data->source = g_io_add_watch(io, G_IO_HUP | G_IO_ERR | G_IO_NVAL, - client_event, client_data); + auth_watch, client_data); server->client_list = g_slist_prepend(server->client_list, (void *)client_data->source); } -- 1.7.1

Marcel Holtmann

2:12 a.m.

New subject: [PATCH 1/4] bluetooth: only use bluetooth_refcount in bluetooth_ref/bluetooth_unref

Hi Fred,

...

plugins/bluetooth.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-) diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c index 93dd7a1..e59bd31 100644 --- a/plugins/bluetooth.c +++ b/plugins/bluetooth.c @@ -505,7 +505,7 @@ static guint adapter_added_watch; static guint adapter_removed_watch; static guint property_watch; -static void bluetooth_ref(void) +static int bluetooth_ref(void) {

why are we now reverting something back that we already applied upstream. Something is wrong here. Regards Marcel

Marcel Holtmann

2:13 a.m.

New subject: [PATCH 2/4] bluetooth: use GSList instead of GHashTable to store adapter path/address

Hi Fred,

...

plugins/bluetooth.c | 86 +++++++++++++++++++++++++++++++++++++++++++------- 1 files changed, 74 insertions(+), 12 deletions(-) diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c index e59bd31..4da662a 100644 --- a/plugins/bluetooth.c +++ b/plugins/bluetooth.c @@ -39,9 +39,14 @@ static DBusConnection *connection; static GHashTable *uuid_hash = NULL; -static GHashTable *adapter_address_hash = NULL; +static GSList *adapter_list = NULL; static gint bluetooth_refcount; +struct adapter_address { + char *adapter; + char *address; +}; +

why are we doing this exactly with a list now instead of a hash table. I don't see the point here. Regards Marcel

Frederic Danis

2:22 a.m.

New subject: [PATCH 2/4] bluetooth: use GSList instead of GHashTable to store adapter path/address

Le 27/01/2011 16:13, Marcel Holtmann a écrit :

...

Hi Fred, > plugins/bluetooth.c | 86 +++++++++++++++++++++++++++++++++++++++++++------- > 1 files changed, 74 insertions(+), 12 deletions(-) > > diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c > index e59bd31..4da662a 100644 > --- a/plugins/bluetooth.c > +++ b/plugins/bluetooth.c > @@ -39,9 +39,14 @@ > > static DBusConnection *connection; > static GHashTable *uuid_hash = NULL; > -static GHashTable *adapter_address_hash = NULL; > +static GSList *adapter_list = NULL; > static gint bluetooth_refcount; > > +struct adapter_address { > + char *adapter; > + char *address; > +}; > + why are we doing this exactly with a list now instead of a hash table. I don't see the point here.

In patch 4, I need to find the adapter path (for the incoming connection) from its address to call the authorization method. So, this is why I change to GSList. -- Frederic Danis Open Source Technology Centre frederic.danis(a)intel.com Intel Corporation

Marcel Holtmann

2:24 a.m.

New subject: [PATCH 2/4] bluetooth: use GSList instead of GHashTable to store adapter path/address

Hi Fred,

...

>> plugins/bluetooth.c | 86 +++++++++++++++++++++++++++++++++++++++++++------- >> 1 files changed, 74 insertions(+), 12 deletions(-) >> >> diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c >> index e59bd31..4da662a 100644 >> --- a/plugins/bluetooth.c >> +++ b/plugins/bluetooth.c >> @@ -39,9 +39,14 @@ >> >> static DBusConnection *connection; >> static GHashTable *uuid_hash = NULL; >> -static GHashTable *adapter_address_hash = NULL; >> +static GSList *adapter_list = NULL; >> static gint bluetooth_refcount; >> >> +struct adapter_address { >> + char *adapter; >> + char *address; >> +}; >> + > > why are we doing this exactly with a list now instead of a hash table. I > don't see the point here. > In patch 4, I need to find the adapter path (for the incoming connection) from its address to call the authorization method. So, this is why I change to GSList.

you can as easily walk a GHashTable than you can walk a GSList. So why not do that instead of moving everything to a list. Regards Marcel

Frederic Danis

2:25 a.m.

New subject: [PATCH 1/4] bluetooth: only use bluetooth_refcount in bluetooth_ref/bluetooth_unref

Le 27/01/2011 16:12, Marcel Holtmann a écrit :

...

Hi Fred, > plugins/bluetooth.c | 14 +++++++++----- > 1 files changed, 9 insertions(+), 5 deletions(-) > > diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c > index 93dd7a1..e59bd31 100644 > --- a/plugins/bluetooth.c > +++ b/plugins/bluetooth.c > @@ -505,7 +505,7 @@ static guint adapter_added_watch; > static guint adapter_removed_watch; > static guint property_watch; > > -static void bluetooth_ref(void) > +static int bluetooth_ref(void) > { why are we now reverting something back that we already applied upstream. Something is wrong here.

I followed Padovan's comment on my first try for this patch : "bluetooth_refcount can only be read/write by bluetooth_ref/unref" which seems good to me -- Frederic Danis Open Source Technology Centre frederic.danis(a)intel.com Intel Corporation

Frederic Danis

2:35 a.m.

New subject: [PATCH 2/4] bluetooth: use GSList instead of GHashTable to store adapter path/address

Le 27/01/2011 16:24, Marcel Holtmann a écrit :

...

Hi Fred, >>> plugins/bluetooth.c | 86 +++++++++++++++++++++++++++++++++++++++++++------- >>> 1 files changed, 74 insertions(+), 12 deletions(-) >>> >>> diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c >>> index e59bd31..4da662a 100644 >>> --- a/plugins/bluetooth.c >>> +++ b/plugins/bluetooth.c >>> @@ -39,9 +39,14 @@ >>> >>> static DBusConnection *connection; >>> static GHashTable *uuid_hash = NULL; >>> -static GHashTable *adapter_address_hash = NULL; >>> +static GSList *adapter_list = NULL; >>> static gint bluetooth_refcount; >>> >>> +struct adapter_address { >>> + char *adapter; >>> + char *address; >>> +}; >>> + >> >> why are we doing this exactly with a list now instead of a hash table. I >> don't see the point here. >> > > In patch 4, I need to find the adapter path (for the incoming > connection) from its address to call the authorization method. > > So, this is why I change to GSList. you can as easily walk a GHashTable than you can walk a GSList. So why not do that instead of moving everything to a list.

I think that find a key from its value may use more cpu from HashTable than from GSList. If you want I can remove this and only use HashTable. -- Frederic Danis Open Source Technology Centre frederic.danis(a)intel.com Intel Corporation

Gustavo F. Padovan

3:17 a.m.

New subject: [PATCH 1/4] bluetooth: only use bluetooth_refcount in bluetooth_ref/bluetooth_unref

* Frederic Danis <frederic.danis(a)linux.intel.com> [2011-01-27 16:25:16 +0100]:

...

Le 27/01/2011 16:12, Marcel Holtmann a écrit : > Hi Fred, > >> plugins/bluetooth.c | 14 +++++++++----- >> 1 files changed, 9 insertions(+), 5 deletions(-) >> >> diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c >> index 93dd7a1..e59bd31 100644 >> --- a/plugins/bluetooth.c >> +++ b/plugins/bluetooth.c >> @@ -505,7 +505,7 @@ static guint adapter_added_watch; >> static guint adapter_removed_watch; >> static guint property_watch; >> >> -static void bluetooth_ref(void) >> +static int bluetooth_ref(void) >> { > > why are we now reverting something back that we already applied > upstream. Something is wrong here. I followed Padovan's comment on my first try for this patch : "bluetooth_refcount can only be read/write by bluetooth_ref/unref" which seems good to me

Yeah, but I'm now wondering why we merged a code access bluetooth_refcount directly. -- Gustavo F. Padovan http://profusion.mobi

Marcel Holtmann

3:18 a.m.

New subject: [PATCH 1/4] bluetooth: only use bluetooth_refcount in bluetooth_ref/bluetooth_unref

Hi Gustavo,

...

> >> plugins/bluetooth.c | 14 +++++++++----- > >> 1 files changed, 9 insertions(+), 5 deletions(-) > >> > >> diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c > >> index 93dd7a1..e59bd31 100644 > >> --- a/plugins/bluetooth.c > >> +++ b/plugins/bluetooth.c > >> @@ -505,7 +505,7 @@ static guint adapter_added_watch; > >> static guint adapter_removed_watch; > >> static guint property_watch; > >> > >> -static void bluetooth_ref(void) > >> +static int bluetooth_ref(void) > >> { > > > > why are we now reverting something back that we already applied > > upstream. Something is wrong here. > > I followed Padovan's comment on my first try for this patch : > "bluetooth_refcount can only be read/write by bluetooth_ref/unref" > which seems good to me Yeah, but I'm now wondering why we merged a code access bluetooth_refcount directly.

that part is just fine. Don't worry about that. Regards Marcel

Gustavo F. Padovan

3:19 a.m.

New subject: [PATCH 2/4] bluetooth: use GSList instead of GHashTable to store adapter path/address

Hi Frederic, * Frederic Danis <frederic.danis(a)linux.intel.com> [2011-01-27 16:35:49 +0100]:

...

Le 27/01/2011 16:24, Marcel Holtmann a écrit : > Hi Fred, > >>>> plugins/bluetooth.c | 86 +++++++++++++++++++++++++++++++++++++++++++------- >>>> 1 files changed, 74 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c >>>> index e59bd31..4da662a 100644 >>>> --- a/plugins/bluetooth.c >>>> +++ b/plugins/bluetooth.c >>>> @@ -39,9 +39,14 @@ >>>> >>>> static DBusConnection *connection; >>>> static GHashTable *uuid_hash = NULL; >>>> -static GHashTable *adapter_address_hash = NULL; >>>> +static GSList *adapter_list = NULL; >>>> static gint bluetooth_refcount; >>>> >>>> +struct adapter_address { >>>> + char *adapter; >>>> + char *address; >>>> +}; >>>> + >>> >>> why are we doing this exactly with a list now instead of a hash table. I >>> don't see the point here. >>> >> >> In patch 4, I need to find the adapter path (for the incoming >> connection) from its address to call the authorization method. >> >> So, this is why I change to GSList. > > you can as easily walk a GHashTable than you can walk a GSList. So why > not do that instead of moving everything to a list. I think that find a key from its value may use more cpu from HashTable than from GSList. If you want I can remove this and only use HashTable.

Please remove it and use the hashtable. -- Gustavo F. Padovan http://profusion.mobi

Gustavo F. Padovan

3:26 a.m.

New subject: [PATCH 3/4] bluetooth: Add bluetooth server support

Hi Frédéric, * Frédéric Danis <frederic.danis(a)linux.intel.com> [2011-01-27 16:05:39 +0100]:

...

What's going on here? You are duplicating many line of codes that we already have in bluetooth.c. Can't you just adapt the code we have there? -- Gustavo F. Padovan http://profusion.mobi

Frederic Danis

3:32 a.m.

New subject: [PATCH 3/4] bluetooth: Add bluetooth server support

Le 27/01/2011 17:26, Gustavo F. Padovan a écrit :

...

Hi Frédéric, * Frédéric Danis<frederic.danis(a)linux.intel.com> [2011-01-27 16:05:39 +0100]: > --- > Makefile.am | 1 + > plugins/bluetooth.c | 340 +++++++++++++++++++++++++++++++++++++++++++++++++++ > plugins/bluetooth.h | 9 ++ > 3 files changed, 350 insertions(+), 0 deletions(-) > > diff --git a/Makefile.am b/Makefile.am > index 9b77e63..c0efe28 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -318,6 +318,7 @@ builtin_sources += plugins/bluetooth.c plugins/bluetooth.h > builtin_modules += hfp > builtin_sources += plugins/hfp.c plugins/bluetooth.h > > +builtin_sources += $(btio_sources) > builtin_cflags += @BLUEZ_CFLAGS@ > builtin_libadd += @BLUEZ_LIBS@ > endif > diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c > index 4da662a..86d4c25 100644 > --- a/plugins/bluetooth.c > +++ b/plugins/bluetooth.c > @@ -35,18 +35,36 @@ > > #include<ofono/dbus.h> > > +#include<btio.h> > #include "bluetooth.h" > > static DBusConnection *connection; > static GHashTable *uuid_hash = NULL; > static GSList *adapter_list = NULL; > static gint bluetooth_refcount; > +static GSList *server_list = NULL; > > struct adapter_address { > char *adapter; > char *address; > }; > > +struct server { > + guint8 channel; > + char *sdp_record; > + GIOChannel *io; > + GHashTable *adapter_hash; > + ConnectFunc connect_cb; > + gpointer user_data; > + GSList *client_list; > +}; > + > +struct cb_data { > + struct server *server; > + char *path; > + guint source; > +}; > + > void bluetooth_create_path(const char *dev_addr, const char *adapter_addr, > char *buf, int size) > { > @@ -429,6 +447,277 @@ static gboolean property_changed(DBusConnection *connection, DBusMessage *msg, > return TRUE; > } > > +static void remove_record(char *path, guint handle, struct server *server) > +{ > + DBusMessage *msg; > + > + msg = dbus_message_new_method_call(BLUEZ_SERVICE, path, > + BLUEZ_SERVICE_INTERFACE, > + "RemoveRecord"); > + if (msg == NULL) { > + ofono_error("Unable to allocate D-Bus RemoveRecord message"); > + return; > + } > + > + dbus_message_append_args(msg, DBUS_TYPE_UINT32,&handle, > + DBUS_TYPE_INVALID); > + g_dbus_send_message(connection, msg); > + > + ofono_info("Unregistered handle for %s, channel %d: 0x%x", path, > + server->channel, handle); > +} > + > +static void server_stop(struct server *server) > +{ > + while (server->client_list) { > + g_source_remove((guint) server->client_list->data); > + server->client_list = g_slist_remove(server->client_list, > + server->client_list->data); > + } > + > + g_hash_table_foreach_remove(server->adapter_hash, > + (GHRFunc) remove_record, server); > + > + if (server->io != NULL) { > + g_io_channel_shutdown(server->io, TRUE, NULL); > + g_io_channel_unref(server->io); > + server->io = NULL; > + } > +} > + > +static void cb_data_destroy(gpointer data) > +{ > + struct cb_data *cb_data = data; > + > + if (cb_data->path != NULL) > + g_free(cb_data->path); > + g_free(cb_data); > +} > + > +static gboolean client_event(GIOChannel *chan, GIOCondition cond, gpointer data) > +{ > + struct cb_data *cb_data = data; > + struct server *server = cb_data->server; > + > + server->client_list = g_slist_remove(server->client_list, > + (void *) cb_data->source); > + > + cb_data_destroy(cb_data); > + > + return FALSE; > +} > + > +static void confirm_event(GIOChannel *io, gpointer user_data) > +{ > + struct server *server = user_data; > + struct cb_data *client_data; > + GError *err = NULL; > + char laddress[18], raddress[18]; > + guint8 channel; > + > + bt_io_get(io, BT_IO_RFCOMM,&err, BT_IO_OPT_SOURCE, laddress, > + BT_IO_OPT_DEST, raddress, > + BT_IO_OPT_CHANNEL,&channel, > + BT_IO_OPT_INVALID); > + if (err) { > + ofono_error("%s", err->message); > + g_error_free(err); > + return; > + } > + > + ofono_info("New connection for %s on channel %u from: %s,", laddress, > + channel, raddress); > + > + if (!bt_io_accept(io, server->connect_cb, server->user_data, > + NULL,&err)) { > + ofono_error("%s", err->message); > + g_error_free(err); > + g_io_channel_unref(io); > + return; > + } > + > + client_data = g_try_new0(struct cb_data, 1); > + if (client_data == NULL) { > + ofono_error("Unable to allocate client cb_data structure"); > + return; > + } > + > + client_data->server = server; > + client_data->source = g_io_add_watch(io, > + G_IO_HUP | G_IO_ERR | G_IO_NVAL, > + client_event, client_data); > + server->client_list = g_slist_prepend(server->client_list, > + (void *)client_data->source); > +} > + > +static void add_record_cb(DBusPendingCall *call, gpointer user_data) > +{ > + struct cb_data *cb_data = user_data; > + DBusMessage *reply = dbus_pending_call_steal_reply(call); > + DBusError derr; > + guint32 handle; > + > + dbus_error_init(&derr); > + > + if (dbus_set_error_from_message(&derr, reply)) { > + ofono_error("Replied with an error: %s, %s", > + derr.name, derr.message); > + dbus_error_free(&derr); > + g_free(cb_data->path); > + goto done; > + } > + > + dbus_message_get_args(reply, NULL, DBUS_TYPE_UINT32,&handle, > + DBUS_TYPE_INVALID); > + > + g_hash_table_insert(cb_data->server->adapter_hash, cb_data->path, > + (void *) handle); > + > + ofono_info("Registered handle for %s, channel %d: 0x%x", cb_data->path, > + cb_data->server->channel, handle); > + > +done: > + /* Do not free cb_data->path, it is used in adapter_hash */ > + g_free(cb_data); > + dbus_message_unref(reply); > +} > + > +static void server_adapter_properties_cb(DBusPendingCall *call, > + gpointer user_data) > +{ > + struct cb_data *cb_data = user_data; > + const char *path = cb_data->path; > + DBusMessage *reply; > + const char *addr; > + struct server *server = cb_data->server; > + > + reply = dbus_pending_call_steal_reply(call); > + > + if (dbus_message_is_error(reply, DBUS_ERROR_SERVICE_UNKNOWN)) { > + DBG("Bluetooth daemon is apparently not available."); > + goto done; > + } > + > + bluetooth_parse_properties(reply, "Address", parse_string,&addr, NULL); > + > + DBG("Adapter Address: %s, Path: %s", addr, path); > + adapter_list = adapter_address_add(adapter_list, path, addr); > + > + DBG("Add record on %s", path); > + > + bluetooth_send_with_reply(path, BLUEZ_SERVICE_INTERFACE, > + "AddRecord", add_record_cb, > + cb_data, NULL, -1, > + DBUS_TYPE_STRING,&server->sdp_record, > + DBUS_TYPE_INVALID); > + > +done: > + dbus_message_unref(reply); > +} > + > +static void server_parse_adapters(DBusMessageIter *array, gpointer user_data) > +{ > + struct server *server = user_data; > + DBusMessageIter value; > + struct cb_data *cb_data; > + > + if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY) > + return; > + > + dbus_message_iter_recurse(array,&value); > + > + while (dbus_message_iter_get_arg_type(&value) > + == DBUS_TYPE_OBJECT_PATH) { > + const char *path; > + > + dbus_message_iter_get_basic(&value,&path); > + > + DBG("Calling GetProperties on %s", path); > + > + cb_data = g_try_new0(struct cb_data, 1); > + if (cb_data == NULL) { > + ofono_error("Unable to allocate cb_data structure"); > + return; > + } > + > + cb_data->server = server; > + cb_data->path = g_strdup(path); > + > + bluetooth_send_with_reply(path, BLUEZ_ADAPTER_INTERFACE, > + "GetProperties", server_adapter_properties_cb, > + cb_data, NULL, -1, DBUS_TYPE_INVALID); > + > + dbus_message_iter_next(&value); > + } > +} > + > +static void server_manager_properties_cb(DBusPendingCall *call, > + gpointer user_data) > +{ > + DBusMessage *reply; > + > + reply = dbus_pending_call_steal_reply(call); > + > + if (dbus_message_is_error(reply, DBUS_ERROR_SERVICE_UNKNOWN)) { > + DBG("Bluetooth daemon is apparently not available."); > + goto done; > + } > + > + bluetooth_parse_properties(reply, "Adapters", server_parse_adapters, > + user_data, NULL); > + > +done: > + dbus_message_unref(reply); > +} What's going on here? You are duplicating many line of codes that we already have in bluetooth.c. Can't you just adapt the code we have there?

Most of the code is same, but the end of callbacks do not finish in same way. If this code is not duplicate, this will complicate the server and/or client code to manage that it is called form the other path. With this code the client and server path are isolated. -- Frederic Danis Open Source Technology Centre frederic.danis(a)intel.com Intel Corporation

4252

days inactive

4252

days old

ofono@ofono.org

Manage subscription

15 comments

4 participants

tags (0)

participants (4)

Frederic Danis
Frédéric Danis
Gustavo F. Padovan
Marcel Holtmann

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

[PATCH v2 0/4] bluetooth: Add bluetooth server support