[PATCH 1/3] plugins/sixaxis: Fix cable pairing not working

[PATCH 1/3] plugins/sixaxis: Fix cable pairing not working

[Bastien Nocera]

In 6c467e9, get_pairing_type() was modified to return a structure that's
allocated on the stack. While not a problem for the direct callers,
as the function is defined as "inline", function that expected to return
this structure themselves would fail, as the stack would be trampled
upon on function exit.

Make such a function (get_pairing_type_for_device()) in the sixaxis
plugin be inline as well so that its caller doesn't get garbage inside
of cable pairing information from a struct.
---
 plugins/sixaxis.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
index 89cd8f1f5..9d0fdb5d3 100644
--- a/plugins/sixaxis.c
+++ b/plugins/sixaxis.c
@@ -435,7 +435,7 @@ static bool setup_device(int fd, const char *sysfs_path,
 	return true;
 }
 
-static const struct cable_pairing *
+static inline const struct cable_pairing *
 get_pairing_type_for_device(struct udev_device *udevice, uint16_t *bus,
 						char **sysfs_path)
 {
-- 
2.14.3

[PATCH 2/3] core/device: Add support for setting SDP record

[Bastien Nocera]

From: Szymon Janc <szymon.janc@codecoup.pl>

This allows to set SDP record for device without resolving services
over SDP. After SDP is provided profiles are probed.
---
 src/device.c | 31 +++++++++++++++++++++++++++++++
 src/device.h |  3 +++
 2 files changed, 34 insertions(+)

diff --git a/src/device.c b/src/device.c
index fd7a64134..f01c8a137 100644
--- a/src/device.c
+++ b/src/device.c
@@ -6072,6 +6072,37 @@ static sdp_list_t *read_device_records(struct btd_device *device)
 	return recs;
 }
 
+void btd_device_set_record(struct btd_device *device, const char *uuid,
+							sdp_record_t *rec)
+{
+	/* This API is only used for BR/EDR */
+	struct bearer_state *state = &device->bredr_state;
+	struct browse_req *req;
+	sdp_list_t *recs = NULL;
+
+	if (!rec)
+		return;
+
+	req = browse_request_new(device, BROWSE_SDP, NULL);
+	if (!req)
+		return;
+
+	recs = sdp_list_append(recs, rec);
+	update_bredr_services(req, recs);
+	sdp_list_free(recs, NULL);
+
+	device->svc_refreshed = true;
+	state->svc_resolved = true;
+
+	device_probe_profiles(device, req->profiles_added);
+
+	/* Propagate services changes */
+	g_dbus_emit_property_changed(dbus_conn, req->device->path,
+						DEVICE_INTERFACE, "UUIDs");
+
+	device_svc_resolved(device, BROWSE_SDP, device->bdaddr_type, 0);
+}
+
 const sdp_record_t *btd_device_get_record(struct btd_device *device,
 							const char *uuid)
 {
diff --git a/src/device.h b/src/device.h
index 850561729..406cd908e 100644
--- a/src/device.h
+++ b/src/device.h
@@ -62,6 +62,9 @@ struct device_addr_type {
 int device_addr_type_cmp(gconstpointer a, gconstpointer b);
 GSList *btd_device_get_uuids(struct btd_device *device);
 void device_probe_profiles(struct btd_device *device, GSList *profiles);
+
+void btd_device_set_record(struct btd_device *device, const char *uuid,
+							sdp_record_t *rec);
 const sdp_record_t *btd_device_get_record(struct btd_device *device,
 						const char *uuid);
 struct gatt_primary *btd_device_get_primary(struct btd_device *device,
-- 
2.14.3

[PATCH 3/3] plugins/sixaxis: Provide DualShock 3 SDP record while adding new device

[Bastien Nocera]

From: Szymon Janc <szymon.janc@codecoup.pl>

This allows to skip SDP search for DualShock 3 devices. Since some
DS3 clones were reported to not provide any SDP record this should
allow to operate them.
---
 Makefile.plugins  |   1 +
 plugins/sixaxis.c | 170 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 171 insertions(+)

diff --git a/Makefile.plugins b/Makefile.plugins
index 1f3b5b552..a1073d794 100644
--- a/Makefile.plugins
+++ b/Makefile.plugins
@@ -109,4 +109,5 @@ plugins_sixaxis_la_SOURCES = plugins/sixaxis.c
 plugins_sixaxis_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version \
 						-no-undefined @UDEV_LIBS@
 plugins_sixaxis_la_CFLAGS = $(AM_CFLAGS) -fvisibility=hidden @UDEV_CFLAGS@
+plugins_sixaxis_la_LIBADD = lib/libbluetooth-internal.la
 endif
diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
index 9d0fdb5d3..3ce26793a 100644
--- a/plugins/sixaxis.c
+++ b/plugins/sixaxis.c
@@ -40,6 +40,8 @@
 
 #include "lib/bluetooth.h"
 #include "lib/sdp.h"
+#include "lib/sdp_lib.h"
+#include "lib/sdp.h"
 #include "lib/uuid.h"
 
 #include "src/adapter.h"
@@ -85,6 +87,171 @@ static void auth_closure_destroy(struct authentication_closure *closure,
 	g_free(closure);
 }
 
+static sdp_record_t *get_sixaxis_sdp_record(void)
+{
+	sdp_record_t *record;
+	uint16_t hid_release, hid_parser, version, timeout;
+	uint8_t sdp_disable, battery, remote_wakeup, norm_connect, boot_device;
+	uint8_t subclass, country, virtual_cable, reconnect;
+	sdp_list_t *svclass_id, *pfseq, *apseq, *root;
+	uuid_t root_uuid, hidkb_uuid, l2cap_uuid, hidp_uuid;
+	sdp_profile_desc_t profile;
+	sdp_list_t *aproto, *proto[3];
+	sdp_data_t *psm, *lang_lst, *lang_lst2, *hid_spec_lst, *hid_spec_lst2;
+	uint8_t dtd = SDP_UINT16;
+	uint8_t dtd2 = SDP_UINT8;
+	uint8_t dtd_data = SDP_TEXT_STR8;
+	void *dtds[2];
+	void *values[2];
+	void *dtds2[2];
+	void *values2[2];
+	int leng[2];
+	uint8_t hid_spec_type = 0x22;
+	uint16_t hid_attr_lang[] = { 0x409, 0x100 };
+	static const uint16_t ctrl = 0x11;
+	static const uint16_t intr = 0x13;
+	uint8_t hid_spec[] = {
+		0x05, 0x01, 0x09, 0x04, 0xa1, 0x01, 0xa1, 0x02, 0x85, 0x01,
+		0x75, 0x08, 0x95, 0x01, 0x15, 0x00, 0x26, 0xff, 0x00, 0x81,
+		0x03, 0x75, 0x01, 0x95, 0x13, 0x15, 0x00, 0x25, 0x01, 0x35,
+		0x00, 0x45, 0x01, 0x05, 0x09, 0x19, 0x01, 0x29, 0x13, 0x81,
+		0x02, 0x75, 0x01, 0x95, 0x0d, 0x06, 0x00, 0xff, 0x81, 0x03,
+		0x15, 0x00, 0x26, 0xff, 0x00, 0x05, 0x01, 0x09, 0x01, 0xa1,
+		0x00, 0x75, 0x08, 0x95, 0x04, 0x35, 0x00, 0x46, 0xff, 0x00,
+		0x09, 0x30, 0x09, 0x31, 0x09, 0x32, 0x09, 0x35, 0x81, 0x02,
+		0xc0, 0x05, 0x01, 0x75, 0x08, 0x95, 0x27, 0x09, 0x01, 0x81,
+		0x02, 0x75, 0x08, 0x95, 0x30, 0x09, 0x01, 0x91, 0x02, 0x75,
+		0x08, 0x95, 0x30, 0x09, 0x01, 0xb1, 0x02, 0xc0, 0xa1, 0x02,
+		0x85, 0x02, 0x75, 0x08, 0x95, 0x30, 0x09, 0x01, 0xb1, 0x02,
+		0xc0, 0xa1, 0x02, 0x85, 0xee, 0x75, 0x08, 0x95, 0x30, 0x09,
+		0x01, 0xb1, 0x02, 0xc0, 0xa1, 0x02, 0x85, 0xef, 0x75, 0x08,
+		0x95, 0x30, 0x09, 0x01, 0xb1, 0x02, 0xc0, 0xc0, 0x00
+	};
+
+	record = sdp_record_alloc();
+	if (!record)
+		return NULL;
+
+	sdp_uuid16_create(&root_uuid, PUBLIC_BROWSE_GROUP);
+	root = sdp_list_append(0, &root_uuid);
+	sdp_set_browse_groups(record, root);
+
+	sdp_add_lang_attr(record);
+
+	sdp_uuid16_create(&hidkb_uuid, HID_SVCLASS_ID);
+	svclass_id = sdp_list_append(0, &hidkb_uuid);
+	sdp_set_service_classes(record, svclass_id);
+
+	sdp_uuid16_create(&profile.uuid, HID_PROFILE_ID);
+	profile.version = 0x0100;
+	pfseq = sdp_list_append(0, &profile);
+	sdp_set_profile_descs(record, pfseq);
+
+	/* protocols */
+	sdp_uuid16_create(&l2cap_uuid, L2CAP_UUID);
+	proto[1] = sdp_list_append(0, &l2cap_uuid);
+	psm = sdp_data_alloc(SDP_UINT16, &ctrl);
+	proto[1] = sdp_list_append(proto[1], psm);
+	apseq = sdp_list_append(0, proto[1]);
+
+	sdp_uuid16_create(&hidp_uuid, HIDP_UUID);
+	proto[2] = sdp_list_append(0, &hidp_uuid);
+	apseq = sdp_list_append(apseq, proto[2]);
+
+	aproto = sdp_list_append(0, apseq);
+	sdp_set_access_protos(record, aproto);
+
+	/* additional protocols */
+	proto[1] = sdp_list_append(0, &l2cap_uuid);
+	psm = sdp_data_alloc(SDP_UINT16, &intr);
+	proto[1] = sdp_list_append(proto[1], psm);
+	apseq = sdp_list_append(0, proto[1]);
+
+	sdp_uuid16_create(&hidp_uuid, HIDP_UUID);
+	proto[2] = sdp_list_append(0, &hidp_uuid);
+	apseq = sdp_list_append(apseq, proto[2]);
+
+	aproto = sdp_list_append(0, apseq);
+	sdp_set_add_access_protos(record, aproto);
+
+	sdp_set_info_attr(record, "Wireless Controller",
+						"Sony Computer Entertainment",
+						"Wireless Controller");
+
+	hid_release = 0x0100;
+	sdp_attr_add_new(record, SDP_ATTR_HID_DEVICE_RELEASE_NUMBER, SDP_UINT16,
+								&hid_release);
+
+	hid_parser = 0x0100;
+	sdp_attr_add_new(record, SDP_ATTR_HID_PARSER_VERSION, SDP_UINT16,
+								&hid_parser);
+
+	subclass = 0x00;
+	sdp_attr_add_new(record, SDP_ATTR_HID_DEVICE_SUBCLASS, SDP_UINT8,
+								&subclass);
+
+	country = 0x21;
+	sdp_attr_add_new(record, SDP_ATTR_HID_COUNTRY_CODE, SDP_UINT8,
+								&country);
+
+	virtual_cable = 0x01;
+	sdp_attr_add_new(record, SDP_ATTR_HID_VIRTUAL_CABLE, SDP_BOOL,
+								&virtual_cable);
+
+	reconnect = 0x01;
+	sdp_attr_add_new(record, SDP_ATTR_HID_RECONNECT_INITIATE, SDP_BOOL,
+								&reconnect);
+
+	dtds[0] = &dtd2;
+	values[0] = &hid_spec_type;
+	dtds[1] = &dtd_data;
+	values[1] = hid_spec;
+	leng[0] = 0;
+	leng[1] = sizeof(hid_spec);
+	hid_spec_lst = sdp_seq_alloc_with_length(dtds, values, leng, 2);
+	hid_spec_lst2 = sdp_data_alloc(SDP_SEQ8, hid_spec_lst);
+	sdp_attr_add(record, SDP_ATTR_HID_DESCRIPTOR_LIST, hid_spec_lst2);
+
+	dtds2[0] = &dtd;
+	values2[0] = &hid_attr_lang[0];
+	dtds2[1] = &dtd;
+	values2[1] = &hid_attr_lang[1];
+	lang_lst = sdp_seq_alloc(dtds2, values2, sizeof(hid_attr_lang) / 2);
+	lang_lst2 = sdp_data_alloc(SDP_SEQ8, lang_lst);
+	sdp_attr_add(record, SDP_ATTR_HID_LANG_ID_BASE_LIST, lang_lst2);
+
+	sdp_disable = 0x00;
+	sdp_attr_add_new(record, SDP_ATTR_HID_SDP_DISABLE, SDP_BOOL,
+								&sdp_disable);
+
+	battery = 0x01;
+	sdp_attr_add_new(record, SDP_ATTR_HID_BATTERY_POWER, SDP_BOOL,
+								&battery);
+
+	remote_wakeup = 0x01;
+	sdp_attr_add_new(record, SDP_ATTR_HID_REMOTE_WAKEUP, SDP_BOOL,
+								&remote_wakeup);
+
+	version = 0x0100;
+	sdp_attr_add_new(record, SDP_ATTR_HID_PROFILE_VERSION, SDP_UINT16,
+								&version);
+
+	timeout = 0x3e80;
+	sdp_attr_add_new(record, SDP_ATTR_HID_SUPERVISION_TIMEOUT, SDP_UINT16,
+								&timeout);
+
+	norm_connect = 0x00;
+	sdp_attr_add_new(record, SDP_ATTR_HID_NORMALLY_CONNECTABLE, SDP_BOOL,
+								&norm_connect);
+
+	boot_device = 0x00;
+	sdp_attr_add_new(record, SDP_ATTR_HID_BOOT_DEVICE, SDP_BOOL,
+								&boot_device);
+
+	return record;
+}
+
+
 static int sixaxis_get_device_bdaddr(int fd, bdaddr_t *bdaddr)
 {
 	uint8_t buf[18];
@@ -360,6 +527,9 @@ static void agent_auth_cb(DBusError *derr, void *user_data)
 	btd_device_set_trusted(closure->device, true);
 	btd_device_set_temporary(closure->device, false);
 
+	if (closure->type == CABLE_PAIRING_SIXAXIS)
+		btd_device_set_record(closure->device, HID_UUID, get_sixaxis_sdp_record());
+
 	ba2str(&closure->bdaddr, device_addr);
 	ba2str(&master_bdaddr, master_addr);
 	ba2str(adapter_bdaddr, adapter_addr);
-- 
2.14.3

Re: [PATCH 3/3] plugins/sixaxis: Provide DualShock 3 SDP record while adding new device

[Bastien Nocera]

On Tue, 2017-11-07 at 19:23 +0100, Bastien Nocera wrote:
> From: Szymon Janc <szymon.janc@codecoup.pl>
> 
> This allows to skip SDP search for DualShock 3 devices. Since some
> DS3 clones were reported to not provide any SDP record this should
> allow to operate them.

As Szymon mentioned on IRC, this won't work as the SDP library will not
be accessible on an installed bluez (I only tested on an uninstalled
setup).

Furthermore, it's not valgrind clean[1].

The plan is to revive an old patch of mine which had the SDP record as
a binary blob, in the same format as bluez stores on disk. I just need
to figure out where it's disappeared.

[1]:
==9283== Invalid read of size 2
==9283==    at 0x4C365EF: memmove (vg_replace_strmem.c:1258)
==9283==    by 0x48165F: sdp_data_alloc_with_length (sdp.c:443)
==9283==    by 0x481E70: sdp_copy_seq (sdp.c:1552)
==9283==    by 0x481E70: sdp_data_value (sdp.c:1536)
==9283==    by 0x481E2A: sdp_copy_seq (sdp.c:1551)
==9283==    by 0x481E2A: sdp_data_value (sdp.c:1536)
==9283==    by 0x482EDA: sdp_copy_attrlist (sdp.c:1573)
==9283==    by 0x482EDA: sdp_list_foreach (sdp_lib.h:69)
==9283==    by 0x482EDA: sdp_copy_record (sdp.c:1591)
==9283==    by 0x475A1B: update_record (device.c:4356)
==9283==    by 0x475A1B: update_bredr_services (device.c:4444)
==9283==    by 0x4762AA: btd_device_set_record (device.c:6092)
==9283==    by 0x88019A1: agent_auth_cb (sixaxis.c:531)
==9283==    by 0x45A037: agent_auth_cb (adapter.c:6050)
==9283==    by 0x45877E: simple_agent_reply (agent.c:376)
==9283==    by 0x53AB601: ??? (in /usr/lib64/libdbus-1.so.3.19.2)
==9283==    by 0x53AEF7E: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.19.2)
==9283==    by 0x4862FF: message_dispatch (mainloop.c:72)
==9283==    by 0x50CB596: ??? (in /usr/lib64/libglib-2.0.so.0.5400.1)
==9283==    by 0x50CEBB6: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.5400.1)
==9283==    by 0x50CEF5F: ??? (in /usr/lib64/libglib-2.0.so.0.5400.1)
==9283==    by 0x50CF271: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.5400.1)
==9283==    by 0x40BD98: main (main.c:770)
==9283==  Address 0x8dcdb04 is 148 bytes inside a block of size 149 alloc'd
==9283==    at 0x4C2FB6B: malloc (vg_replace_malloc.c:299)
==9283==    by 0x880B854: sdp_data_alloc_with_length (sdp.c:437)
==9283==    by 0x880BF8F: sdp_seq_alloc_with_length (sdp.c:526)
==9283==    by 0x880184B: get_sixaxis_sdp_record (sixaxis.c:211)
==9283==    by 0x880184B: agent_auth_cb (sixaxis.c:531)
==9283==    by 0x45A037: agent_auth_cb (adapter.c:6050)
==9283==    by 0x45877E: simple_agent_reply (agent.c:376)
==9283==    by 0x53AB601: ??? (in /usr/lib64/libdbus-1.so.3.19.2)
==9283==    by 0x53AEF7E: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.19.2)
==9283==    by 0x4862FF: message_dispatch (mainloop.c:72)
==9283==    by 0x50CB596: ??? (in /usr/lib64/libglib-2.0.so.0.5400.1)
==9283==    by 0x50CEBB6: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.5400.1)
==9283==    by 0x50CEF5F: ??? (in /usr/lib64/libglib-2.0.so.0.5400.1)
==9283==    by 0x50CF271: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.5400.1)
==9283==    by 0x40BD98: main (main.c:770)

> ---
>  Makefile.plugins  |   1 +
>  plugins/sixaxis.c | 170
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 171 insertions(+)
> 
> diff --git a/Makefile.plugins b/Makefile.plugins
> index 1f3b5b552..a1073d794 100644
> --- a/Makefile.plugins
> +++ b/Makefile.plugins
> @@ -109,4 +109,5 @@ plugins_sixaxis_la_SOURCES = plugins/sixaxis.c
>  plugins_sixaxis_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version \
>  						-no-undefined
> @UDEV_LIBS@
>  plugins_sixaxis_la_CFLAGS = $(AM_CFLAGS) -fvisibility=hidden
> @UDEV_CFLAGS@
> +plugins_sixaxis_la_LIBADD = lib/libbluetooth-internal.la
>  endif
> diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
> index 9d0fdb5d3..3ce26793a 100644
> --- a/plugins/sixaxis.c
> +++ b/plugins/sixaxis.c
> @@ -40,6 +40,8 @@
>  
>  #include "lib/bluetooth.h"
>  #include "lib/sdp.h"
> +#include "lib/sdp_lib.h"
> +#include "lib/sdp.h"
>  #include "lib/uuid.h"
>  
>  #include "src/adapter.h"
> @@ -85,6 +87,171 @@ static void auth_closure_destroy(struct
> authentication_closure *closure,
>  	g_free(closure);
>  }
>  
> +static sdp_record_t *get_sixaxis_sdp_record(void)
> +{
> +	sdp_record_t *record;
> +	uint16_t hid_release, hid_parser, version, timeout;
> +	uint8_t sdp_disable, battery, remote_wakeup, norm_connect,
> boot_device;
> +	uint8_t subclass, country, virtual_cable, reconnect;
> +	sdp_list_t *svclass_id, *pfseq, *apseq, *root;
> +	uuid_t root_uuid, hidkb_uuid, l2cap_uuid, hidp_uuid;
> +	sdp_profile_desc_t profile;
> +	sdp_list_t *aproto, *proto[3];
> +	sdp_data_t *psm, *lang_lst, *lang_lst2, *hid_spec_lst,
> *hid_spec_lst2;
> +	uint8_t dtd = SDP_UINT16;
> +	uint8_t dtd2 = SDP_UINT8;
> +	uint8_t dtd_data = SDP_TEXT_STR8;
> +	void *dtds[2];
> +	void *values[2];
> +	void *dtds2[2];
> +	void *values2[2];
> +	int leng[2];
> +	uint8_t hid_spec_type = 0x22;
> +	uint16_t hid_attr_lang[] = { 0x409, 0x100 };
> +	static const uint16_t ctrl = 0x11;
> +	static const uint16_t intr = 0x13;
> +	uint8_t hid_spec[] = {
> +		0x05, 0x01, 0x09, 0x04, 0xa1, 0x01, 0xa1, 0x02,
> 0x85, 0x01,
> +		0x75, 0x08, 0x95, 0x01, 0x15, 0x00, 0x26, 0xff,
> 0x00, 0x81,
> +		0x03, 0x75, 0x01, 0x95, 0x13, 0x15, 0x00, 0x25,
> 0x01, 0x35,
> +		0x00, 0x45, 0x01, 0x05, 0x09, 0x19, 0x01, 0x29,
> 0x13, 0x81,
> +		0x02, 0x75, 0x01, 0x95, 0x0d, 0x06, 0x00, 0xff,
> 0x81, 0x03,
> +		0x15, 0x00, 0x26, 0xff, 0x00, 0x05, 0x01, 0x09,
> 0x01, 0xa1,
> +		0x00, 0x75, 0x08, 0x95, 0x04, 0x35, 0x00, 0x46,
> 0xff, 0x00,
> +		0x09, 0x30, 0x09, 0x31, 0x09, 0x32, 0x09, 0x35,
> 0x81, 0x02,
> +		0xc0, 0x05, 0x01, 0x75, 0x08, 0x95, 0x27, 0x09,
> 0x01, 0x81,
> +		0x02, 0x75, 0x08, 0x95, 0x30, 0x09, 0x01, 0x91,
> 0x02, 0x75,
> +		0x08, 0x95, 0x30, 0x09, 0x01, 0xb1, 0x02, 0xc0,
> 0xa1, 0x02,
> +		0x85, 0x02, 0x75, 0x08, 0x95, 0x30, 0x09, 0x01,
> 0xb1, 0x02,
> +		0xc0, 0xa1, 0x02, 0x85, 0xee, 0x75, 0x08, 0x95,
> 0x30, 0x09,
> +		0x01, 0xb1, 0x02, 0xc0, 0xa1, 0x02, 0x85, 0xef,
> 0x75, 0x08,
> +		0x95, 0x30, 0x09, 0x01, 0xb1, 0x02, 0xc0, 0xc0, 0x00
> +	};
> +
> +	record = sdp_record_alloc();
> +	if (!record)
> +		return NULL;
> +
> +	sdp_uuid16_create(&root_uuid, PUBLIC_BROWSE_GROUP);
> +	root = sdp_list_append(0, &root_uuid);
> +	sdp_set_browse_groups(record, root);
> +
> +	sdp_add_lang_attr(record);
> +
> +	sdp_uuid16_create(&hidkb_uuid, HID_SVCLASS_ID);
> +	svclass_id = sdp_list_append(0, &hidkb_uuid);
> +	sdp_set_service_classes(record, svclass_id);
> +
> +	sdp_uuid16_create(&profile.uuid, HID_PROFILE_ID);
> +	profile.version = 0x0100;
> +	pfseq = sdp_list_append(0, &profile);
> +	sdp_set_profile_descs(record, pfseq);
> +
> +	/* protocols */
> +	sdp_uuid16_create(&l2cap_uuid, L2CAP_UUID);
> +	proto[1] = sdp_list_append(0, &l2cap_uuid);
> +	psm = sdp_data_alloc(SDP_UINT16, &ctrl);
> +	proto[1] = sdp_list_append(proto[1], psm);
> +	apseq = sdp_list_append(0, proto[1]);
> +
> +	sdp_uuid16_create(&hidp_uuid, HIDP_UUID);
> +	proto[2] = sdp_list_append(0, &hidp_uuid);
> +	apseq = sdp_list_append(apseq, proto[2]);
> +
> +	aproto = sdp_list_append(0, apseq);
> +	sdp_set_access_protos(record, aproto);
> +
> +	/* additional protocols */
> +	proto[1] = sdp_list_append(0, &l2cap_uuid);
> +	psm = sdp_data_alloc(SDP_UINT16, &intr);
> +	proto[1] = sdp_list_append(proto[1], psm);
> +	apseq = sdp_list_append(0, proto[1]);
> +
> +	sdp_uuid16_create(&hidp_uuid, HIDP_UUID);
> +	proto[2] = sdp_list_append(0, &hidp_uuid);
> +	apseq = sdp_list_append(apseq, proto[2]);
> +
> +	aproto = sdp_list_append(0, apseq);
> +	sdp_set_add_access_protos(record, aproto);
> +
> +	sdp_set_info_attr(record, "Wireless Controller",
> +						"Sony Computer
> Entertainment",
> +						"Wireless
> Controller");
> +
> +	hid_release = 0x0100;
> +	sdp_attr_add_new(record, SDP_ATTR_HID_DEVICE_RELEASE_NUMBER,
> SDP_UINT16,
> +								&hid
> _release);
> +
> +	hid_parser = 0x0100;
> +	sdp_attr_add_new(record, SDP_ATTR_HID_PARSER_VERSION,
> SDP_UINT16,
> +								&hid
> _parser);
> +
> +	subclass = 0x00;
> +	sdp_attr_add_new(record, SDP_ATTR_HID_DEVICE_SUBCLASS,
> SDP_UINT8,
> +								&sub
> class);
> +
> +	country = 0x21;
> +	sdp_attr_add_new(record, SDP_ATTR_HID_COUNTRY_CODE,
> SDP_UINT8,
> +								&cou
> ntry);
> +
> +	virtual_cable = 0x01;
> +	sdp_attr_add_new(record, SDP_ATTR_HID_VIRTUAL_CABLE,
> SDP_BOOL,
> +								&vir
> tual_cable);
> +
> +	reconnect = 0x01;
> +	sdp_attr_add_new(record, SDP_ATTR_HID_RECONNECT_INITIATE,
> SDP_BOOL,
> +								&rec
> onnect);
> +
> +	dtds[0] = &dtd2;
> +	values[0] = &hid_spec_type;
> +	dtds[1] = &dtd_data;
> +	values[1] = hid_spec;
> +	leng[0] = 0;
> +	leng[1] = sizeof(hid_spec);
> +	hid_spec_lst = sdp_seq_alloc_with_length(dtds, values, leng,
> 2);
> +	hid_spec_lst2 = sdp_data_alloc(SDP_SEQ8, hid_spec_lst);
> +	sdp_attr_add(record, SDP_ATTR_HID_DESCRIPTOR_LIST,
> hid_spec_lst2);
> +
> +	dtds2[0] = &dtd;
> +	values2[0] = &hid_attr_lang[0];
> +	dtds2[1] = &dtd;
> +	values2[1] = &hid_attr_lang[1];
> +	lang_lst = sdp_seq_alloc(dtds2, values2,
> sizeof(hid_attr_lang) / 2);
> +	lang_lst2 = sdp_data_alloc(SDP_SEQ8, lang_lst);
> +	sdp_attr_add(record, SDP_ATTR_HID_LANG_ID_BASE_LIST,
> lang_lst2);
> +
> +	sdp_disable = 0x00;
> +	sdp_attr_add_new(record, SDP_ATTR_HID_SDP_DISABLE, SDP_BOOL,
> +								&sdp
> _disable);
> +
> +	battery = 0x01;
> +	sdp_attr_add_new(record, SDP_ATTR_HID_BATTERY_POWER,
> SDP_BOOL,
> +								&bat
> tery);
> +
> +	remote_wakeup = 0x01;
> +	sdp_attr_add_new(record, SDP_ATTR_HID_REMOTE_WAKEUP,
> SDP_BOOL,
> +								&rem
> ote_wakeup);
> +
> +	version = 0x0100;
> +	sdp_attr_add_new(record, SDP_ATTR_HID_PROFILE_VERSION,
> SDP_UINT16,
> +								&ver
> sion);
> +
> +	timeout = 0x3e80;
> +	sdp_attr_add_new(record, SDP_ATTR_HID_SUPERVISION_TIMEOUT,
> SDP_UINT16,
> +								&tim
> eout);
> +
> +	norm_connect = 0x00;
> +	sdp_attr_add_new(record, SDP_ATTR_HID_NORMALLY_CONNECTABLE,
> SDP_BOOL,
> +								&nor
> m_connect);
> +
> +	boot_device = 0x00;
> +	sdp_attr_add_new(record, SDP_ATTR_HID_BOOT_DEVICE, SDP_BOOL,
> +								&boo
> t_device);
> +
> +	return record;
> +}
> +
> +
>  static int sixaxis_get_device_bdaddr(int fd, bdaddr_t *bdaddr)
>  {
>  	uint8_t buf[18];
> @@ -360,6 +527,9 @@ static void agent_auth_cb(DBusError *derr, void
> *user_data)
>  	btd_device_set_trusted(closure->device, true);
>  	btd_device_set_temporary(closure->device, false);
>  
> +	if (closure->type == CABLE_PAIRING_SIXAXIS)
> +		btd_device_set_record(closure->device, HID_UUID,
> get_sixaxis_sdp_record());
> +
>  	ba2str(&closure->bdaddr, device_addr);
>  	ba2str(&master_bdaddr, master_addr);
>  	ba2str(adapter_bdaddr, adapter_addr);

Re: [PATCH 1/3] plugins/sixaxis: Fix cable pairing not working

[Johan Hedberg]

Hi Bastien,

On Tue, Nov 07, 2017, Bastien Nocera wrote:
> In 6c467e9, get_pairing_type() was modified to return a structure that's
> allocated on the stack. While not a problem for the direct callers,
> as the function is defined as "inline", function that expected to return
> this structure themselves would fail, as the stack would be trampled
> upon on function exit.

The devices array inside get_pairing() is declared as static, so I don't
see how this can be on the stack? Are these the structures you're
referring to? That said, it seems weird to have an inline function
declare a static variable, since then every single caller of the
function would cause another static variable to be allocated, right? It
seems to me that if the function is still desired to be inline then the
cable_pairing array should at least be moved into some c-file, so that
it only gets created once. Perhaps a better solution is however to stop
having get_pairing() as inline to begin with?

Johan

Re: [PATCH 1/3] plugins/sixaxis: Fix cable pairing not working

[Bastien Nocera]

On Wed, 2017-11-08 at 09:49 +0200, Johan Hedberg wrote:
> Hi Bastien,
> 
> On Tue, Nov 07, 2017, Bastien Nocera wrote:
> > In 6c467e9, get_pairing_type() was modified to return a structure
> > that's
> > allocated on the stack. While not a problem for the direct callers,
> > as the function is defined as "inline", function that expected to
> > return
> > this structure themselves would fail, as the stack would be
> > trampled
> > upon on function exit.
> 
> The devices array inside get_pairing() is declared as static, so I
> don't
> see how this can be on the stack?

Looks like I chased down a wild goose, and that while it "fixed" my
problem, the root cause was something completely different. There's a
new patch coming up.

>  Are these the structures you're
> referring to? That said, it seems weird to have an inline function
> declare a static variable, since then every single caller of the
> function would cause another static variable to be allocated, right?
> It
> seems to me that if the function is still desired to be inline then
> the
> cable_pairing array should at least be moved into some c-file, so
> that
> it only gets created once. Perhaps a better solution is however to
> stop
> having get_pairing() as inline to begin with?

The reason to having this inline in a shared header was explained in
the commit message I believe. We need this code both in the
core/builtin input plugin and in the sixaxis plugin. We'd either need
to make this function public in the core, meaning in the external
plugin API, or copy/pasted between the 2 parts, both of which are
inconvenient.

If the function wasn't inline, we'd have 2 functions with the same
name, and the external plugin, sixaxis in this case, would fail to
load.