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

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

[=?unknown-8bit?q?Fr=C3=A9d=C3=A9ric?= Danis]

[-- Attachment #1: Type: text/plain, Size: 816 bytes --]

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(-)

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

[=?unknown-8bit?q?Fr=C3=A9d=C3=A9ric?= Danis]

[-- Attachment #1: Type: text/plain, Size: 1298 bytes --]

---
 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

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

[=?unknown-8bit?q?Fr=C3=A9d=C3=A9ric?= Danis]

[-- Attachment #1: Type: text/plain, Size: 4530 bytes --]

---
 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

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

[=?unknown-8bit?q?Fr=C3=A9d=C3=A9ric?= Danis]

[-- Attachment #1: Type: text/plain, Size: 11922 bytes --]

---
 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

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

[=?unknown-8bit?q?Fr=C3=A9d=C3=A9ric?= Danis]

[-- Attachment #1: Type: text/plain, Size: 5496 bytes --]

---
 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

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

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

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

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

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 751 bytes --]

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

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

[Frederic Danis]

[-- Attachment #1: Type: text/plain, Size: 1132 bytes --]

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

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

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1133 bytes --]

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

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

[Frederic Danis]

[-- Attachment #1: Type: text/plain, Size: 966 bytes --]

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

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

[Frederic Danis]

[-- Attachment #1: Type: text/plain, Size: 1477 bytes --]

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

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

[Gustavo F. Padovan]

[-- Attachment #1: Type: text/plain, Size: 1078 bytes --]

* Frederic Danis <frederic.danis@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

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

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1017 bytes --]

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

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

[Gustavo F. Padovan]

[-- Attachment #1: Type: text/plain, Size: 1587 bytes --]

Hi Frederic,

* Frederic Danis <frederic.danis@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

Re: [PATCH 3/4] bluetooth: Add bluetooth server support

[Gustavo F. Padovan]

[-- Attachment #1: Type: text/plain, Size: 8464 bytes --]

Hi Frédéric,

* Frédéric Danis <frederic.danis@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?

-- 
Gustavo F. Padovan
http://profusion.mobi

Re: [PATCH 3/4] bluetooth: Add bluetooth server support

[Frederic Danis]

[-- Attachment #1: Type: text/plain, Size: 9194 bytes --]

Le 27/01/2011 17:26, Gustavo F. Padovan a écrit :
> Hi Frédéric,
>
> * Frédéric Danis<frederic.danis@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