Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH bluez] bnep: don't error() if kernel lacks bnep module
From: David Herrmann @ 2013-09-13 11:07 UTC (permalink / raw)
  To: David Herrmann, linux-bluetooth@vger.kernel.org, Marcel Holtmann
In-Reply-To: <20130913105816.GA15267@x220.p-661hnu-f1>

Hi

On Fri, Sep 13, 2013 at 12:58 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi David,
>
> On Fri, Sep 13, 2013, David Herrmann wrote:
>> On Fri, Sep 13, 2013 at 12:01 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> > Hi David,
>> >
>> > On Fri, Sep 06, 2013, David Herrmann wrote:
>> >> If a user disables bnep kernel support, they normally do that on purpose.
>> >> It's misleading to print an error during module-startup in this situation.
>> >> Therefore, only print a hint that bnep-support is missing if
>> >> EPROTONOSUPPORT is returned by the kernel.
>> >>
>> >> Furthermore, allow modules to forward ENOSYS as error to bluetoothd core
>> >> to handle it as "module is not compatible with system setup" instead of an
>> >> error. ENOSYS is commonly used to signal "missing kernel infrastructure"
>> >> so it seems appropriate here.
>> >> ---
>> >>  profiles/network/common.c  |  7 ++++++-
>> >>  profiles/network/manager.c | 17 ++++++++++++++---
>> >>  src/plugin.c               | 11 +++++++++--
>> >>  3 files changed, 29 insertions(+), 6 deletions(-)
>> >
>> > First of all, sorry for the delay with reviewing this. I forgot about it
>> > and only bumped into it now when checking what I'd missed from the last
>> > month in my linux-bluetooth folder.
>> >
>> >> diff --git a/profiles/network/common.c b/profiles/network/common.c
>> >> index e069892..17bff30 100644
>> >> --- a/profiles/network/common.c
>> >> +++ b/profiles/network/common.c
>> >> @@ -110,8 +110,13 @@ int bnep_init(void)
>> >>
>> >>       if (ctl < 0) {
>> >>               int err = -errno;
>> >> -             error("Failed to open control socket: %s (%d)",
>> >> +
>> >> +             if (err == -EPROTONOSUPPORT)
>> >> +                     info("kernel lacks bnep-protocol support");
>> >
>> > I think warn() might be more appropriate here.
>>
>> Using warn() defeats the whole purpose of this patch. I want to run
>> bluetoothd without getting a warning or error, and obviously, there is
>> nothing to warn _me_ about. I'm ok if you want err() or warn() here so
>> other users get warned, but in that case we should just drop the
>> patch.
>
> I thought the main idea was to have a clearer log message when the
> module is not there. None of the warn/error/info messages can be
> suppressed (unlike DBG which is off by default). To me warn() seems more
> appropriate than info() since it's meaning is "this is something that
> may or may not be of concern to the user".

It's not about suppressing messages. It's about putting proper
attributes on them. If I look at my system-log, I am usually only
interested in errors and warnings. And each of them needs my attention
to get fixed. Otherwise, the purpose of an error or warning is missed.
If every running daemon spits warnings to the log which I cannot
"fix", warnings will no longer be any special. Same for errors,
obviously.

So imho every warning should be "fixable" by the administrator. And in
this case I don't think enabling BNEP in the kernel is the appropriate
fix. Instead, if an administrator disables BNEP, bluetoothd should
interpret that as a "configuration-choice".

>> So it's up to you to decide. If info() is ok, I will fix the issues
>> you mentioned below, otherwise just drop it.
>
> I do think the patch has value even though info() is not used by having
> an understandable log message for what's going on.

In that case I will resend it. Please let me know whether to use
info() or warn().

Thanks
David

^ permalink raw reply

* [PATCH v2 0/3] Bluetooth: Various L2CAP fixes
From: johan.hedberg @ 2013-09-13 11:29 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

This one contains a fix for the locking bug spotted by Lizardo in patch
3/3 as well as the minor coding style issue in the same patch.

Johan

----------------------------------------------------------------
Johan Hedberg (3):
      Bluetooth: Fix L2CAP Disconnect response for unknown CID
      Bluetooth: Fix responding to invalid L2CAP signaling commands
      Bluetooth: Fix waiting for clearing of BT_SK_SUSPEND flag

 include/net/bluetooth/bluetooth.h |  1 +
 net/bluetooth/af_bluetooth.c      | 39 +++++++++++++++++++++++++++++++++++++
 net/bluetooth/l2cap_core.c        | 10 +++++++++-
 net/bluetooth/l2cap_sock.c        |  6 ++++++
 net/bluetooth/rfcomm/sock.c       |  9 +++++++--
 5 files changed, 62 insertions(+), 3 deletions(-)


^ permalink raw reply

* [PATCH v2 1/3] Bluetooth: Fix L2CAP Disconnect response for unknown CID
From: johan.hedberg @ 2013-09-13 11:29 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1379071767-16604-1-git-send-email-johan.hedberg@gmail.com>

From: Johan Hedberg <johan.hedberg@intel.com>

If we receive an L2CAP Disconnect Request for an unknown CID we should
not just silently drop it but reply with a proper Command Reject
response. This patch fixes this by ensuring that the disconnect handler
returns a proper error instead of 0 and will cause the function caller
to send the right response.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
v2: no changes

 net/bluetooth/l2cap_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index b3bb7bc..ea3792f 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4206,7 +4206,7 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn,
 	chan = __l2cap_get_chan_by_scid(conn, dcid);
 	if (!chan) {
 		mutex_unlock(&conn->chan_lock);
-		return 0;
+		return -EINVAL;
 	}
 
 	l2cap_chan_lock(chan);
-- 
1.8.4.rc3


^ permalink raw reply related

* [PATCH v2 2/3] Bluetooth: Fix responding to invalid L2CAP signaling commands
From: johan.hedberg @ 2013-09-13 11:29 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1379071767-16604-1-git-send-email-johan.hedberg@gmail.com>

From: Johan Hedberg <johan.hedberg@intel.com>

When we have an LE link we should not respond to any data on the BR/EDR
L2CAP signaling channel (0x0001) and vice-versa when we have a BR/EDR
link we should not respond to LE L2CAP (CID 0x0005) signaling commands.
This patch fixes this issue by checking for a valid link type and
ignores data if it is wrong.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
v2: no changes

 net/bluetooth/l2cap_core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index ea3792f..1d03644 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5297,6 +5297,7 @@ static inline int l2cap_le_sig_cmd(struct l2cap_conn *conn,
 static inline void l2cap_le_sig_channel(struct l2cap_conn *conn,
 					struct sk_buff *skb)
 {
+	struct hci_conn *hcon = conn->hcon;
 	u8 *data = skb->data;
 	int len = skb->len;
 	struct l2cap_cmd_hdr cmd;
@@ -5304,6 +5305,9 @@ static inline void l2cap_le_sig_channel(struct l2cap_conn *conn,
 
 	l2cap_raw_recv(conn, skb);
 
+	if (hcon->type != LE_LINK)
+		return;
+
 	while (len >= L2CAP_CMD_HDR_SIZE) {
 		u16 cmd_len;
 		memcpy(&cmd, data, L2CAP_CMD_HDR_SIZE);
@@ -5342,6 +5346,7 @@ static inline void l2cap_le_sig_channel(struct l2cap_conn *conn,
 static inline void l2cap_sig_channel(struct l2cap_conn *conn,
 				     struct sk_buff *skb)
 {
+	struct hci_conn *hcon = conn->hcon;
 	u8 *data = skb->data;
 	int len = skb->len;
 	struct l2cap_cmd_hdr cmd;
@@ -5349,6 +5354,9 @@ static inline void l2cap_sig_channel(struct l2cap_conn *conn,
 
 	l2cap_raw_recv(conn, skb);
 
+	if (hcon->type != ACL_LINK)
+		return;
+
 	while (len >= L2CAP_CMD_HDR_SIZE) {
 		u16 cmd_len;
 		memcpy(&cmd, data, L2CAP_CMD_HDR_SIZE);
-- 
1.8.4.rc3


^ permalink raw reply related

* [PATCH v2 3/3] Bluetooth: Fix waiting for clearing of BT_SK_SUSPEND flag
From: johan.hedberg @ 2013-09-13 11:29 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1379071767-16604-1-git-send-email-johan.hedberg@gmail.com>

From: Johan Hedberg <johan.hedberg@intel.com>

In the case of blocking sockets we should not proceed with sendmsg() if
the socket has the BT_SK_SUSPEND flag set. So far the code was only
ensuring that POLLOUT doesn't get set for non-blocking sockets using
poll() but there was no code in place to ensure that blocking sockets do
the right thing when writing to them.

This patch adds a new bt_sock_wait_unsuspend helper function to sleep in
the sendmsg call if the BT_SK_SUSPEND flag is set, and wake up as soon
as it is unset. It also updates the L2CAP and RFCOMM sendmsg callbacks
to take advantage of this new helper function.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
v2: Fix missing release_sock() in rfcomm_sock_sendmsg()

 include/net/bluetooth/bluetooth.h |  1 +
 net/bluetooth/af_bluetooth.c      | 39 +++++++++++++++++++++++++++++++++++++++
 net/bluetooth/l2cap_sock.c        |  6 ++++++
 net/bluetooth/rfcomm/sock.c       |  9 +++++++--
 4 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 10d43d8..3299d42 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -249,6 +249,7 @@ int  bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
 uint bt_sock_poll(struct file *file, struct socket *sock, poll_table *wait);
 int  bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
 int  bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
+int  bt_sock_wait_unsuspend(struct sock *sk, unsigned long flags);
 
 void bt_accept_enqueue(struct sock *parent, struct sock *sk);
 void bt_accept_unlink(struct sock *sk);
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 9096137..6081e78 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -525,6 +525,45 @@ int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
 }
 EXPORT_SYMBOL(bt_sock_wait_state);
 
+int bt_sock_wait_unsuspend(struct sock *sk, unsigned long flags)
+{
+	DECLARE_WAITQUEUE(wait, current);
+	unsigned long timeo;
+	int err = 0;
+
+	BT_DBG("sk %p", sk);
+
+	timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
+
+	add_wait_queue(sk_sleep(sk), &wait);
+	set_current_state(TASK_INTERRUPTIBLE);
+	while (test_bit(BT_SK_SUSPEND, &bt_sk(sk)->flags)) {
+		if (!timeo) {
+			err = -EAGAIN;
+			break;
+		}
+
+		if (signal_pending(current)) {
+			err = sock_intr_errno(timeo);
+			break;
+		}
+
+		release_sock(sk);
+		timeo = schedule_timeout(timeo);
+		lock_sock(sk);
+		set_current_state(TASK_INTERRUPTIBLE);
+
+		err = sock_error(sk);
+		if (err)
+			break;
+	}
+	__set_current_state(TASK_RUNNING);
+	remove_wait_queue(sk_sleep(sk), &wait);
+
+	return err;
+}
+EXPORT_SYMBOL(bt_sock_wait_unsuspend);
+
 #ifdef CONFIG_PROC_FS
 struct bt_seq_state {
 	struct bt_sock_list *l;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 0098af8..1ddbc1a 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -777,6 +777,12 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 	if (sk->sk_state != BT_CONNECTED)
 		return -ENOTCONN;
 
+	lock_sock(sk);
+	err = bt_sock_wait_unsuspend(sk, msg->msg_flags);
+	release_sock(sk);
+	if (err)
+		return err;
+
 	l2cap_chan_lock(chan);
 	err = l2cap_chan_send(chan, msg, len, sk->sk_priority);
 	l2cap_chan_unlock(chan);
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 30b3721..7a48cfb 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -544,7 +544,7 @@ static int rfcomm_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 	struct sock *sk = sock->sk;
 	struct rfcomm_dlc *d = rfcomm_pi(sk)->dlc;
 	struct sk_buff *skb;
-	int sent = 0;
+	int err, sent = 0;
 
 	if (test_bit(RFCOMM_DEFER_SETUP, &d->flags))
 		return -ENOTCONN;
@@ -559,9 +559,14 @@ static int rfcomm_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 
 	lock_sock(sk);
 
+	err = bt_sock_wait_unsuspend(sk, msg->msg_flags);
+	if (err) {
+		release_sock(sk);
+		return err;
+	}
+
 	while (len) {
 		size_t size = min_t(size_t, len, d->mtu);
-		int err;
 
 		skb = sock_alloc_send_skb(sk, size + RFCOMM_SKB_RESERVE,
 				msg->msg_flags & MSG_DONTWAIT, &err);
-- 
1.8.4.rc3


^ permalink raw reply related

* [PATCH 1/8] gitignore: Add tools/btinfo
From: Szymon Janc @ 2013-09-13 11:30 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index d1c31f4..8a25a3e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -62,6 +62,7 @@ tools/mpris-player
 tools/bluetooth-player
 tools/l2cap-tester
 tools/sco-tester
+tools/btinfo
 test/sap_client.pyc
 test/bluezutils.pyc
 unit/test-eir
-- 
1.8.4


^ permalink raw reply related

* [PATCH 2/8] core: Minor whitespace fix
From: Szymon Janc @ 2013-09-13 11:30 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1379071852-10094-1-git-send-email-szymon.janc@tieto.com>

---
 src/main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/main.c b/src/main.c
index dc0478e..eafe2ed 100644
--- a/src/main.c
+++ b/src/main.c
@@ -282,10 +282,10 @@ static void init_defaults(void)
 	if (sscanf(VERSION, "%hhu.%hhu", &major, &minor) != 2)
 		return;
 
-        main_opts.did_source = 0x0002;		/* USB */
-        main_opts.did_vendor = 0x1d6b;		/* Linux Foundation */
-        main_opts.did_product = 0x0246;		/* BlueZ */
-        main_opts.did_version = (major << 8 | minor);
+	main_opts.did_source = 0x0002;		/* USB */
+	main_opts.did_vendor = 0x1d6b;		/* Linux Foundation */
+	main_opts.did_product = 0x0246;		/* BlueZ */
+	main_opts.did_version = (major << 8 | minor);
 }
 
 static GMainLoop *event_loop;
-- 
1.8.4


^ permalink raw reply related

* [PATCH 3/8] sap: Keep reference to btd_adapter in struct sap_server
From: Szymon Janc @ 2013-09-13 11:30 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1379071852-10094-1-git-send-email-szymon.janc@tieto.com>

Path and adapter address can be obtained easily from btd_adapter
and there is no need to keep local copy.
---
 profiles/sap/manager.c |  6 ++----
 profiles/sap/server.c  | 23 +++++++++++++----------
 profiles/sap/server.h  |  2 +-
 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/profiles/sap/manager.c b/profiles/sap/manager.c
index 24b73e7..bfb81a5 100644
--- a/profiles/sap/manager.c
+++ b/profiles/sap/manager.c
@@ -35,11 +35,9 @@
 
 static int sap_server_probe(struct btd_profile *p, struct btd_adapter *adapter)
 {
-	const char *path = adapter_get_path(adapter);
-
-	DBG("path %s", path);
+	DBG("path %s", adapter_get_path(adapter));
 
-	return sap_server_register(path, adapter_get_address(adapter));
+	return sap_server_register(adapter);
 }
 
 static void sap_server_remove(struct btd_profile *p,
diff --git a/profiles/sap/server.c b/profiles/sap/server.c
index dfcf0f1..1aacfe9 100644
--- a/profiles/sap/server.c
+++ b/profiles/sap/server.c
@@ -73,7 +73,7 @@ struct sap_connection {
 };
 
 struct sap_server {
-	char *path;
+	struct btd_adapter *adapter;
 	uint32_t record_id;
 	GIOChannel *listen_io;
 	struct sap_connection *conn;
@@ -620,7 +620,8 @@ static void sap_set_connected(struct sap_server *server)
 {
 	server->conn->state = SAP_STATE_CONNECTED;
 
-	g_dbus_emit_property_changed(btd_get_dbus_connection(), server->path,
+	g_dbus_emit_property_changed(btd_get_dbus_connection(),
+					adapter_get_path(server->adapter),
 					SAP_SERVER_INTERFACE, "Connected");
 }
 
@@ -1144,7 +1145,8 @@ static void sap_io_destroy(void *data)
 	if (conn->state != SAP_STATE_CONNECT_IN_PROGRESS &&
 				conn->state != SAP_STATE_CONNECT_MODEM_BUSY)
 		g_dbus_emit_property_changed(btd_get_dbus_connection(),
-					server->path, SAP_SERVER_INTERFACE,
+					adapter_get_path(server->adapter),
+					SAP_SERVER_INTERFACE,
 					"Connected");
 
 	if (conn->state == SAP_STATE_CONNECT_IN_PROGRESS ||
@@ -1326,7 +1328,7 @@ static void server_remove(struct sap_server *server)
 		server->listen_io = NULL;
 	}
 
-	g_free(server->path);
+	btd_adapter_unref(server->adapter);
 	g_free(server);
 }
 
@@ -1335,12 +1337,12 @@ static void destroy_sap_interface(void *data)
 	struct sap_server *server = data;
 
 	DBG("Unregistered interface %s on path %s", SAP_SERVER_INTERFACE,
-								server->path);
+					adapter_get_path(server->adapter));
 
 	server_remove(server);
 }
 
-int sap_server_register(const char *path, const bdaddr_t *src)
+int sap_server_register(struct btd_adapter *adapter)
 {
 	sdp_record_t *record = NULL;
 	GError *gerr = NULL;
@@ -1358,19 +1360,19 @@ int sap_server_register(const char *path, const bdaddr_t *src)
 		goto sdp_err;
 	}
 
-	if (add_record_to_server(src, record) < 0) {
+	if (add_record_to_server(adapter_get_address(adapter), record) < 0) {
 		error("Adding SAP SDP record to the SDP server failed.");
 		sdp_record_free(record);
 		goto sdp_err;
 	}
 
 	server = g_new0(struct sap_server, 1);
-	server->path = g_strdup(path);
+	server->adapter = btd_adapter_ref(adapter);
 	server->record_id = record->handle;
 
 	io = bt_io_listen(NULL, connect_confirm_cb, server,
 			NULL, &gerr,
-			BT_IO_OPT_SOURCE_BDADDR, src,
+			BT_IO_OPT_SOURCE_BDADDR, adapter_get_address(adapter),
 			BT_IO_OPT_CHANNEL, SAP_SERVER_CHANNEL,
 			BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_HIGH,
 			BT_IO_OPT_MASTER, TRUE,
@@ -1383,7 +1385,8 @@ int sap_server_register(const char *path, const bdaddr_t *src)
 	server->listen_io = io;
 
 	if (!g_dbus_register_interface(btd_get_dbus_connection(),
-					server->path, SAP_SERVER_INTERFACE,
+					adapter_get_path(server->adapter),
+					SAP_SERVER_INTERFACE,
 					server_methods, NULL,
 					server_properties, server,
 					destroy_sap_interface)) {
diff --git a/profiles/sap/server.h b/profiles/sap/server.h
index 73b38ab..d7e674a 100644
--- a/profiles/sap/server.h
+++ b/profiles/sap/server.h
@@ -20,5 +20,5 @@
 
 #include <gdbus/gdbus.h>
 
-int sap_server_register(const char *path, const bdaddr_t *src);
+int sap_server_register(struct btd_adapter *adapter);
 void sap_server_unregister(const char *path);
-- 
1.8.4


^ permalink raw reply related

* [PATCH 4/8] sdp: Decouple Device ID profile implementation
From: Szymon Janc @ 2013-09-13 11:30 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1379071852-10094-1-git-send-email-szymon.janc@tieto.com>

Make DeviceID profile similar to other profiles implementations. Use
btd_profile for handling DeviceID profile while adding/removing
adapters. The nice drawback is that SDP code no longer depends on
main_opts.
---
 Makefile.plugins             |   3 +
 profiles/deviceid/deviceid.c | 181 +++++++++++++++++++++++++++++++++++++++++++
 src/sdpd-server.c            |   4 -
 src/sdpd-service.c           |  58 --------------
 src/sdpd.h                   |   2 -
 5 files changed, 184 insertions(+), 64 deletions(-)
 create mode 100644 profiles/deviceid/deviceid.c

diff --git a/Makefile.plugins b/Makefile.plugins
index 7c5f71d..df5d2a1 100644
--- a/Makefile.plugins
+++ b/Makefile.plugins
@@ -82,6 +82,9 @@ builtin_sources += profiles/scanparam/scan.c
 builtin_modules += deviceinfo
 builtin_sources += profiles/deviceinfo/deviceinfo.c
 
+builtin_modules += deviceid
+builtin_sources += profiles/deviceid/deviceid.c
+
 if EXPERIMENTAL
 builtin_modules += alert
 builtin_sources += profiles/alert/server.c
diff --git a/profiles/deviceid/deviceid.c b/profiles/deviceid/deviceid.c
new file mode 100644
index 0000000..e5fc35a
--- /dev/null
+++ b/profiles/deviceid/deviceid.c
@@ -0,0 +1,181 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2013  Intel Corporation
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <bluetooth/sdp.h>
+#include <bluetooth/sdp_lib.h>
+
+#include <glib.h>
+
+#include "sdpd.h"
+#include "hcid.h"
+#include "adapter.h"
+#include "profile.h"
+#include "plugin.h"
+#include "log.h"
+
+struct deviceid_adapter {
+	struct btd_adapter *adapter;
+	uint32_t handle;
+};
+
+static GSList *deviceid_adapters = NULL;
+
+static struct deviceid_adapter *find_adapter(struct btd_adapter *adapter)
+{
+	GSList *list;
+
+	for (list = deviceid_adapters; list; list = list->next) {
+		struct deviceid_adapter *da = list->data;
+
+		if (da->adapter == adapter)
+			return da;
+	}
+
+	return NULL;
+}
+
+static sdp_record_t *create_record(uint16_t source, uint16_t vendor,
+					uint16_t product, uint16_t version)
+{
+	const uint16_t spec = 0x0103;
+	const uint8_t primary = 1;
+	sdp_list_t *class_list, *group_list, *profile_list;
+	uuid_t class_uuid, group_uuid;
+	sdp_data_t *sdp_data, *primary_data, *source_data;
+	sdp_data_t *spec_data, *vendor_data, *product_data, *version_data;
+	sdp_profile_desc_t profile;
+	sdp_record_t *record = sdp_record_alloc();
+
+	DBG("%04x:%04x:%04x:%04x", source, vendor, product, version);
+
+	record->handle = sdp_next_handle();
+
+	sdp_data = sdp_data_alloc(SDP_UINT32, &record->handle);
+	sdp_attr_add(record, SDP_ATTR_RECORD_HANDLE, sdp_data);
+
+	sdp_uuid16_create(&class_uuid, PNP_INFO_SVCLASS_ID);
+	class_list = sdp_list_append(0, &class_uuid);
+	sdp_set_service_classes(record, class_list);
+	sdp_list_free(class_list, NULL);
+
+	sdp_uuid16_create(&group_uuid, PUBLIC_BROWSE_GROUP);
+	group_list = sdp_list_append(NULL, &group_uuid);
+	sdp_set_browse_groups(record, group_list);
+	sdp_list_free(group_list, NULL);
+
+	sdp_uuid16_create(&profile.uuid, PNP_INFO_PROFILE_ID);
+	profile.version = spec;
+	profile_list = sdp_list_append(NULL, &profile);
+	sdp_set_profile_descs(record, profile_list);
+	sdp_list_free(profile_list, NULL);
+
+	spec_data = sdp_data_alloc(SDP_UINT16, &spec);
+	sdp_attr_add(record, 0x0200, spec_data);
+
+	vendor_data = sdp_data_alloc(SDP_UINT16, &vendor);
+	sdp_attr_add(record, 0x0201, vendor_data);
+
+	product_data = sdp_data_alloc(SDP_UINT16, &product);
+	sdp_attr_add(record, 0x0202, product_data);
+
+	version_data = sdp_data_alloc(SDP_UINT16, &version);
+	sdp_attr_add(record, 0x0203, version_data);
+
+	primary_data = sdp_data_alloc(SDP_BOOL, &primary);
+	sdp_attr_add(record, 0x0204, primary_data);
+
+	source_data = sdp_data_alloc(SDP_UINT16, &source);
+	sdp_attr_add(record, 0x0205, source_data);
+
+	return record;
+}
+
+static int deviceid_adapter_probe(struct btd_profile *p,
+						struct btd_adapter *adapter)
+{
+	struct deviceid_adapter *dadapter;
+	sdp_record_t *rec;
+	int ret;
+
+	DBG("path %s", adapter_get_path(adapter));
+
+	rec = create_record(main_opts.did_source, main_opts.did_vendor,
+				main_opts.did_product, main_opts.did_version);
+
+	ret = add_record_to_server(adapter_get_address(adapter), rec);
+	if (ret < 0) {
+		sdp_record_free(rec);
+		return ret;
+	}
+
+	dadapter = g_new0(struct deviceid_adapter, 1);
+	dadapter->adapter = adapter;
+	dadapter->handle = rec->handle;
+
+	deviceid_adapters = g_slist_prepend(deviceid_adapters, dadapter);
+
+	return 0;
+}
+
+static void deviceid_adapter_remove(struct btd_profile *p,
+						struct btd_adapter *adapter)
+{
+	struct deviceid_adapter *dadapter;
+
+	DBG("path %s", adapter_get_path(adapter));
+
+	dadapter = find_adapter(adapter);
+	if (!dadapter)
+		return;
+
+	remove_record_from_server(dadapter->handle);
+}
+
+struct btd_profile deviceid_profile = {
+	.name		= "deviceid",
+	.adapter_probe	= deviceid_adapter_probe,
+	.adapter_remove	= deviceid_adapter_remove,
+};
+
+static int deviceid_init(void)
+{
+	if (main_opts.did_source == 0) {
+		info("Device ID information disabled");
+		return -1;
+	}
+
+	return btd_profile_register(&deviceid_profile);
+}
+
+static void deviceid_exit(void)
+{
+	btd_profile_unregister(&deviceid_profile);
+}
+
+BLUETOOTH_PLUGIN_DEFINE(deviceid, VERSION,
+			BLUETOOTH_PLUGIN_PRIORITY_DEFAULT,
+			deviceid_init, deviceid_exit)
diff --git a/src/sdpd-server.c b/src/sdpd-server.c
index 181d248..8267e11 100644
--- a/src/sdpd-server.c
+++ b/src/sdpd-server.c
@@ -238,10 +238,6 @@ int start_sdp_server(uint16_t mtu, uint32_t flags)
 		return -1;
 	}
 
-	if (main_opts.did_source > 0)
-		register_device_id(main_opts.did_source, main_opts.did_vendor,
-				main_opts.did_product, main_opts.did_version);
-
 	io = g_io_channel_unix_new(l2cap_sock);
 	g_io_channel_set_close_on_unref(io, TRUE);
 
diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index 38bf808..c4d3a02 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
@@ -176,64 +176,6 @@ void register_server_service(void)
 	update_db_timestamp();
 }
 
-void register_device_id(uint16_t source, uint16_t vendor,
-					uint16_t product, uint16_t version)
-{
-	const uint16_t spec = 0x0103;
-	const uint8_t primary = 1;
-	sdp_list_t *class_list, *group_list, *profile_list;
-	uuid_t class_uuid, group_uuid;
-	sdp_data_t *sdp_data, *primary_data, *source_data;
-	sdp_data_t *spec_data, *vendor_data, *product_data, *version_data;
-	sdp_profile_desc_t profile;
-	sdp_record_t *record = sdp_record_alloc();
-
-	DBG("Adding device id record for %04x:%04x:%04x:%04x",
-					source, vendor, product, version);
-
-	record->handle = sdp_next_handle();
-
-	sdp_record_add(BDADDR_ANY, record);
-	sdp_data = sdp_data_alloc(SDP_UINT32, &record->handle);
-	sdp_attr_add(record, SDP_ATTR_RECORD_HANDLE, sdp_data);
-
-	sdp_uuid16_create(&class_uuid, PNP_INFO_SVCLASS_ID);
-	class_list = sdp_list_append(0, &class_uuid);
-	sdp_set_service_classes(record, class_list);
-	sdp_list_free(class_list, NULL);
-
-	sdp_uuid16_create(&group_uuid, PUBLIC_BROWSE_GROUP);
-	group_list = sdp_list_append(NULL, &group_uuid);
-	sdp_set_browse_groups(record, group_list);
-	sdp_list_free(group_list, NULL);
-
-	sdp_uuid16_create(&profile.uuid, PNP_INFO_PROFILE_ID);
-	profile.version = spec;
-	profile_list = sdp_list_append(NULL, &profile);
-	sdp_set_profile_descs(record, profile_list);
-	sdp_list_free(profile_list, NULL);
-
-	spec_data = sdp_data_alloc(SDP_UINT16, &spec);
-	sdp_attr_add(record, 0x0200, spec_data);
-
-	vendor_data = sdp_data_alloc(SDP_UINT16, &vendor);
-	sdp_attr_add(record, 0x0201, vendor_data);
-
-	product_data = sdp_data_alloc(SDP_UINT16, &product);
-	sdp_attr_add(record, 0x0202, product_data);
-
-	version_data = sdp_data_alloc(SDP_UINT16, &version);
-	sdp_attr_add(record, 0x0203, version_data);
-
-	primary_data = sdp_data_alloc(SDP_BOOL, &primary);
-	sdp_attr_add(record, 0x0204, primary_data);
-
-	source_data = sdp_data_alloc(SDP_UINT16, &source);
-	sdp_attr_add(record, 0x0205, source_data);
-
-	update_db_timestamp();
-}
-
 int add_record_to_server(const bdaddr_t *src, sdp_record_t *rec)
 {
 	sdp_data_t *data;
diff --git a/src/sdpd.h b/src/sdpd.h
index 9a0e1e9..28d7f6d 100644
--- a/src/sdpd.h
+++ b/src/sdpd.h
@@ -56,8 +56,6 @@ int service_remove_req(sdp_req_t *req, sdp_buf_t *rsp);
 
 void register_public_browse_group(void);
 void register_server_service(void);
-void register_device_id(uint16_t source, uint16_t vendor,
-					uint16_t product, uint16_t version);
 
 int record_sort(const void *r1, const void *r2);
 void sdp_svcdb_reset(void);
-- 
1.8.4


^ permalink raw reply related

* [PATCH 5/8] adapter: Handle adding new SDP records
From: Szymon Janc @ 2013-09-13 11:30 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1379071852-10094-1-git-send-email-szymon.janc@tieto.com>

Make adapter in charge of updating SDP database. This allow to decouple
SDP of code used for notifying adapters about SDP database change.
---
 profiles/audio/a2dp.c        |  3 +--
 profiles/audio/avrcp.c       |  4 ++--
 profiles/deviceid/deviceid.c |  2 +-
 profiles/health/hdp_util.c   |  3 +--
 profiles/network/server.c    |  2 +-
 profiles/sap/server.c        |  2 +-
 src/adapter.c                | 15 +++++++++++----
 src/adapter.h                |  3 ++-
 src/attrib-server.c          |  3 +--
 src/profile.c                |  7 +++----
 src/sdpd-database.c          | 12 ------------
 11 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 3f3cc1b..d96a8b5 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -1298,8 +1298,7 @@ struct a2dp_sep *a2dp_add_sep(struct btd_adapter *adapter, uint8_t type,
 		return NULL;
 	}
 
-	if (add_record_to_server(adapter_get_address(server->adapter),
-								record) < 0) {
+	if (adapter_service_add(server->adapter, record) < 0) {
 		error("Unable to register A2DP service record");
 		sdp_record_free(record);
 		avdtp_unregister_sep(sep->lsep);
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 9f164e4..e75e804 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -3829,7 +3829,7 @@ done:
 		return -1;
 	}
 
-	if (add_record_to_server(adapter_get_address(adapter), record) < 0) {
+	if (adapter_service_add(adapter, record) < 0) {
 		error("Unable to register AVRCP target service record");
 		avrcp_target_server_remove(p, adapter);
 		sdp_record_free(record);
@@ -3912,7 +3912,7 @@ done:
 		return -1;
 	}
 
-	if (add_record_to_server(adapter_get_address(adapter), record) < 0) {
+	if (adapter_service_add(adapter, record) < 0) {
 		error("Unable to register AVRCP service record");
 		avrcp_controller_server_remove(p, adapter);
 		sdp_record_free(record);
diff --git a/profiles/deviceid/deviceid.c b/profiles/deviceid/deviceid.c
index e5fc35a..8eb51c8 100644
--- a/profiles/deviceid/deviceid.c
+++ b/profiles/deviceid/deviceid.c
@@ -126,7 +126,7 @@ static int deviceid_adapter_probe(struct btd_profile *p,
 	rec = create_record(main_opts.did_source, main_opts.did_vendor,
 				main_opts.did_product, main_opts.did_version);
 
-	ret = add_record_to_server(adapter_get_address(adapter), rec);
+	ret = adapter_service_add(adapter, rec);
 	if (ret < 0) {
 		sdp_record_free(rec);
 		return ret;
diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
index b53f1db..7748a90 100644
--- a/profiles/health/hdp_util.c
+++ b/profiles/health/hdp_util.c
@@ -733,8 +733,7 @@ gboolean hdp_update_sdp_record(struct hdp_adapter *adapter, GSList *app_list)
 	if (sdp_set_record_state(sdp_record, adapter->record_state++) < 0)
 		goto fail;
 
-	if (add_record_to_server(adapter_get_address(adapter->btd_adapter),
-					sdp_record) < 0)
+	if (adapter_service_add(adapter->btd_adapter, sdp_record) < 0)
 		goto fail;
 	adapter->sdp_handler = sdp_record->handle;
 	return TRUE;
diff --git a/profiles/network/server.c b/profiles/network/server.c
index 043e1fc..d537531 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -587,7 +587,7 @@ static uint32_t register_server_record(struct network_server *ns)
 		return 0;
 	}
 
-	if (add_record_to_server(&ns->src, record) < 0) {
+	if (adapter_service_add(ns->na->adapter, record) < 0) {
 		error("Failed to register service record");
 		sdp_record_free(record);
 		return 0;
diff --git a/profiles/sap/server.c b/profiles/sap/server.c
index 1aacfe9..089bc7a 100644
--- a/profiles/sap/server.c
+++ b/profiles/sap/server.c
@@ -1360,7 +1360,7 @@ int sap_server_register(struct btd_adapter *adapter)
 		goto sdp_err;
 	}
 
-	if (add_record_to_server(adapter_get_address(adapter), record) < 0) {
+	if (adapter_service_add(adapter, record) < 0) {
 		error("Adding SAP SDP record to the SDP server failed.");
 		sdp_record_free(record);
 		goto sdp_err;
diff --git a/src/adapter.c b/src/adapter.c
index 17f5508..8fd42d9 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -934,18 +934,24 @@ static int uuid_cmp(const void *a, const void *b)
 	return sdp_uuid_cmp(&rec->svclass, uuid);
 }
 
-void adapter_service_insert(struct btd_adapter *adapter, void *r)
+int adapter_service_add(struct btd_adapter *adapter, sdp_record_t *rec)
 {
-	sdp_record_t *rec = r;
 	sdp_list_t *browse_list = NULL;
 	uuid_t browse_uuid;
 	gboolean new_uuid;
+	int ret;
 
 	DBG("%s", adapter->path);
 
+	ret = add_record_to_server(&adapter->bdaddr, rec);
+	if (ret < 0)
+		return ret;
+
 	/* skip record without a browse group */
-	if (sdp_get_browse_groups(rec, &browse_list) < 0)
-		return;
+	if (sdp_get_browse_groups(rec, &browse_list) < 0) {
+		DBG("skipping record without browse group");
+		return 0;
+	}
 
 	sdp_uuid16_create(&browse_uuid, PUBLIC_BROWSE_GROUP);
 
@@ -968,6 +974,7 @@ void adapter_service_insert(struct btd_adapter *adapter, void *r)
 
 done:
 	sdp_list_free(browse_list, free);
+	return 0;
 }
 
 void adapter_service_remove(struct btd_adapter *adapter, void *r)
diff --git a/src/adapter.h b/src/adapter.h
index 32b12c0..ef6d4ed 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -102,7 +102,8 @@ struct btd_device *adapter_find_device(struct btd_adapter *adapter,
 const char *adapter_get_path(struct btd_adapter *adapter);
 const bdaddr_t *adapter_get_address(struct btd_adapter *adapter);
 int adapter_set_name(struct btd_adapter *adapter, const char *name);
-void adapter_service_insert(struct btd_adapter *adapter, void *rec);
+
+int adapter_service_add(struct btd_adapter *adapter, sdp_record_t *rec);
 void adapter_service_remove(struct btd_adapter *adapter, void *rec);
 
 struct agent *adapter_get_agent(struct btd_adapter *adapter);
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 3f629b0..5a76d2e 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -326,8 +326,7 @@ static uint32_t attrib_create_sdp_new(struct gatt_server *server,
 				"http://www.bluez.org/");
 	}
 
-	if (add_record_to_server(adapter_get_address(server->adapter), record)
-			== 0)
+	if (adapter_service_add(server->adapter, record) == 0)
 		return record->handle;
 
 	sdp_record_free(record);
diff --git a/src/profile.c b/src/profile.c
index 523e119..f2c1e6f 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -1179,7 +1179,7 @@ static void ext_direct_connect(GIOChannel *io, GError *err, gpointer user_data)
 static uint32_t ext_register_record(struct ext_profile *ext,
 							struct ext_io *l2cap,
 							struct ext_io *rfcomm,
-							const bdaddr_t *src)
+							struct btd_adapter *a)
 {
 	sdp_record_t *rec;
 	char *dyn_record = NULL;
@@ -1202,7 +1202,7 @@ static uint32_t ext_register_record(struct ext_profile *ext,
 		return 0;
 	}
 
-	if (add_record_to_server(src, rec) < 0) {
+	if (adapter_service_add(a, rec) < 0) {
 		error("Failed to register service record");
 		sdp_record_free(rec);
 		return 0;
@@ -1304,8 +1304,7 @@ static uint32_t ext_start_servers(struct ext_profile *ext,
 		}
 	}
 
-	return ext_register_record(ext, l2cap, rfcomm,
-						adapter_get_address(adapter));
+	return ext_register_record(ext, l2cap, rfcomm, adapter);
 
 failed:
 	if (l2cap) {
diff --git a/src/sdpd-database.c b/src/sdpd-database.c
index cf33f19..600ddbf 100644
--- a/src/sdpd-database.c
+++ b/src/sdpd-database.c
@@ -168,7 +168,6 @@ void sdp_svcdb_set_collectable(sdp_record_t *record, int sock)
  */
 void sdp_record_add(const bdaddr_t *device, sdp_record_t *rec)
 {
-	struct btd_adapter *adapter;
 	sdp_access_t *dev;
 
 	SDPDBG("Adding rec : 0x%lx", (long) rec);
@@ -184,15 +183,6 @@ void sdp_record_add(const bdaddr_t *device, sdp_record_t *rec)
 	dev->handle = rec->handle;
 
 	access_db = sdp_list_insert_sorted(access_db, dev, access_sort);
-
-	if (bacmp(device, BDADDR_ANY) == 0) {
-		adapter_foreach(adapter_service_insert, rec);
-		return;
-	}
-
-	adapter = adapter_find(device);
-	if (adapter)
-		adapter_service_insert(adapter, rec);
 }
 
 static sdp_list_t *record_locate(uint32_t handle)
@@ -333,7 +323,5 @@ void sdp_init_services_list(bdaddr_t *device)
 			continue;
 
 		SDPDBG("adding record with handle %x", access->handle);
-
-		adapter_foreach(adapter_service_insert, rec);
 	}
 }
-- 
1.8.4


^ permalink raw reply related

* [PATCH 6/8] adapter: Handle removing of SDP records
From: Szymon Janc @ 2013-09-13 11:30 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1379071852-10094-1-git-send-email-szymon.janc@tieto.com>

Make adapter in charge of updating SDP database. This allow to decouple
SDP of code used for notifying adapters about SDP database change.
---
 plugins/gatt-example.c       |  2 +-
 profiles/audio/a2dp.c        | 11 +++++++----
 profiles/audio/avrcp.c       |  4 ++--
 profiles/deviceid/deviceid.c |  2 +-
 profiles/health/hdp.c        |  2 +-
 profiles/health/hdp_util.c   |  3 ++-
 profiles/network/server.c    |  4 ++--
 profiles/sap/server.c        |  2 +-
 src/adapter.c                | 13 ++++++++-----
 src/adapter.h                |  2 +-
 src/attrib-server.c          |  9 +++++----
 src/attrib-server.h          |  2 +-
 src/profile.c                |  2 +-
 src/sdpd-database.c          |  8 --------
 14 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/plugins/gatt-example.c b/plugins/gatt-example.c
index b926947..9b4187a 100644
--- a/plugins/gatt-example.c
+++ b/plugins/gatt-example.c
@@ -72,7 +72,7 @@ static void gatt_example_adapter_free(struct gatt_example_adapter *gadapter)
 	while (gadapter->sdp_handles != NULL) {
 		uint32_t handle = GPOINTER_TO_UINT(gadapter->sdp_handles->data);
 
-		attrib_free_sdp(handle);
+		attrib_free_sdp(gadapter->adapter, handle);
 		gadapter->sdp_handles = g_slist_remove(gadapter->sdp_handles,
 						gadapter->sdp_handles->data);
 	}
diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index d96a8b5..8477b5d 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -1326,7 +1326,8 @@ void a2dp_remove_sep(struct a2dp_sep *sep)
 			return;
 		server->sources = g_slist_remove(server->sources, sep);
 		if (server->sources == NULL && server->source_record_id) {
-			remove_record_from_server(server->source_record_id);
+			adapter_service_remove(server->adapter,
+						server->source_record_id);
 			server->source_record_id = 0;
 		}
 	} else {
@@ -1334,7 +1335,8 @@ void a2dp_remove_sep(struct a2dp_sep *sep)
 			return;
 		server->sinks = g_slist_remove(server->sinks, sep);
 		if (server->sinks == NULL && server->sink_record_id) {
-			remove_record_from_server(server->sink_record_id);
+			adapter_service_remove(server->adapter,
+						server->sink_record_id);
 			server->sink_record_id = 0;
 		}
 	}
@@ -1943,7 +1945,8 @@ static void a2dp_source_server_remove(struct btd_profile *p,
 					(GDestroyNotify) a2dp_unregister_sep);
 
 	if (server->source_record_id) {
-		remove_record_from_server(server->source_record_id);
+		adapter_service_remove(server->adapter,
+					server->source_record_id);
 		server->source_record_id = 0;
 	}
 
@@ -1988,7 +1991,7 @@ static void a2dp_sink_server_remove(struct btd_profile *p,
 	g_slist_free_full(server->sinks, (GDestroyNotify) a2dp_unregister_sep);
 
 	if (server->sink_record_id) {
-		remove_record_from_server(server->sink_record_id);
+		adapter_service_remove(server->adapter, server->sink_record_id);
 		server->sink_record_id = 0;
 	}
 
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index e75e804..b1b2ae6 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -3797,7 +3797,7 @@ static void avrcp_target_server_remove(struct btd_profile *p,
 		return;
 
 	if (server->tg_record_id != 0) {
-		remove_record_from_server(server->tg_record_id);
+		adapter_service_remove(adapter, server->tg_record_id);
 		server->tg_record_id = 0;
 	}
 
@@ -3880,7 +3880,7 @@ static void avrcp_controller_server_remove(struct btd_profile *p,
 		return;
 
 	if (server->ct_record_id != 0) {
-		remove_record_from_server(server->ct_record_id);
+		adapter_service_remove(adapter, server->ct_record_id);
 		server->ct_record_id = 0;
 	}
 
diff --git a/profiles/deviceid/deviceid.c b/profiles/deviceid/deviceid.c
index 8eb51c8..27b218b 100644
--- a/profiles/deviceid/deviceid.c
+++ b/profiles/deviceid/deviceid.c
@@ -152,7 +152,7 @@ static void deviceid_adapter_remove(struct btd_profile *p,
 	if (!dadapter)
 		return;
 
-	remove_record_from_server(dadapter->handle);
+	adapter_service_remove(adapter, dadapter->handle);
 }
 
 struct btd_profile deviceid_profile = {
diff --git a/profiles/health/hdp.c b/profiles/health/hdp.c
index 7f24756..7b4e799 100644
--- a/profiles/health/hdp.c
+++ b/profiles/health/hdp.c
@@ -1403,7 +1403,7 @@ void hdp_adapter_unregister(struct btd_adapter *adapter)
 	hdp_adapter = l->data;
 	adapters = g_slist_remove(adapters, hdp_adapter);
 	if (hdp_adapter->sdp_handler > 0)
-		remove_record_from_server(hdp_adapter->sdp_handler);
+		adapter_service_remove(adapter, hdp_adapter->sdp_handler);
 	release_adapter_instance(hdp_adapter);
 	btd_adapter_unref(hdp_adapter->btd_adapter);
 	g_free(hdp_adapter);
diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
index 7748a90..34e4671 100644
--- a/profiles/health/hdp_util.c
+++ b/profiles/health/hdp_util.c
@@ -693,7 +693,8 @@ gboolean hdp_update_sdp_record(struct hdp_adapter *adapter, GSList *app_list)
 	sdp_record_t *sdp_record;
 
 	if (adapter->sdp_handler > 0)
-		remove_record_from_server(adapter->sdp_handler);
+		adapter_service_remove(adapter->btd_adapter,
+					adapter->sdp_handler);
 
 	if (app_list == NULL) {
 		adapter->sdp_handler = 0;
diff --git a/profiles/network/server.c b/profiles/network/server.c
index d537531..7b784e5 100644
--- a/profiles/network/server.c
+++ b/profiles/network/server.c
@@ -628,7 +628,7 @@ static void server_disconnect(DBusConnection *conn, void *user_data)
 	ns->watch_id = 0;
 
 	if (ns->record_id) {
-		remove_record_from_server(ns->record_id);
+		adapter_service_remove(ns->na->adapter, ns->record_id);
 		ns->record_id = 0;
 	}
 
@@ -722,7 +722,7 @@ static void server_free(void *data)
 	server_remove_sessions(ns);
 
 	if (ns->record_id)
-		remove_record_from_server(ns->record_id);
+		adapter_service_remove(ns->na->adapter, ns->record_id);
 
 	g_dbus_remove_watch(btd_get_dbus_connection(), ns->watch_id);
 	g_free(ns->name);
diff --git a/profiles/sap/server.c b/profiles/sap/server.c
index 089bc7a..63314a7 100644
--- a/profiles/sap/server.c
+++ b/profiles/sap/server.c
@@ -1320,7 +1320,7 @@ static void server_remove(struct sap_server *server)
 
 	sap_server_remove_conn(server);
 
-	remove_record_from_server(server->record_id);
+	adapter_service_remove(server->adapter, server->record_id);
 
 	if (server->listen_io) {
 		g_io_channel_shutdown(server->listen_io, TRUE, NULL);
diff --git a/src/adapter.c b/src/adapter.c
index 8fd42d9..0ec1381 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -977,18 +977,21 @@ done:
 	return 0;
 }
 
-void adapter_service_remove(struct btd_adapter *adapter, void *r)
+void adapter_service_remove(struct btd_adapter *adapter, uint32_t handle)
 {
-	sdp_record_t *rec = r;
+	sdp_record_t *rec = sdp_record_find(handle);
 
 	DBG("%s", adapter->path);
 
+	if (!rec)
+		return;
+
 	adapter->services = sdp_list_remove(adapter->services, rec);
 
-	if (sdp_list_find(adapter->services, &rec->svclass, uuid_cmp))
-		return;
+	if (sdp_list_find(adapter->services, &rec->svclass, uuid_cmp) == NULL)
+		remove_uuid(adapter, &rec->svclass);
 
-	remove_uuid(adapter, &rec->svclass);
+	remove_record_from_server(rec->handle);
 }
 
 static struct btd_device *adapter_create_device(struct btd_adapter *adapter,
diff --git a/src/adapter.h b/src/adapter.h
index ef6d4ed..5d124e7 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -104,7 +104,7 @@ const bdaddr_t *adapter_get_address(struct btd_adapter *adapter);
 int adapter_set_name(struct btd_adapter *adapter, const char *name);
 
 int adapter_service_add(struct btd_adapter *adapter, sdp_record_t *rec);
-void adapter_service_remove(struct btd_adapter *adapter, void *rec);
+void adapter_service_remove(struct btd_adapter *adapter, uint32_t handle);
 
 struct agent *adapter_get_agent(struct btd_adapter *adapter);
 
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 5a76d2e..2861a00 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -139,10 +139,11 @@ static void gatt_server_free(struct gatt_server *server)
 	g_slist_free_full(server->clients, (GDestroyNotify) channel_free);
 
 	if (server->gatt_sdp_handle > 0)
-		remove_record_from_server(server->gatt_sdp_handle);
+		adapter_service_remove(server->adapter,
+					server->gatt_sdp_handle);
 
 	if (server->gap_sdp_handle > 0)
-		remove_record_from_server(server->gap_sdp_handle);
+		adapter_service_remove(server->adapter, server->gap_sdp_handle);
 
 	if (server->adapter != NULL)
 		btd_adapter_unref(server->adapter);
@@ -1377,9 +1378,9 @@ uint32_t attrib_create_sdp(struct btd_adapter *adapter, uint16_t handle,
 	return attrib_create_sdp_new(l->data, handle, name);
 }
 
-void attrib_free_sdp(uint32_t sdp_handle)
+void attrib_free_sdp(struct btd_adapter *adapter, uint32_t sdp_handle)
 {
-	remove_record_from_server(sdp_handle);
+	adapter_service_remove(adapter, sdp_handle);
 }
 
 static uint16_t find_uuid16_avail(struct btd_adapter *adapter, uint16_t nitems)
diff --git a/src/attrib-server.h b/src/attrib-server.h
index 2148017..90ba17c 100644
--- a/src/attrib-server.h
+++ b/src/attrib-server.h
@@ -36,6 +36,6 @@ int attrib_gap_set(struct btd_adapter *adapter, uint16_t uuid,
 					const uint8_t *value, size_t len);
 uint32_t attrib_create_sdp(struct btd_adapter *adapter, uint16_t handle,
 							const char *name);
-void attrib_free_sdp(uint32_t sdp_handle);
+void attrib_free_sdp(struct btd_adapter *adapter, uint32_t sdp_handle);
 guint attrib_channel_attach(GAttrib *attrib);
 gboolean attrib_channel_detach(GAttrib *attrib, guint id);
diff --git a/src/profile.c b/src/profile.c
index f2c1e6f..accd007 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -1371,7 +1371,7 @@ static void ext_remove_records(struct ext_profile *ext,
 
 		ext->records = g_slist_remove(ext->records, r);
 
-		remove_record_from_server(r->handle);
+		adapter_service_remove(adapter, r->handle);
 		btd_adapter_unref(r->adapter);
 		g_free(r);
 	}
diff --git a/src/sdpd-database.c b/src/sdpd-database.c
index 600ddbf..e4d4f98 100644
--- a/src/sdpd-database.c
+++ b/src/sdpd-database.c
@@ -36,7 +36,6 @@
 
 #include "sdpd.h"
 #include "log.h"
-#include "adapter.h"
 
 static sdp_list_t *service_db;
 static sdp_list_t *access_db;
@@ -254,13 +253,6 @@ int sdp_record_remove(uint32_t handle)
 
 	a = p->data;
 
-	if (bacmp(&a->device, BDADDR_ANY) != 0) {
-		struct btd_adapter *adapter = adapter_find(&a->device);
-		if (adapter)
-			adapter_service_remove(adapter, r);
-	} else
-		adapter_foreach(adapter_service_remove, r);
-
 	access_db = sdp_list_remove(access_db, a);
 	access_free(a);
 
-- 
1.8.4


^ permalink raw reply related

* [PATCH 7/8] Remove not needed sdp_init_services_list function
From: Szymon Janc @ 2013-09-13 11:30 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1379071852-10094-1-git-send-email-szymon.janc@tieto.com>

It is doing nothing now and can be removed.
---
 src/adapter.c       |  2 --
 src/sdpd-database.c | 21 ---------------------
 src/sdpd.h          |  2 --
 3 files changed, 25 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 0ec1381..4a20df5 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -5613,8 +5613,6 @@ static int adapter_register(struct btd_adapter *adapter)
 		agent_unref(agent);
 	}
 
-	sdp_init_services_list(&adapter->bdaddr);
-
 	btd_adapter_gatt_server_start(adapter);
 
 	load_config(adapter);
diff --git a/src/sdpd-database.c b/src/sdpd-database.c
index e4d4f98..f65a526 100644
--- a/src/sdpd-database.c
+++ b/src/sdpd-database.c
@@ -296,24 +296,3 @@ uint32_t sdp_next_handle(void)
 
 	return handle;
 }
-
-void sdp_init_services_list(bdaddr_t *device)
-{
-	sdp_list_t *p;
-
-	DBG("");
-
-	for (p = access_db; p != NULL; p = p->next) {
-		sdp_access_t *access = p->data;
-		sdp_record_t *rec;
-
-		if (bacmp(BDADDR_ANY, &access->device))
-			continue;
-
-		rec = sdp_record_find(access->handle);
-		if (rec == NULL)
-			continue;
-
-		SDPDBG("adding record with handle %x", access->handle);
-	}
-}
diff --git a/src/sdpd.h b/src/sdpd.h
index 28d7f6d..77cafbd 100644
--- a/src/sdpd.h
+++ b/src/sdpd.h
@@ -79,5 +79,3 @@ void stop_sdp_server(void);
 
 int add_record_to_server(const bdaddr_t *src, sdp_record_t *rec);
 int remove_record_from_server(uint32_t handle);
-
-void sdp_init_services_list(bdaddr_t *device);
-- 
1.8.4


^ permalink raw reply related

* [PATCH 8/8] unit: Remove not needed functions from test-sdp
From: Szymon Janc @ 2013-09-13 11:30 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1379071852-10094-1-git-send-email-szymon.janc@tieto.com>

SDP code no longer depends on adapter code.
---
 unit/test-sdp.c | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/unit/test-sdp.c b/unit/test-sdp.c
index 5aa6948..6d699e2 100644
--- a/unit/test-sdp.c
+++ b/unit/test-sdp.c
@@ -134,35 +134,6 @@ void btd_debug(const char *format, ...)
 {
 }
 
-struct btd_adapter;
-
-typedef void (*adapter_cb) (struct btd_adapter *adapter, gpointer user_data);
-
-void adapter_foreach(adapter_cb func, gpointer user_data);
-
-void adapter_foreach(adapter_cb func, gpointer user_data)
-{
-}
-
-struct btd_adapter *adapter_find(const bdaddr_t *sba);
-
-struct btd_adapter *adapter_find(const bdaddr_t *sba)
-{
-	return NULL;
-}
-
-void adapter_service_insert(struct btd_adapter *adapter, void *rec);
-
-void adapter_service_insert(struct btd_adapter *adapter, void *rec)
-{
-}
-
-void adapter_service_remove(struct btd_adapter *adapter, void *rec);
-
-void adapter_service_remove(struct btd_adapter *adapter, void *rec)
-{
-}
-
 static void context_quit(struct context *context)
 {
 	g_main_loop_quit(context->main_loop);
-- 
1.8.4


^ permalink raw reply related

* Re: [PATCH 1/4] obexd: Add folder property to map_msg_create
From: Christian Fetzer @ 2013-09-13 11:33 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZKYBd=S0G3Hvno7wQxgtCy5nwnamTD=1sKcVw+kBkAVVg@mail.gmail.com>

Hi Luiz,

On 09/13/2013 12:11 PM, Luiz Augusto von Dentz wrote:
> Hi Christian,
> 
> On Fri, Sep 13, 2013 at 12:23 PM, Christian Fetzer
> <christian.fetzer@oss.bmw-carit.de> wrote:
>> From: Christian Fetzer <christian.fetzer@bmw-carit.de>
>>
>> Message interfaces are not necessarily created for the current folder,
>> therefore the folder needs to be specified in a parameter.
>>
>> For example, messages can be created for sub folders when using the folder
>> parameter in ListMessages.
>> ---
>>  obexd/client/map.c | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/obexd/client/map.c b/obexd/client/map.c
>> index f0dcf72..9a1b140 100644
>> --- a/obexd/client/map.c
>> +++ b/obexd/client/map.c
>> @@ -779,7 +779,8 @@ static const GDBusPropertyTable map_msg_properties[] = {
>>         { }
>>  };
>>
>> -static struct map_msg *map_msg_create(struct map_data *data, const char *handle)
>> +static struct map_msg *map_msg_create(struct map_data *data, const char *handle,
>> +                                                       const char *folder)
>>  {
>>         struct map_msg *msg;
>>
>> @@ -788,7 +789,7 @@ static struct map_msg *map_msg_create(struct map_data *data, const char *handle)
>>         msg->path = g_strdup_printf("%s/message%s",
>>                                         obc_session_get_path(data->session),
>>                                         handle);
>> -       msg->folder = g_strdup(obc_session_get_folder(data->session));
>> +       msg->folder = g_strdup(folder);
>>
>>         if (!g_dbus_register_interface(conn, msg->path, MAP_MSG_INTERFACE,
>>                                                 map_msg_methods, NULL,
>> @@ -1057,7 +1058,8 @@ static void msg_element(GMarkupParseContext *ctxt, const char *element,
>>
>>         msg = g_hash_table_lookup(data->messages, values[i]);
>>         if (msg == NULL) {
>> -               msg = map_msg_create(data, values[i]);
>> +               msg = map_msg_create(data, values[i],
>> +                                       obc_session_get_folder(data->session));
> 
> Is this really fixing anything? Because it seems it is just changing
> places where obc_session_get_folder is called when what you should
> probably be doing is to store the folder parameter given to
> ListMessages. 


The fix itself is in patch 2. But as explained in the cover letter,
I'll need the parameter as well when creating new messages from
event reports. (I have a first notification API patchset ready but
wanted to wait until this is applied.)


> I actually regret to have this parameter as it makes
> things a little bit more complicated just to avoid SetFolder.
> 
> Probably Session.SetPath would make more sense than having each
> profile interface treating it differently but that would require to
> break APIs so it is better not to do it right now.
> 


Fully agree. Maybe it's a good thing to keep 
those points in mind for future API changes.

Christian

^ permalink raw reply

* Re: [PATCH 2/4] obexd: Fix setting message folder for relative folder in ListMessages
From: Christian Fetzer @ 2013-09-13 11:45 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZL+d8CpQNJ2z750s30naUoY-049XpCs3gOtGMsG1hCJVw@mail.gmail.com>

Hi Luiz,

On 09/13/2013 12:47 PM, Luiz Augusto von Dentz wrote:
> Hi Christian,
> 
> On Fri, Sep 13, 2013 at 12:23 PM, Christian Fetzer
> <christian.fetzer@oss.bmw-carit.de> wrote:
>> From: Christian Fetzer <christian.fetzer@bmw-carit.de>
>>
>> The method ListMessages allows to specify a relative subfolder.
>> This subfolder needs to be added to the current path when registering
>> a new message interface.
>> ---
>>  obexd/client/map.c | 19 +++++++++++++++++--
>>  1 file changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/obexd/client/map.c b/obexd/client/map.c
>> index 9a1b140..54011d8 100644
>> --- a/obexd/client/map.c
>> +++ b/obexd/client/map.c
>> @@ -96,6 +96,7 @@ static const char * const filter_list[] = {
>>  struct map_data {
>>         struct obc_session *session;
>>         DBusMessage *msg;
>> +       char *folder;
>>         GHashTable *messages;
>>         int16_t mas_instance_id;
>>         uint8_t supported_message_types;
>> @@ -1058,8 +1059,7 @@ static void msg_element(GMarkupParseContext *ctxt, const char *element,
>>
>>         msg = g_hash_table_lookup(data->messages, values[i]);
>>         if (msg == NULL) {
>> -               msg = map_msg_create(data, values[i],
>> -                                       obc_session_get_folder(data->session));
>> +               msg = map_msg_create(data, values[i], data->folder);
>>                 if (msg == NULL)
>>                         return;
>>         }
>> @@ -1153,6 +1153,19 @@ static void message_listing_cb(struct obc_session *session,
>>  done:
>>         g_dbus_send_message(conn, reply);
>>         dbus_message_unref(map->msg);
>> +       g_free(map->folder);
>> +       map->folder = NULL;
>> +}
>> +
>> +static char *get_absolute_folder(const char *root, const char *subfolder)
>> +{
>> +       if (!subfolder || strlen(subfolder) == 0)
>> +               return g_strdup(root);
>> +       else
>> +               if (g_str_has_suffix(root, "/"))
>> +                       return g_strconcat(root, subfolder, NULL);
>> +               else
>> +                       return g_strconcat(root, "/", subfolder, NULL);
>>  }
>>
>>  static DBusMessage *get_message_listing(struct map_data *map,
>> @@ -1175,6 +1188,8 @@ static DBusMessage *get_message_listing(struct map_data *map,
>>         if (obc_session_queue(map->session, transfer, message_listing_cb, map,
>>                                                                 &err)) {
>>                 map->msg = dbus_message_ref(message);
>> +               map->folder = get_absolute_folder(obc_session_get_folder(
>> +                                                       map->session), folder);
>>                 return NULL;
>>         }
>>
>> --
>> 1.8.3.4
> 
> This will probably not work in case of multiple outstanding requests
> the last will always overwrite the folder, which btw will leak, so
> probably we need a per request data.
> 
> 

Yes, the issue exists already in the current code base, because the stored
dbus message is overridden if any MAP function is called before the previous
one is finished.

I've been able to reproduce a crash with it and already started to write a patch
that adds a pending_request on top of this patch.

Do you prefer to have a fix first and rebase this patchset on it, or do you
prefer to get the notification API patchset first? (which is already ready to be sent)

Christian

^ permalink raw reply

* Re: [PATCH 2/4] obexd: Fix setting message folder for relative folder in ListMessages
From: Luiz Augusto von Dentz @ 2013-09-13 11:54 UTC (permalink / raw)
  To: Christian Fetzer; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <5232FAC8.5090903@oss.bmw-carit.de>

Hi Christian,

On Fri, Sep 13, 2013 at 2:45 PM, Christian Fetzer
<christian.fetzer@oss.bmw-carit.de> wrote:
> Hi Luiz,
>
> On 09/13/2013 12:47 PM, Luiz Augusto von Dentz wrote:
>> Hi Christian,
>>
>> On Fri, Sep 13, 2013 at 12:23 PM, Christian Fetzer
>> <christian.fetzer@oss.bmw-carit.de> wrote:
>>> From: Christian Fetzer <christian.fetzer@bmw-carit.de>
>>>
>>> The method ListMessages allows to specify a relative subfolder.
>>> This subfolder needs to be added to the current path when registering
>>> a new message interface.
>>> ---
>>>  obexd/client/map.c | 19 +++++++++++++++++--
>>>  1 file changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/obexd/client/map.c b/obexd/client/map.c
>>> index 9a1b140..54011d8 100644
>>> --- a/obexd/client/map.c
>>> +++ b/obexd/client/map.c
>>> @@ -96,6 +96,7 @@ static const char * const filter_list[] = {
>>>  struct map_data {
>>>         struct obc_session *session;
>>>         DBusMessage *msg;
>>> +       char *folder;
>>>         GHashTable *messages;
>>>         int16_t mas_instance_id;
>>>         uint8_t supported_message_types;
>>> @@ -1058,8 +1059,7 @@ static void msg_element(GMarkupParseContext *ctxt, const char *element,
>>>
>>>         msg = g_hash_table_lookup(data->messages, values[i]);
>>>         if (msg == NULL) {
>>> -               msg = map_msg_create(data, values[i],
>>> -                                       obc_session_get_folder(data->session));
>>> +               msg = map_msg_create(data, values[i], data->folder);
>>>                 if (msg == NULL)
>>>                         return;
>>>         }
>>> @@ -1153,6 +1153,19 @@ static void message_listing_cb(struct obc_session *session,
>>>  done:
>>>         g_dbus_send_message(conn, reply);
>>>         dbus_message_unref(map->msg);
>>> +       g_free(map->folder);
>>> +       map->folder = NULL;
>>> +}
>>> +
>>> +static char *get_absolute_folder(const char *root, const char *subfolder)
>>> +{
>>> +       if (!subfolder || strlen(subfolder) == 0)
>>> +               return g_strdup(root);
>>> +       else
>>> +               if (g_str_has_suffix(root, "/"))
>>> +                       return g_strconcat(root, subfolder, NULL);
>>> +               else
>>> +                       return g_strconcat(root, "/", subfolder, NULL);
>>>  }
>>>
>>>  static DBusMessage *get_message_listing(struct map_data *map,
>>> @@ -1175,6 +1188,8 @@ static DBusMessage *get_message_listing(struct map_data *map,
>>>         if (obc_session_queue(map->session, transfer, message_listing_cb, map,
>>>                                                                 &err)) {
>>>                 map->msg = dbus_message_ref(message);
>>> +               map->folder = get_absolute_folder(obc_session_get_folder(
>>> +                                                       map->session), folder);
>>>                 return NULL;
>>>         }
>>>
>>> --
>>> 1.8.3.4
>>
>> This will probably not work in case of multiple outstanding requests
>> the last will always overwrite the folder, which btw will leak, so
>> probably we need a per request data.
>>
>>
>
> Yes, the issue exists already in the current code base, because the stored
> dbus message is overridden if any MAP function is called before the previous
> one is finished.
>
> I've been able to reproduce a crash with it and already started to write a patch
> that adds a pending_request on top of this patch.
>
> Do you prefer to have a fix first and rebase this patchset on it, or do you
> prefer to get the notification API patchset first? (which is already ready to be sent)

Fixes should take precedence over regular patches specially if it is
fixing crashes.


-- 
Luiz Augusto von Dentz

^ permalink raw reply

* Re: [PATCH 3/8] sap: Keep reference to btd_adapter in struct sap_server
From: Johan Hedberg @ 2013-09-13 12:33 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1379071852-10094-3-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Fri, Sep 13, 2013, Szymon Janc wrote:
> Path and adapter address can be obtained easily from btd_adapter
> and there is no need to keep local copy.
> ---
>  profiles/sap/manager.c |  6 ++----
>  profiles/sap/server.c  | 23 +++++++++++++----------
>  profiles/sap/server.h  |  2 +-
>  3 files changed, 16 insertions(+), 15 deletions(-)

I've applied patches 1-3 but for the rest I'm waiting for a v2 or some
more discussion (on IRC or here).

Johan

^ permalink raw reply

* Re: [Bluetooth Low Energy] Pairing and writing characteristic property issue
From: Luiz Augusto von Dentz @ 2013-09-13 12:39 UTC (permalink / raw)
  To: Nedim Hadzic; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <285F7E29A611CC4197B552ED66CC20B8CE898C@XMB102ADS.rim.net>

Hi Nedim,

On Thu, Sep 12, 2013 at 7:55 PM, Nedim Hadzic <nhadzic@blackberry.com> wrote:
> Hi again,
>
> I am not sure if I sent mail to wrong list, but if I did can someone inform me. I am still having a problems with this issues so I would appreciate some help.

First I should let you know that we don't top post on linux-bluetooth,
so please when you reply please do inline comments.

> ________________________________________
> From: linux-bluetooth-owner@vger.kernel.org [linux-bluetooth-owner@vger.kernel.org] on behalf of Nedim Hadzic [nhadzic@blackberry.com]
> Sent: 11 September 2013 07:34
> To: linux-bluetooth@vger.kernel.org
> Subject: [Bluetooth Low Energy] Pairing and writing characteristic property issue
>
> Hello everyone,
>
> Recently I started working with Bluetooth Low Energy devices on Linux (developing some applications, testing devices etc). I am using two Bluetooth Low Energy devices SensorTag from Texas Instruments and HearRateMonitor BlueHR.
>
> Bluez version: 4.101-0ubuntu8b1
>
> SensorTag: I can connect to the device, read and write characteristic values using gatttool and I can pair with the device using a simple-agent tool for this. After I pair with the device, I want to change value of the characteristic using org.bluez.Characteristic interface and SetProperty method, but it does not change; nor PropertyChanged signal is emitted nor I get any error. I checked hcidump:
> 2013-09-11 12:07:15.827211 < ACL data: handle 75 flags 0x00 dlen 8
>     ATT: Write cmd (0x52)
>       handle 0x0029 value  0x01
>
> In case of changing the characteristic value with gatttool hcidump is following:
> 2013-09-11 12:20:07.239012 < ACL data: handle 75 flags 0x00 dlen 8
>     ATT: Write req (0x12)
>       handle 0x0029 value  0x01
>
> Difference is in part of write cmd and req. Only difference between these two approaches is that with gatttool, you first connect to LE device (LE Create Connection in hcidump), and in hcdump when accessing through interface and using method SetProperty there is no interface or method for connecting. Any guesses what can be done?
>
>
> HearRateMonitor: I can connect to the device, read characteristic values using gatttool ( with adding "-t random" part to the command), but I can not pair with the device using simple-agent: Error: Creating device failed: org.bluez.Error.AuthenticationFailed: Authentication Failed.
> Hcidump is the following:
> 2013-09-11 11:58:41.747749 < HCI Command: LE Create Connection (0x08|0x000d) plen 25
>     bdaddr D5:EA:8B:A9:EC:70 type 1
> 2013-09-11 11:58:41.751989 > HCI Event: Command Status (0x0f) plen 4
>     LE Create Connection (0x08|0x000d) status 0x00 ncmd 1
> 2013-09-11 11:58:44.311996 > HCI Event: LE Meta Event (0x3e) plen 19
>     LE Connection Complete
>       status 0x00 handle 75, role master
>       bdaddr D5:EA:8B:A9:EC:70 (Random)
> 2013-09-11 11:58:44.312173 < ACL data: handle 75 flags 0x00 dlen 11
>     SMP: Pairing Request (0x01)
>       capability 0x04 oob 0x00 auth req 0x01
>       max key size 0x10 init key dist 0x00 resp key dist 0x01
>       Capability: KeyboardDisplay (OOB data not present)
>       Authentication: Bonding (No MITM Protection)
>       Initiator Key Distribution:
>       Responder Key Distribution:  LTK
> 2013-09-11 11:58:44.348985 > HCI Event: Number of Completed Packets (0x13) plen 5
>     handle 75 packets 1
> 2013-09-11 11:58:44.418969 > ACL data: handle 75 flags 0x02 dlen 6
>     SMP: Pairing Failed (0x05)
>       reason 0x05
>       Reason Pairing Not Supported
> 2013-09-11 11:58:44.560004 > HCI Event: Disconn Complete (0x05) plen 4
>     status 0x00 handle 75 reason 0x13
>     Reason: Remote User Terminated Connection
>
> This device is using random device address, but I do not know how to pair with it in that case. I try to pair with it using a smartphone with support and it is working. Any help, or input on this?

I believe we don't support pairing with devices using random/private,
but you should probably upgrade if you are planning to use Bluetooth
LE BlueZ 5.x is recommended.

^ permalink raw reply

* Re: [Bluetooth Low Energy] Pairing and writing characteristic property issue
From: Johan Hedberg @ 2013-09-13 12:57 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: Nedim Hadzic, linux-bluetooth@vger.kernel.org
In-Reply-To: <CABBYNZ+smnsbiFhVB1XY50Ffm_d10TYPP2j4Ru+UBnonyjjk8g@mail.gmail.com>

Hi Luiz,

On Fri, Sep 13, 2013, Luiz Augusto von Dentz wrote:
> > This device is using random device address, but I do not know how to
> > pair with it in that case. I try to pair with it using a smartphone
> > with support and it is working. Any help, or input on this?
> 
> I believe we don't support pairing with devices using random/private,
> but you should probably upgrade if you are planning to use Bluetooth
> LE BlueZ 5.x is recommended.

In general we don't really support any LE related stuff with BlueZ 4
simply because LE pairing requires the mgmt interface and it wasn't
stable yet during BlueZ 4 times.

Regarding pairing with random addressed devices, BlueZ 5 will allow that
but only for a single connection. Once you disconnect the pairing info
is gone since it'd anyway be unusable if the remote side changed its
address. Until we get IRK generation support for the kernel this is how
user space will keep behaving.

Johan

^ permalink raw reply

* Re: [Bluetooth Low Energy] Pairing and writing characteristic property issue
From: Johan Hedberg @ 2013-09-13 13:05 UTC (permalink / raw)
  To: Luiz Augusto von Dentz, Nedim Hadzic,
	linux-bluetooth@vger.kernel.org
In-Reply-To: <20130913125758.GA29830@x220.p-661hnu-f1>

Hi,

On Fri, Sep 13, 2013, Johan Hedberg wrote:
> On Fri, Sep 13, 2013, Luiz Augusto von Dentz wrote:
> > > This device is using random device address, but I do not know how to
> > > pair with it in that case. I try to pair with it using a smartphone
> > > with support and it is working. Any help, or input on this?
> > 
> > I believe we don't support pairing with devices using random/private,
> > but you should probably upgrade if you are planning to use Bluetooth
> > LE BlueZ 5.x is recommended.
> 
> In general we don't really support any LE related stuff with BlueZ 4
> simply because LE pairing requires the mgmt interface and it wasn't
> stable yet during BlueZ 4 times.
> 
> Regarding pairing with random addressed devices, BlueZ 5 will allow that
> but only for a single connection. Once you disconnect the pairing info
> is gone since it'd anyway be unusable if the remote side changed its
> address. Until we get IRK generation support for the kernel this is how
> user space will keep behaving.

Correcting myself: it's only private random address devices that will
have this behavior; static random addresses will work fine and keep the
pairing info persistent.

Johan

^ permalink raw reply

* [PATCH 1/6] obexd: Add request struct to MAP
From: Christian Fetzer @ 2013-09-13 15:28 UTC (permalink / raw)
  To: linux-bluetooth

From: Christian Fetzer <christian.fetzer@bmw-carit.de>

This adds a pending_request struct in order to store the D-Bus request
data.

The current version stores the received D-Bus message in the MAP session
struct. The stored message is overridden by intermediate D-Bus method
calls which can lead into a crash.

Trace:
  arguments to dbus_message_unref() were incorrect,
  assertion "!message->in_cache" failed in file dbus-message.c line 1618.

 0  0x00007ffff6a6a1c9 in raise () from /usr/lib/libc.so.6
 1  0x00007ffff6a6b5c8 in abort () from /usr/lib/libc.so.6
 2  0x00007ffff7313de5 in ?? () from /usr/lib/libdbus-1.so.3
 3  0x00007ffff730ab91 in ?? () from /usr/lib/libdbus-1.so.3
 4  0x000000000043721c in message_listing_cb (session=0x6a7d30,
    transfer=0x6a9450, err=0x0, user_data=0x6a9950) at obexd/client/map.c:1166
 5  0x000000000042f7af in session_terminate_transfer (session=0x6a7d30,
    transfer=0x6a9450, gerr=0x0) at obexd/client/session.c:830
 6  0x000000000042f83d in session_notify_complete (session=0x6a7d30,
    transfer=0x6a9450) at obexd/client/session.c:845
 7  0x000000000042f8dc in transfer_complete (transfer=0x6a9450, err=0x0,
    user_data=0x6a7d30) at obexd/client/session.c:865
 8  0x0000000000439ee7 in xfer_complete (obex=0x677250, err=0x0,
    user_data=0x6a9450) at obexd/client/transfer.c:577
 9  0x000000000043a05f in get_xfer_progress_first (obex=0x677250, err=0x0,
    rsp=0x678730, user_data=0x6a9450) at obexd/client/transfer.c:621
 10 0x0000000000413f08 in handle_response (obex=0x677250, err=0x0,
    rsp=0x678730) at gobex/gobex.c:949
 11 0x00000000004147db in incoming_data (io=0x6a8a00, cond=G_IO_IN,
    user_data=0x677250) at gobex/gobex.c:1192
 12 0x00007ffff702dda6 in g_main_context_dispatch ()
   from /usr/lib/libglib-2.0.so.0
 13 0x00007ffff702e0f8 in ?? () from /usr/lib/libglib-2.0.so.0
 14 0x00007ffff702e4fa in g_main_loop_run () from /usr/lib/libglib-2.0.so.0
 15 0x0000000000427ce8 in main (argc=1, argv=0x7fffffffdd48)
    at obexd/src/main.c:319
---
 obexd/client/map.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/obexd/client/map.c b/obexd/client/map.c
index 95f0334..f969aad 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -101,6 +101,11 @@ struct map_data {
 	uint8_t supported_message_types;
 };
 
+struct pending_request {
+	struct map_data *map;
+	DBusMessage *msg;
+};
+
 #define MAP_MSG_FLAG_PRIORITY	0x01
 #define MAP_MSG_FLAG_READ	0x02
 #define MAP_MSG_FLAG_SENT	0x04
@@ -134,6 +139,25 @@ struct map_parser {
 
 static DBusConnection *conn = NULL;
 
+static struct pending_request *pending_request_new(struct map_data *map,
+							DBusMessage *message)
+{
+	struct pending_request *p;
+
+	p = g_new0(struct pending_request, 1);
+	p->map = map;
+	p->msg = dbus_message_ref(message);
+
+	return p;
+}
+
+static void pending_request_free(struct pending_request *p)
+{
+	dbus_message_unref(p->msg);
+
+	g_free(p);
+}
+
 static void simple_cb(struct obc_session *session,
 						struct obc_transfer *transfer,
 						GError *err, void *user_data)
-- 
1.8.3.4


^ permalink raw reply related

* [PATCH 2/6] obexd: Use pending request in SetFolder
From: Christian Fetzer @ 2013-09-13 15:28 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1379086116-22617-1-git-send-email-christian.fetzer@oss.bmw-carit.de>

From: Christian Fetzer <christian.fetzer@bmw-carit.de>

diff --git a/obexd/client/map.c b/obexd/client/map.c
index f969aad..1b2b25d 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -162,18 +162,18 @@ static void simple_cb(struct obc_session *session,
 						struct obc_transfer *transfer,
 						GError *err, void *user_data)
 {
+	struct pending_request *request = user_data;
 	DBusMessage *reply;
-	struct map_data *map = user_data;
 
 	if (err != NULL)
-		reply = g_dbus_create_error(map->msg,
+		reply = g_dbus_create_error(request->msg,
 						ERROR_INTERFACE ".Failed",
 						"%s", err->message);
 	else
-		reply = dbus_message_new_method_return(map->msg);
+		reply = dbus_message_new_method_return(request->msg);
 
 	g_dbus_send_message(conn, reply);
-	dbus_message_unref(map->msg);
+	pending_request_free(request);
 }
 
 static DBusMessage *map_setpath(DBusConnection *connection,
@@ -181,6 +181,7 @@ static DBusMessage *map_setpath(DBusConnection *connection,
 {
 	struct map_data *map = user_data;
 	const char *folder;
+	struct pending_request *request;
 	GError *err = NULL;
 
 	if (dbus_message_get_args(message, NULL, DBUS_TYPE_STRING, &folder,
@@ -189,18 +190,19 @@ static DBusMessage *map_setpath(DBusConnection *connection,
 					ERROR_INTERFACE ".InvalidArguments",
 					NULL);
 
-	obc_session_setpath(map->session, folder, simple_cb, map, &err);
+	request = pending_request_new(map, message);
+
+	obc_session_setpath(map->session, folder, simple_cb, request, &err);
 	if (err != NULL) {
 		DBusMessage *reply;
 		reply =  g_dbus_create_error(message,
 						ERROR_INTERFACE ".Failed",
 						"%s", err->message);
 		g_error_free(err);
+		pending_request_free(request);
 		return reply;
 	}
 
-	map->msg = dbus_message_ref(message);
-
 	return NULL;
 }
 
-- 
1.8.3.4


^ permalink raw reply related

* [PATCH 3/6] obexd: Use pending request in ListFolders
From: Christian Fetzer @ 2013-09-13 15:28 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1379086116-22617-1-git-send-email-christian.fetzer@oss.bmw-carit.de>

From: Christian Fetzer <christian.fetzer@bmw-carit.de>

diff --git a/obexd/client/map.c b/obexd/client/map.c
index 1b2b25d..29d33fa 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -243,7 +243,7 @@ static void folder_listing_cb(struct obc_session *session,
 						struct obc_transfer *transfer,
 						GError *err, void *user_data)
 {
-	struct map_data *map = user_data;
+	struct pending_request *request = user_data;
 	GMarkupParseContext *ctxt;
 	DBusMessage *reply;
 	DBusMessageIter iter, array;
@@ -252,7 +252,7 @@ static void folder_listing_cb(struct obc_session *session,
 	int perr;
 
 	if (err != NULL) {
-		reply = g_dbus_create_error(map->msg,
+		reply = g_dbus_create_error(request->msg,
 						ERROR_INTERFACE ".Failed",
 						"%s", err->message);
 		goto done;
@@ -260,14 +260,14 @@ static void folder_listing_cb(struct obc_session *session,
 
 	perr = obc_transfer_get_contents(transfer, &contents, &size);
 	if (perr < 0) {
-		reply = g_dbus_create_error(map->msg,
+		reply = g_dbus_create_error(request->msg,
 						ERROR_INTERFACE ".Failed",
 						"Error reading contents: %s",
 						strerror(-perr));
 		goto done;
 	}
 
-	reply = dbus_message_new_method_return(map->msg);
+	reply = dbus_message_new_method_return(request->msg);
 	if (reply == NULL)
 		return;
 
@@ -285,13 +285,14 @@ static void folder_listing_cb(struct obc_session *session,
 
 done:
 	g_dbus_send_message(conn, reply);
-	dbus_message_unref(map->msg);
+	pending_request_free(request);
 }
 
 static DBusMessage *get_folder_listing(struct map_data *map,
 							DBusMessage *message,
 							GObexApparam *apparam)
 {
+	struct pending_request *request;
 	struct obc_transfer *transfer;
 	GError *err = NULL;
 	DBusMessage *reply;
@@ -304,12 +305,16 @@ static DBusMessage *get_folder_listing(struct map_data *map,
 
 	obc_transfer_set_apparam(transfer, apparam);
 
-	if (obc_session_queue(map->session, transfer, folder_listing_cb, map,
-								&err)) {
-		map->msg = dbus_message_ref(message);
-		return NULL;
+	request = pending_request_new(map, message);
+
+	if (!obc_session_queue(map->session, transfer, folder_listing_cb,
+							request, &err)) {
+		pending_request_free(request);
+		goto fail;
 	}
 
+	return NULL;
+
 fail:
 	reply = g_dbus_create_error(message, ERROR_INTERFACE ".Failed", "%s",
 								err->message);
-- 
1.8.3.4


^ permalink raw reply related

* [PATCH 4/6] obexd: Use pending request in ListMessages
From: Christian Fetzer @ 2013-09-13 15:28 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1379086116-22617-1-git-send-email-christian.fetzer@oss.bmw-carit.de>

From: Christian Fetzer <christian.fetzer@bmw-carit.de>

diff --git a/obexd/client/map.c b/obexd/client/map.c
index 29d33fa..290cfee 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -133,7 +133,7 @@ struct map_msg {
 };
 
 struct map_parser {
-	struct map_data *data;
+	struct pending_request *request;
 	DBusMessageIter *iter;
 };
 
@@ -1082,7 +1082,7 @@ static void msg_element(GMarkupParseContext *ctxt, const char *element,
 				gpointer user_data, GError **gerr)
 {
 	struct map_parser *parser = user_data;
-	struct map_data *data = parser->data;
+	struct map_data *data = parser->request->map;
 	DBusMessageIter entry, *iter = parser->iter;
 	struct map_msg *msg;
 	const char *key;
@@ -1137,7 +1137,7 @@ static void message_listing_cb(struct obc_session *session,
 						struct obc_transfer *transfer,
 						GError *err, void *user_data)
 {
-	struct map_data *map = user_data;
+	struct pending_request *request = user_data;
 	struct map_parser *parser;
 	GMarkupParseContext *ctxt;
 	DBusMessage *reply;
@@ -1147,7 +1147,7 @@ static void message_listing_cb(struct obc_session *session,
 	int perr;
 
 	if (err != NULL) {
-		reply = g_dbus_create_error(map->msg,
+		reply = g_dbus_create_error(request->msg,
 						ERROR_INTERFACE ".Failed",
 						"%s", err->message);
 		goto done;
@@ -1155,14 +1155,14 @@ static void message_listing_cb(struct obc_session *session,
 
 	perr = obc_transfer_get_contents(transfer, &contents, &size);
 	if (perr < 0) {
-		reply = g_dbus_create_error(map->msg,
+		reply = g_dbus_create_error(request->msg,
 						ERROR_INTERFACE ".Failed",
 						"Error reading contents: %s",
 						strerror(-perr));
 		goto done;
 	}
 
-	reply = dbus_message_new_method_return(map->msg);
+	reply = dbus_message_new_method_return(request->msg);
 	if (reply == NULL)
 		return;
 
@@ -1179,7 +1179,7 @@ static void message_listing_cb(struct obc_session *session,
 					&array);
 
 	parser = g_new(struct map_parser, 1);
-	parser->data = map;
+	parser->request = request;
 	parser->iter = &array;
 
 	ctxt = g_markup_parse_context_new(&msg_parser, 0, parser, NULL);
@@ -1191,7 +1191,7 @@ static void message_listing_cb(struct obc_session *session,
 
 done:
 	g_dbus_send_message(conn, reply);
-	dbus_message_unref(map->msg);
+	pending_request_free(request);
 }
 
 static DBusMessage *get_message_listing(struct map_data *map,
@@ -1199,6 +1199,7 @@ static DBusMessage *get_message_listing(struct map_data *map,
 							const char *folder,
 							GObexApparam *apparam)
 {
+	struct pending_request *request;
 	struct obc_transfer *transfer;
 	GError *err = NULL;
 	DBusMessage *reply;
@@ -1211,12 +1212,16 @@ static DBusMessage *get_message_listing(struct map_data *map,
 
 	obc_transfer_set_apparam(transfer, apparam);
 
-	if (obc_session_queue(map->session, transfer, message_listing_cb, map,
-								&err)) {
-		map->msg = dbus_message_ref(message);
-		return NULL;
+	request = pending_request_new(map, message);
+
+	if (!obc_session_queue(map->session, transfer, message_listing_cb,
+							request, &err)) {
+		pending_request_free(request);
+		goto fail;
 	}
 
+	return NULL;
+
 fail:
 	reply = g_dbus_create_error(message, ERROR_INTERFACE ".Failed", "%s",
 								err->message);
-- 
1.8.3.4


^ permalink raw reply related

* [PATCH 5/6] obexd: Use pending request in UpdateInbox
From: Christian Fetzer @ 2013-09-13 15:28 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1379086116-22617-1-git-send-email-christian.fetzer@oss.bmw-carit.de>

From: Christian Fetzer <christian.fetzer@bmw-carit.de>

diff --git a/obexd/client/map.c b/obexd/client/map.c
index 290cfee..8b56143 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -1559,21 +1559,21 @@ static void update_inbox_cb(struct obc_session *session,
 				struct obc_transfer *transfer,
 				GError *err, void *user_data)
 {
-	struct map_data *map = user_data;
+	struct pending_request *request = user_data;
 	DBusMessage *reply;
 
 	if (err != NULL) {
-		reply = g_dbus_create_error(map->msg,
+		reply = g_dbus_create_error(request->msg,
 						ERROR_INTERFACE ".Failed",
 						"%s", err->message);
 		goto done;
 	}
 
-	reply = dbus_message_new_method_return(map->msg);
+	reply = dbus_message_new_method_return(request->msg);
 
 done:
 	g_dbus_send_message(conn, reply);
-	dbus_message_unref(map->msg);
+	pending_request_free(request);
 }
 
 static DBusMessage *map_update_inbox(DBusConnection *connection,
@@ -1584,6 +1584,7 @@ static DBusMessage *map_update_inbox(DBusConnection *connection,
 	char contents[2];
 	struct obc_transfer *transfer;
 	GError *err = NULL;
+	struct pending_request *request;
 
 	contents[0] = FILLER_BYTE;
 	contents[1] = '\0';
@@ -1594,11 +1595,13 @@ static DBusMessage *map_update_inbox(DBusConnection *connection,
 	if (transfer == NULL)
 		goto fail;
 
+	request = pending_request_new(map, message);
+
 	if (!obc_session_queue(map->session, transfer, update_inbox_cb,
-								map, &err))
+							request, &err)) {
+		pending_request_free(request);
 		goto fail;
-
-	map->msg = dbus_message_ref(message);
+	}
 
 	return NULL;
 
-- 
1.8.3.4


^ permalink raw reply related


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