Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH BlueZ 02/16] android/avctp: Make handler return ssize_t
From: Luiz Augusto von Dentz @ 2014-03-02 18:48 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1393786109-6554-1-git-send-email-luiz.dentz@gmail.com>

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This makes possible to return errors such as -EAGAIN to indicate the
request would block and a response will be sent asynchronously.
---
 android/avctp.c     | 17 ++++++++++++-----
 android/avctp.h     |  2 +-
 android/avrcp-lib.c |  2 +-
 unit/test-avctp.c   |  2 +-
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/android/avctp.c b/android/avctp.c
index bc1bd80..aa5abc9 100644
--- a/android/avctp.c
+++ b/android/avctp.c
@@ -311,7 +311,7 @@ static gboolean auto_release(gpointer user_data)
 	return FALSE;
 }
 
-static size_t handle_panel_passthrough(struct avctp *session,
+static ssize_t handle_panel_passthrough(struct avctp *session,
 					uint8_t transaction, uint8_t *code,
 					uint8_t *subunit, uint8_t *operands,
 					size_t operand_count, void *user_data)
@@ -402,7 +402,7 @@ done:
 	return operand_count;
 }
 
-static size_t handle_unit_info(struct avctp *session,
+static ssize_t handle_unit_info(struct avctp *session,
 					uint8_t transaction, uint8_t *code,
 					uint8_t *subunit, uint8_t *operands,
 					size_t operand_count, void *user_data)
@@ -428,7 +428,7 @@ static size_t handle_unit_info(struct avctp *session,
 	return operand_count;
 }
 
-static size_t handle_subunit_info(struct avctp *session,
+static ssize_t handle_subunit_info(struct avctp *session,
 					uint8_t transaction, uint8_t *code,
 					uint8_t *subunit, uint8_t *operands,
 					size_t operand_count, void *user_data)
@@ -849,8 +849,9 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
 	uint8_t *operands, code, subunit;
 	struct avctp_header *avctp;
 	struct avc_header *avc;
-	int ret, packet_size, operand_count, sock;
+	int packet_size, operand_count, sock;
 	struct avctp_pdu_handler *handler;
+	ssize_t ret;
 
 	if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
 		goto failed;
@@ -910,10 +911,16 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond, gpointer data)
 	code = avc->code;
 	subunit = avc->subunit_type;
 
-	packet_size += handler->cb(session, avctp->transaction, &code,
+	ret = handler->cb(session, avctp->transaction, &code,
 					&subunit, operands, operand_count,
 					handler->user_data);
+	if (ret < 0) {
+		if (ret == -EAGAIN)
+			return TRUE;
+		goto failed;
+	}
 
+	packet_size += ret;
 	avc->code = code;
 	avc->subunit_type = subunit;
 
diff --git a/android/avctp.h b/android/avctp.h
index dfa0ca6..2f419a2 100644
--- a/android/avctp.h
+++ b/android/avctp.h
@@ -113,7 +113,7 @@ struct avctp;
 typedef bool (*avctp_passthrough_cb) (struct avctp *session,
 					uint8_t op, bool pressed,
 					void *user_data);
-typedef size_t (*avctp_control_pdu_cb) (struct avctp *session,
+typedef ssize_t (*avctp_control_pdu_cb) (struct avctp *session,
 					uint8_t transaction, uint8_t *code,
 					uint8_t *subunit, uint8_t *operands,
 					size_t operand_count, void *user_data);
diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index 6fed825..cdad6ac 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -115,7 +115,7 @@ void avrcp_shutdown(struct avrcp *session)
 	g_free(session);
 }
 
-static size_t handle_vendordep_pdu(struct avctp *conn, uint8_t transaction,
+static ssize_t handle_vendordep_pdu(struct avctp *conn, uint8_t transaction,
 					uint8_t *code, uint8_t *subunit,
 					uint8_t *operands, size_t operand_count,
 					void *user_data)
diff --git a/unit/test-avctp.c b/unit/test-avctp.c
index be1dfd7..0759731 100644
--- a/unit/test-avctp.c
+++ b/unit/test-avctp.c
@@ -227,7 +227,7 @@ static void execute_context(struct context *context)
 	destroy_context(context);
 }
 
-static size_t handler(struct avctp *session,
+static ssize_t handler(struct avctp *session,
 					uint8_t transaction, uint8_t *code,
 					uint8_t *subunit, uint8_t *operands,
 					size_t operand_count, void *user_data)
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH BlueZ 01/16] android/avrcp-lib: Add avrcp_send function
From: Luiz Augusto von Dentz @ 2014-03-02 18:48 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds avrcp_send function which can be used to response to
outstanding requests.
---
 android/avrcp-lib.c | 27 +++++++++++++++++++++++++++
 android/avrcp-lib.h |  4 +++-
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index 34eea66..6fed825 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -253,6 +253,33 @@ int avrcp_init_uinput(struct avrcp *session, const char *name,
 	return avctp_init_uinput(session->conn, name, address);
 }
 
+int avrcp_send(struct avrcp *session, uint8_t transaction, uint8_t code,
+					uint8_t subunit, uint8_t pdu_id,
+					uint8_t *params, size_t params_len)
+{
+	struct avrcp_header *pdu = (void *) session->tx_buf;
+	size_t len = sizeof(*pdu);
+
+	memset(pdu, 0, len);
+
+	hton24(pdu->company_id, IEEEID_BTSIG);
+	pdu->pdu_id = pdu_id;
+	pdu->packet_type = AVRCP_PACKET_TYPE_SINGLE;
+
+	if (params_len > 0) {
+		len += params_len;
+
+		if (len > session->tx_mtu)
+			return -ENOBUFS;
+
+		memcpy(pdu->params, params, params_len);
+		pdu->params_len = htons(params_len);
+	}
+
+	return avctp_send_vendordep(session->conn, transaction, code, subunit,
+							session->tx_buf, len);
+}
+
 static int avrcp_send_req(struct avrcp *session, uint8_t code, uint8_t subunit,
 					uint8_t pdu_id, uint8_t *params,
 					size_t params_len, avctp_rsp_cb func,
diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
index 0407cb4..a33bdfe 100644
--- a/android/avrcp-lib.h
+++ b/android/avrcp-lib.h
@@ -102,7 +102,9 @@ void avrcp_set_passthrough_handlers(struct avrcp *session,
 			void *user_data);
 int avrcp_init_uinput(struct avrcp *session, const char *name,
 							const char *address);
-
+int avrcp_send(struct avrcp *session, uint8_t transaction, uint8_t code,
+					uint8_t subunit, uint8_t pdu_id,
+					uint8_t *params, size_t params_len);
 int avrcp_get_capabilities(struct avrcp *session, uint8_t param,
 					avctp_rsp_cb func, void *user_data);
 int avrcp_list_player_attributes(struct avrcp *session, avctp_rsp_cb func,
-- 
1.8.5.3


^ permalink raw reply related

* Re: [PATCH v2 1/7] android: Add flags parameter to register service command
From: Marcel Holtmann @ 2014-03-02 17:56 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1615014.kasN2TjSTh@leonov>

Hi Szymon,

>>> This will allow to configure daemon services.
>>> ---
>>> android/hal-a2dp.c      | 1 +
>>> android/hal-avrcp.c     | 1 +
>>> android/hal-bluetooth.c | 1 +
>>> android/hal-handsfree.c | 1 +
>>> android/hal-hidhost.c   | 1 +
>>> android/hal-ipc-api.txt | 7 +++++++
>> 
>> please do not mix code with API docs updates.
> 
> OK.
> 
>> 
>>> android/hal-msg.h       | 1 +
>>> android/hal-pan.c       | 1 +
>>> 8 files changed, 14 insertions(+)
>>> 
>>> diff --git a/android/hal-a2dp.c b/android/hal-a2dp.c
>>> index c898995..87d89a2 100644
>>> --- a/android/hal-a2dp.c
>>> +++ b/android/hal-a2dp.c
>>> @@ -109,6 +109,7 @@ static bt_status_t init(btav_callbacks_t *callbacks)
>>> 
>>> 				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
>>> 	
>>> 	cmd.service_id = HAL_SERVICE_ID_A2DP;
>>> 
>>> +	cmd.flags = 0;
>>> 
>>> 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
>>> 	
>>> 					sizeof(cmd), &cmd, 0, NULL, NULL);
>>> 
>>> diff --git a/android/hal-avrcp.c b/android/hal-avrcp.c
>>> index 46e25a0..4907f4a 100644
>>> --- a/android/hal-avrcp.c
>>> +++ b/android/hal-avrcp.c
>>> @@ -220,6 +220,7 @@ static bt_status_t init(btrc_callbacks_t *callbacks)
>>> 
>>> 				sizeof(ev_handlers) / sizeof(ev_handlers[0]));
>>> 	
>>> 	cmd.service_id = HAL_SERVICE_ID_AVRCP;
>>> 
>>> +	cmd.flags = 0;
>>> 
>>> 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
>>> 	
>>> 					sizeof(cmd), &cmd, 0, NULL, NULL);
>>> 
>>> diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
>>> index 6871f5d..4b31ff1 100644
>>> --- a/android/hal-bluetooth.c
>>> +++ b/android/hal-bluetooth.c
>>> @@ -442,6 +442,7 @@ static int init(bt_callbacks_t *callbacks)
>>> 
>>> 	}
>>> 	
>>> 	cmd.service_id = HAL_SERVICE_ID_SOCKET;
>>> 
>>> +	cmd.flags = 0;
>>> 
>>> 	status = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
>>> 	
>>> 					sizeof(cmd), &cmd, NULL, NULL, NULL);
>>> 
>>> diff --git a/android/hal-handsfree.c b/android/hal-handsfree.c
>>> index 1b150c3..422f52c 100644
>>> --- a/android/hal-handsfree.c
>>> +++ b/android/hal-handsfree.c
>>> @@ -212,6 +212,7 @@ static bt_status_t init(bthf_callbacks_t *callbacks)
>>> 
>>> 				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
>>> 	
>>> 	cmd.service_id = HAL_SERVICE_ID_HANDSFREE;
>>> 
>>> +	cmd.flags = 0;
>>> 
>>> 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
>>> 	
>>> 					sizeof(cmd), &cmd, 0, NULL, NULL);
>>> 
>>> diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
>>> index c758d2a..fe1b4df 100644
>>> --- a/android/hal-hidhost.c
>>> +++ b/android/hal-hidhost.c
>>> @@ -354,6 +354,7 @@ static bt_status_t init(bthh_callbacks_t *callbacks)
>>> 
>>> 				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
>>> 	
>>> 	cmd.service_id = HAL_SERVICE_ID_HIDHOST;
>>> 
>>> +	cmd.flags = 0;
>>> 
>>> 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
>>> 	
>>> 					sizeof(cmd), &cmd, 0, NULL, NULL);
>>> 
>>> diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
>>> index 1a19c80..f68324d 100644
>>> --- a/android/hal-ipc-api.txt
>>> +++ b/android/hal-ipc-api.txt
>>> @@ -124,12 +124,16 @@ Core Service (ID 0)
>>> 
>>> 	Opcode 0x01 - Register module command/response
>>> 	
>>> 		Command parameters: Service id (1 octet)
>>> 
>>> +		                    Flags (1 octet)
>> 
>> We might better go with a Flags (4 octets) here. Otherwise you only have 8
>> options which might be enough or not. Your choice.
>> 
>> Or instead of a bit mask, we do a Configuration (1 octet) were we have the
>> default configuration with 0x00 and then alternatives with all other
>> values. That might be actually a bit better.
> 
> I'll see how it goes with configuration options, but flags could be used also 
> to enable eg. debug options which might be clumsy without flags.

what debug options are you thinking about?

I also think we should just call it Mode (1 octet). Keep it simple.

>>> 		Response parameters: <none>
>>> 		
>>> 		In case a command is sent for an undeclared service ID, it will
>>> 		be rejected. Also there will be no notifications for undeclared
>>> 		service ID.
>>> 
>>> +		Flags parameter values should be defined as needed by
>>> +		respective services.
>>> +
>>> 
>>> 		In case of an error, the error response will be returned.
>>> 	
>>> 	Opcode 0x02 - Unregister module command/response
>>> 
>>> @@ -749,6 +753,9 @@ Bluetooth Handsfree HAL (ID 5)
>>> 
>>> Android HAL name: "handsfree" (BT_PROFILE_HANDSFREE_ID)
>>> 
>>> +	Service flags:               0x01 = Disable HSP AG
>>> +	                             0x02 = Disable HFP AG
>>> +
>> 
>> If you do not want HSP and HFP, then there is no point in registering
>> handsfree ID at all. I am tending currently towards the configuration
>> option.
> 
>> 	0x00 = Provide Handsfree profile
>> 	0x01 = Provide Headset profile
>> 
>> There is really no point in providing both? Or is there a case where you
>> would provide both?
> 
> Android on phones provides both. On tablets HSP only. So we could go with
> 0x00 = Provide HFP and HSP
> 0x01 = Provide HSP only
> 
> or just with single flag to disable HFP.

Do we really want to expose HSP gateway when HFP is available. I am not even sure this makes sense. With Bluedroid, are they using the same RFCOMM channel or how does that actually work. You can not connect them at the same time anyway.

I would really go for the simple case of either Handsfree or Headset profile, but not both.

Regards

Marcel


^ permalink raw reply

* Re: [PATCH BlueZ] monitor: Validate HCI event/command parameter length when parsing
From: Johan Hedberg @ 2014-03-02 16:39 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1393701808-27186-1-git-send-email-anderson.lizardo@openbossa.org>

Hi Lizardo,

On Sat, Mar 01, 2014, Anderson Lizardo wrote:
> Print an error message if the parameter length for HCI commands/events
> does not match the actual remaining packet size.
> 
> This mainly avoids using garbage bytes when parsing corrupted packets.
> The check was inspired on those used when parsing SCO/ACL packets.
> ---
>  monitor/packet.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Applied. Thanks.

Johan

^ permalink raw reply

* Re: Passive scanning of iBeacons results in a "Data Buffer Overflow"
From: Adam Warski @ 2014-03-02 15:07 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: BlueZ development
In-Reply-To: <CAJdJm_PN3xGjdNOtr3o-J0SJ8N6MJXBChUCwOWOZBrdFKDBfGA@mail.gmail.com>

Thanks for the analysis :) I guess I’m more of a beginner in the area and moving slower.
I also tried with bluez 5.13 (as .15 contains some LE changes), but same thing happens.

> Maybe you already did your own investigation, but here goes my findings:
> 
> When opening the dump file on a hex editor, we can see that this first
> bogus packet starts (skipping BTSnoop's own header) at offset 0x45a4
> (with bytes "3E 2A 02 01 ..."). At offset 0x45b4 we can see that the
> packet becomes truncated by missing these bytes (based on previous
> correct packets):
> 
> 1A FF 4C 00 02 15 B9 40 7F 30 F5 F8 46 6E AF F9
> 
> From now on, the packets become garbage because they are "shifted" due
> to these missing bytes.
> 
> I think we can rule out any problems on the hcidump/btmon tools (as
> the raw logs come directly from the kernel), so I see two
> possibilities here:
> 
> 1) There might be a bug in HCI packet reassembly logic (either in
> hci_core.c or btusb.c).
> 
> 2) (most likely) your BT dongle may be broken (i.e. generating
> truncated USB packets from time to time)

The root cause seem to be the “--duplicates --passive” flags. According to the BLE spec the duplicates are filtered out in the Link Layer part. Do I understand correctly that that’s on the hardware side?

If I just run “hcitool lescan --passive”, I see the advertisement only once, and nothing else happens - which includes no truncated packets. So the --duplicates flag seems to be the culprit.

> So my suggestion would be to try with a different dongle, preferably
> of a different chipset vendor. Note that "TP-Link" is just the brand
> name, you can confirm the chipset using lsusb or (even better)
> "hciconfig hci0 -a" (check "Manufacturer" field).

I messed up the model number, looked at the wrong dongle, TP-Link is the WiFi one ;)
The bluetooth one is an IOGear GBU521 (http://www.iogear.com/product/GBU521/).

I don’t have another dongle right now (I tried a different IOGear, but same thing), but I’ll try setting a VM on my laptop/getting a different USB dongle.

> Can you also provide the "lsusb" and "hciconfig hci0 -a" output (feel
> free to modify the BT address for privacy reasons)? I think it is
> important to have this archived so similar reports can be traced to
> this hardware and firmware.

Sure:
$ sudo hciconfig hci0 -a
hci0:	Type: BR/EDR  Bus: USB
	BD Address: 00:01:81:C6:E8:82  ACL MTU: 1021:8  SCO MTU: 64:1
	UP RUNNING
	RX bytes:547 acl:0 sco:0 events:27 errors:0
	TX bytes:384 acl:0 sco:0 commands:27 errors:0
	Features: 0xbf 0xfe 0xcf 0xfe 0xdb 0xff 0x7b 0x87
	Packet type: DM1 DM3 DM5 DH1 DH3 DH5 HV1 HV2 HV3
	Link policy: RSWITCH SNIFF
	Link mode: SLAVE ACCEPT
	Name: 'BCM20702A'
	Class: 0x000000
	Service Classes: Unspecified
	Device Class: Miscellaneous,
	HCI Version: 4.0 (0x6)  Revision: 0x1000
	LMP Version: 4.0 (0x6)  Subversion: 0x220e
	Manufacturer: Broadcom Corporation (15)

$ lsusb
Bus 001 Device 002: ID 0424:9512 Standard Microsystems Corp.
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 001 Device 003: ID 0424:ec00 Standard Microsystems Corp.
Bus 001 Device 004: ID 0bda:8179 Realtek Semiconductor Corp.
Bus 001 Device 005: ID 0a5c:21e8 Broadcom Corp.

> Another good source of information is the kernel debug logs. Collect
> it by enabling dynamic debugging:
> 
> # dmesg -c  # just to clear the kernel log buffer
> # echo 'module bluetooth +pf' | cat > /sys/kernel/debug/dynamic_debug/control
> # echo 'module btusb +pf' | cat > /sys/kernel/debug/dynamic_debug/control
> # btmon -w lescan.dump # So we can correlate the kernel log with the
> actual packets
> # hcitool lescan --duplicates # Run on another terminal, until packets
> become corrupt
> # dmesg > dmesg.txt
> # echo 'module bluetooth -pf' | cat > /sys/kernel/debug/dynamic_debug/control
> # echo 'module btusb -pf' | cat > /sys/kernel/debug/dynamic_debug/control
> # gzip dmesg.txt (if it is too big)
> 
> Then send dmesg.txt.gz + lescan.dump to the list. You may want to mask
> out your BT address on the log as well.

I mounted debugfs but as I don’t have dynamic_debug I guess I have to recompile the kernel with the appropriate flag. This will take a while ;)

Adam

-- 
Adam Warski

http://twitter.com/#!/adamwarski
http://www.softwaremill.com
http://www.warski.org


^ permalink raw reply

* Re: [PATCH v2 1/7] android: Add flags parameter to register service command
From: Szymon Janc @ 2014-03-01 22:25 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <A4A690E8-D8B1-4459-99B6-24A5D49B253F@holtmann.org>

Hi Marcel,

On Saturday 01 of March 2014 13:45:15 Marcel Holtmann wrote:
> Hi Szymon,
> 
> > This will allow to configure daemon services.
> > ---
> > android/hal-a2dp.c      | 1 +
> > android/hal-avrcp.c     | 1 +
> > android/hal-bluetooth.c | 1 +
> > android/hal-handsfree.c | 1 +
> > android/hal-hidhost.c   | 1 +
> > android/hal-ipc-api.txt | 7 +++++++
> 
> please do not mix code with API docs updates.

OK.

> 
> > android/hal-msg.h       | 1 +
> > android/hal-pan.c       | 1 +
> > 8 files changed, 14 insertions(+)
> > 
> > diff --git a/android/hal-a2dp.c b/android/hal-a2dp.c
> > index c898995..87d89a2 100644
> > --- a/android/hal-a2dp.c
> > +++ b/android/hal-a2dp.c
> > @@ -109,6 +109,7 @@ static bt_status_t init(btav_callbacks_t *callbacks)
> > 
> > 				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
> > 	
> > 	cmd.service_id = HAL_SERVICE_ID_A2DP;
> > 
> > +	cmd.flags = 0;
> > 
> > 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> > 	
> > 					sizeof(cmd), &cmd, 0, NULL, NULL);
> > 
> > diff --git a/android/hal-avrcp.c b/android/hal-avrcp.c
> > index 46e25a0..4907f4a 100644
> > --- a/android/hal-avrcp.c
> > +++ b/android/hal-avrcp.c
> > @@ -220,6 +220,7 @@ static bt_status_t init(btrc_callbacks_t *callbacks)
> > 
> > 				sizeof(ev_handlers) / sizeof(ev_handlers[0]));
> > 	
> > 	cmd.service_id = HAL_SERVICE_ID_AVRCP;
> > 
> > +	cmd.flags = 0;
> > 
> > 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> > 	
> > 					sizeof(cmd), &cmd, 0, NULL, NULL);
> > 
> > diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> > index 6871f5d..4b31ff1 100644
> > --- a/android/hal-bluetooth.c
> > +++ b/android/hal-bluetooth.c
> > @@ -442,6 +442,7 @@ static int init(bt_callbacks_t *callbacks)
> > 
> > 	}
> > 	
> > 	cmd.service_id = HAL_SERVICE_ID_SOCKET;
> > 
> > +	cmd.flags = 0;
> > 
> > 	status = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> > 	
> > 					sizeof(cmd), &cmd, NULL, NULL, NULL);
> > 
> > diff --git a/android/hal-handsfree.c b/android/hal-handsfree.c
> > index 1b150c3..422f52c 100644
> > --- a/android/hal-handsfree.c
> > +++ b/android/hal-handsfree.c
> > @@ -212,6 +212,7 @@ static bt_status_t init(bthf_callbacks_t *callbacks)
> > 
> > 				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
> > 	
> > 	cmd.service_id = HAL_SERVICE_ID_HANDSFREE;
> > 
> > +	cmd.flags = 0;
> > 
> > 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> > 	
> > 					sizeof(cmd), &cmd, 0, NULL, NULL);
> > 
> > diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
> > index c758d2a..fe1b4df 100644
> > --- a/android/hal-hidhost.c
> > +++ b/android/hal-hidhost.c
> > @@ -354,6 +354,7 @@ static bt_status_t init(bthh_callbacks_t *callbacks)
> > 
> > 				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
> > 	
> > 	cmd.service_id = HAL_SERVICE_ID_HIDHOST;
> > 
> > +	cmd.flags = 0;
> > 
> > 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> > 	
> > 					sizeof(cmd), &cmd, 0, NULL, NULL);
> > 
> > diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
> > index 1a19c80..f68324d 100644
> > --- a/android/hal-ipc-api.txt
> > +++ b/android/hal-ipc-api.txt
> > @@ -124,12 +124,16 @@ Core Service (ID 0)
> > 
> > 	Opcode 0x01 - Register module command/response
> > 	
> > 		Command parameters: Service id (1 octet)
> > 
> > +		                    Flags (1 octet)
> 
> We might better go with a Flags (4 octets) here. Otherwise you only have 8
> options which might be enough or not. Your choice.
> 
> Or instead of a bit mask, we do a Configuration (1 octet) were we have the
> default configuration with 0x00 and then alternatives with all other
> values. That might be actually a bit better.

I'll see how it goes with configuration options, but flags could be used also 
to enable eg. debug options which might be clumsy without flags.

> > 		Response parameters: <none>
> > 		
> > 		In case a command is sent for an undeclared service ID, it will
> > 		be rejected. Also there will be no notifications for undeclared
> > 		service ID.
> > 
> > +		Flags parameter values should be defined as needed by
> > +		respective services.
> > +
> > 
> > 		In case of an error, the error response will be returned.
> > 	
> > 	Opcode 0x02 - Unregister module command/response
> > 
> > @@ -749,6 +753,9 @@ Bluetooth Handsfree HAL (ID 5)
> > 
> > Android HAL name: "handsfree" (BT_PROFILE_HANDSFREE_ID)
> > 
> > +	Service flags:               0x01 = Disable HSP AG
> > +	                             0x02 = Disable HFP AG
> > +
> 
> If you do not want HSP and HFP, then there is no point in registering
> handsfree ID at all. I am tending currently towards the configuration
> option.

> 	0x00 = Provide Handsfree profile
> 	0x01 = Provide Headset profile
> 
> There is really no point in providing both? Or is there a case where you
> would provide both?

Android on phones provides both. On tablets HSP only. So we could go with
0x00 = Provide HFP and HSP
0x01 = Provide HSP only

or just with single flag to disable HFP.

-- 
BR
Szymon Janc

^ permalink raw reply

* Re: [PATCH v2 1/7] android: Add flags parameter to register service command
From: Marcel Holtmann @ 2014-03-01 21:45 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1393709289-6395-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

> This will allow to configure daemon services.
> ---
> android/hal-a2dp.c      | 1 +
> android/hal-avrcp.c     | 1 +
> android/hal-bluetooth.c | 1 +
> android/hal-handsfree.c | 1 +
> android/hal-hidhost.c   | 1 +
> android/hal-ipc-api.txt | 7 +++++++

please do not mix code with API docs updates.

> android/hal-msg.h       | 1 +
> android/hal-pan.c       | 1 +
> 8 files changed, 14 insertions(+)
> 
> diff --git a/android/hal-a2dp.c b/android/hal-a2dp.c
> index c898995..87d89a2 100644
> --- a/android/hal-a2dp.c
> +++ b/android/hal-a2dp.c
> @@ -109,6 +109,7 @@ static bt_status_t init(btav_callbacks_t *callbacks)
> 				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
> 
> 	cmd.service_id = HAL_SERVICE_ID_A2DP;
> +	cmd.flags = 0;
> 
> 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> 					sizeof(cmd), &cmd, 0, NULL, NULL);
> diff --git a/android/hal-avrcp.c b/android/hal-avrcp.c
> index 46e25a0..4907f4a 100644
> --- a/android/hal-avrcp.c
> +++ b/android/hal-avrcp.c
> @@ -220,6 +220,7 @@ static bt_status_t init(btrc_callbacks_t *callbacks)
> 				sizeof(ev_handlers) / sizeof(ev_handlers[0]));
> 
> 	cmd.service_id = HAL_SERVICE_ID_AVRCP;
> +	cmd.flags = 0;
> 
> 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> 					sizeof(cmd), &cmd, 0, NULL, NULL);
> diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> index 6871f5d..4b31ff1 100644
> --- a/android/hal-bluetooth.c
> +++ b/android/hal-bluetooth.c
> @@ -442,6 +442,7 @@ static int init(bt_callbacks_t *callbacks)
> 	}
> 
> 	cmd.service_id = HAL_SERVICE_ID_SOCKET;
> +	cmd.flags = 0;
> 
> 	status = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> 					sizeof(cmd), &cmd, NULL, NULL, NULL);
> diff --git a/android/hal-handsfree.c b/android/hal-handsfree.c
> index 1b150c3..422f52c 100644
> --- a/android/hal-handsfree.c
> +++ b/android/hal-handsfree.c
> @@ -212,6 +212,7 @@ static bt_status_t init(bthf_callbacks_t *callbacks)
> 				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
> 
> 	cmd.service_id = HAL_SERVICE_ID_HANDSFREE;
> +	cmd.flags = 0;
> 
> 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> 					sizeof(cmd), &cmd, 0, NULL, NULL);
> diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
> index c758d2a..fe1b4df 100644
> --- a/android/hal-hidhost.c
> +++ b/android/hal-hidhost.c
> @@ -354,6 +354,7 @@ static bt_status_t init(bthh_callbacks_t *callbacks)
> 				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
> 
> 	cmd.service_id = HAL_SERVICE_ID_HIDHOST;
> +	cmd.flags = 0;
> 
> 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> 					sizeof(cmd), &cmd, 0, NULL, NULL);
> diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
> index 1a19c80..f68324d 100644
> --- a/android/hal-ipc-api.txt
> +++ b/android/hal-ipc-api.txt
> @@ -124,12 +124,16 @@ Core Service (ID 0)
> 	Opcode 0x01 - Register module command/response
> 
> 		Command parameters: Service id (1 octet)
> +		                    Flags (1 octet)

We might better go with a Flags (4 octets) here. Otherwise you only have 8 options which might be enough or not. Your choice.

Or instead of a bit mask, we do a Configuration (1 octet) were we have the default configuration with 0x00 and then alternatives with all other values. That might be actually a bit better.

> 		Response parameters: <none>
> 
> 		In case a command is sent for an undeclared service ID, it will
> 		be rejected. Also there will be no notifications for undeclared
> 		service ID.
> 
> +		Flags parameter values should be defined as needed by
> +		respective services.
> +
> 		In case of an error, the error response will be returned.
> 
> 	Opcode 0x02 - Unregister module command/response
> @@ -749,6 +753,9 @@ Bluetooth Handsfree HAL (ID 5)
> 
> Android HAL name: "handsfree" (BT_PROFILE_HANDSFREE_ID)
> 
> +	Service flags:               0x01 = Disable HSP AG
> +	                             0x02 = Disable HFP AG
> +

If you do not want HSP and HFP, then there is no point in registering handsfree ID at all. I am tending currently towards the configuration option.

	0x00 = Provide Handsfree profile
	0x01 = Provide Headset profile

There is really no point in providing both? Or is there a case where you would provide both?

Regards

Marcel


^ permalink raw reply

* [PATCH v2 1/7] android: Add flags parameter to register service command
From: Szymon Janc @ 2014-03-01 21:28 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

This will allow to configure daemon services.
---
 android/hal-a2dp.c      | 1 +
 android/hal-avrcp.c     | 1 +
 android/hal-bluetooth.c | 1 +
 android/hal-handsfree.c | 1 +
 android/hal-hidhost.c   | 1 +
 android/hal-ipc-api.txt | 7 +++++++
 android/hal-msg.h       | 1 +
 android/hal-pan.c       | 1 +
 8 files changed, 14 insertions(+)

diff --git a/android/hal-a2dp.c b/android/hal-a2dp.c
index c898995..87d89a2 100644
--- a/android/hal-a2dp.c
+++ b/android/hal-a2dp.c
@@ -109,6 +109,7 @@ static bt_status_t init(btav_callbacks_t *callbacks)
 				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
 
 	cmd.service_id = HAL_SERVICE_ID_A2DP;
+	cmd.flags = 0;
 
 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
 					sizeof(cmd), &cmd, 0, NULL, NULL);
diff --git a/android/hal-avrcp.c b/android/hal-avrcp.c
index 46e25a0..4907f4a 100644
--- a/android/hal-avrcp.c
+++ b/android/hal-avrcp.c
@@ -220,6 +220,7 @@ static bt_status_t init(btrc_callbacks_t *callbacks)
 				sizeof(ev_handlers) / sizeof(ev_handlers[0]));
 
 	cmd.service_id = HAL_SERVICE_ID_AVRCP;
+	cmd.flags = 0;
 
 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
 					sizeof(cmd), &cmd, 0, NULL, NULL);
diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 6871f5d..4b31ff1 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -442,6 +442,7 @@ static int init(bt_callbacks_t *callbacks)
 	}
 
 	cmd.service_id = HAL_SERVICE_ID_SOCKET;
+	cmd.flags = 0;
 
 	status = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
 					sizeof(cmd), &cmd, NULL, NULL, NULL);
diff --git a/android/hal-handsfree.c b/android/hal-handsfree.c
index 1b150c3..422f52c 100644
--- a/android/hal-handsfree.c
+++ b/android/hal-handsfree.c
@@ -212,6 +212,7 @@ static bt_status_t init(bthf_callbacks_t *callbacks)
 				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
 
 	cmd.service_id = HAL_SERVICE_ID_HANDSFREE;
+	cmd.flags = 0;
 
 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
 					sizeof(cmd), &cmd, 0, NULL, NULL);
diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
index c758d2a..fe1b4df 100644
--- a/android/hal-hidhost.c
+++ b/android/hal-hidhost.c
@@ -354,6 +354,7 @@ static bt_status_t init(bthh_callbacks_t *callbacks)
 				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
 
 	cmd.service_id = HAL_SERVICE_ID_HIDHOST;
+	cmd.flags = 0;
 
 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
 					sizeof(cmd), &cmd, 0, NULL, NULL);
diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
index 1a19c80..f68324d 100644
--- a/android/hal-ipc-api.txt
+++ b/android/hal-ipc-api.txt
@@ -124,12 +124,16 @@ Core Service (ID 0)
 	Opcode 0x01 - Register module command/response
 
 		Command parameters: Service id (1 octet)
+		                    Flags (1 octet)
 		Response parameters: <none>
 
 		In case a command is sent for an undeclared service ID, it will
 		be rejected. Also there will be no notifications for undeclared
 		service ID.
 
+		Flags parameter values should be defined as needed by
+		respective services.
+
 		In case of an error, the error response will be returned.
 
 	Opcode 0x02 - Unregister module command/response
@@ -749,6 +753,9 @@ Bluetooth Handsfree HAL (ID 5)
 
 Android HAL name: "handsfree" (BT_PROFILE_HANDSFREE_ID)
 
+	Service flags:               0x01 = Disable HSP AG
+	                             0x02 = Disable HFP AG
+
 	Opcode 0x00 - Error response
 
 		Response parameters: Status (1 octet)
diff --git a/android/hal-msg.h b/android/hal-msg.h
index 1e12868..e66043e 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -57,6 +57,7 @@ static const char BLUEZ_HAL_SK_PATH[] = "\0bluez_hal_socket";
 #define HAL_OP_REGISTER_MODULE		0x01
 struct hal_cmd_register_module {
 	uint8_t service_id;
+	uint8_t flags;
 } __attribute__((packed));
 
 #define HAL_OP_UNREGISTER_MODULE	0x02
diff --git a/android/hal-pan.c b/android/hal-pan.c
index 5ee49ef..e6a351d 100644
--- a/android/hal-pan.c
+++ b/android/hal-pan.c
@@ -157,6 +157,7 @@ static bt_status_t pan_init(const btpan_callbacks_t *callbacks)
 				sizeof(ev_handlers)/sizeof(ev_handlers[0]));
 
 	cmd.service_id = HAL_SERVICE_ID_PAN;
+	cmd.flags = 0;
 
 	ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
 					sizeof(cmd), &cmd, 0, NULL, NULL);
-- 
1.8.5.3


^ permalink raw reply related

* Re: [PATCHv4 1/6] shared/hfp: Add prefix handlers functionality
From: Szymon Janc @ 2014-03-01 20:37 UTC (permalink / raw)
  To: Marcin Kraglak; +Cc: linux-bluetooth
In-Reply-To: <1393704046-4317-1-git-send-email-marcin.kraglak@tieto.com>

Hi Marcin,

On Saturday 01 of March 2014 21:00:41 Marcin Kraglak wrote:
> Add two functions: hfp_gw_register_prefix_handler() and
> hfp_gw_unregister_prefix_handler(). It will allow user to register for
> specific command. Current implementation just put/remove handler data
> from queue.
> ---
>  Makefile.tools   |  1 +
>  src/shared/hfp.c | 99
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/shared/hfp.h |
> 18 +++++++++++
>  3 files changed, 118 insertions(+)
> 
> diff --git a/Makefile.tools b/Makefile.tools
> index 9f7ba9f..31e1093 100644
> --- a/Makefile.tools
> +++ b/Makefile.tools
> @@ -63,6 +63,7 @@ emulator_hfp_SOURCES = emulator/hfp.c \
>  				monitor/mainloop.h monitor/mainloop.c \
>  				src/shared/io.h src/shared/io-mainloop.c \
>  				src/shared/util.h src/shared/util.c \
> +				src/shared/queue.h src/shared/queue.c \
>  				src/shared/ringbuf.h src/shared/ringbuf.c \
>  				src/shared/hfp.h src/shared/hfp.c
> 
> diff --git a/src/shared/hfp.c b/src/shared/hfp.c
> index 854cf46..4bf993d 100644
> --- a/src/shared/hfp.c
> +++ b/src/shared/hfp.c
> @@ -32,6 +32,7 @@
> 
>  #include "src/shared/util.h"
>  #include "src/shared/ringbuf.h"
> +#include "src/shared/queue.h"
>  #include "src/shared/io.h"
>  #include "src/shared/hfp.h"
> 
> @@ -42,6 +43,7 @@ struct hfp_gw {
>  	struct io *io;
>  	struct ringbuf *read_buf;
>  	struct ringbuf *write_buf;
> +	struct queue *cmd_handlers;
>  	bool writer_active;
>  	bool permissive_syntax;
>  	bool result_pending;
> @@ -60,6 +62,37 @@ struct hfp_gw {
>  	bool destroyed;
>  };
> 
> +struct cmd_handler {
> +	char *prefix;
> +	void *user_data;
> +	hfp_destroy_func_t destroy;
> +	hfp_result_func_t callback;
> +};
> +
> +static void destroy_cmd_handler(void *data)
> +{
> +	struct cmd_handler *handler = data;
> +
> +	if (handler->destroy)
> +		handler->destroy(handler->user_data);
> +
> +	free(handler);
> +}
> +
> +static bool match_handler_prefix(const void *a, const void *b)
> +{
> +	const struct cmd_handler *handler = a;
> +	const char *prefix = b;
> +
> +	if (strlen(handler->prefix) != strlen(prefix))
> +		return false;
> +
> +	if (memcmp(handler->prefix, prefix, strlen(prefix)))
> +		return false;
> +
> +	return true;
> +}
> +
>  static void write_watch_destroy(void *user_data)
>  {
>  	struct hfp_gw *hfp = user_data;
> @@ -196,8 +229,19 @@ struct hfp_gw *hfp_gw_new(int fd)
>  		return NULL;
>  	}
> 
> +	hfp->cmd_handlers = queue_new();
> +	if (!hfp->cmd_handlers) {
> +		io_destroy(hfp->io);
> +		ringbuf_free(hfp->write_buf);
> +		ringbuf_free(hfp->read_buf);
> +		free(hfp);
> +		return NULL;
> +	}
> +
>  	if (!io_set_read_handler(hfp->io, can_read_data,
>  					hfp, read_watch_destroy)) {
> +		queue_destroy(hfp->cmd_handlers,
> +						destroy_cmd_handler);
>  		io_destroy(hfp->io);
>  		ringbuf_free(hfp->write_buf);
>  		ringbuf_free(hfp->read_buf);
> @@ -250,6 +294,9 @@ void hfp_gw_unref(struct hfp_gw *hfp)
>  	ringbuf_free(hfp->write_buf);
>  	hfp->write_buf = NULL;
> 
> +	queue_destroy(hfp->cmd_handlers, destroy_cmd_handler);
> +	hfp->cmd_handlers = NULL;
> +
>  	if (!hfp->in_disconnect) {
>  		free(hfp);
>  		return;
> @@ -405,6 +452,58 @@ bool hfp_gw_set_command_handler(struct hfp_gw *hfp,
>  	return true;
>  }
> 
> +bool hfp_gw_register(struct hfp_gw *hfp, hfp_result_func_t callback,
> +						const char *prefix,
> +						void *user_data,
> +						hfp_destroy_func_t destroy)
> +{
> +	struct cmd_handler *handler;
> +
> +	handler = new0(struct cmd_handler, 1);
> +	if (!handler)
> +		return false;
> +
> +	handler->callback = callback;
> +	handler->user_data = user_data;
> +
> +	handler->prefix = strdup(prefix);
> +	if (!handler->prefix) {
> +		free(handler);
> +		return false;
> +	}
> +
> +	if (queue_find(hfp->cmd_handlers, match_handler_prefix,
> +							handler->prefix)) {
> +		destroy_cmd_handler(handler);
> +		return false;
> +	}
> +
> +	handler->destroy = destroy;
> +
> +	return queue_push_tail(hfp->cmd_handlers, handler);
> +}
> +
> +bool hfp_gw_unregister(struct hfp_gw *hfp, const char *prefix)
> +{
> +	struct cmd_handler *handler;
> +	char *lookup_prefix;
> +
> +	lookup_prefix = strdup(prefix);
> +	if (!lookup_prefix)
> +		return false;
> +
> +	handler = queue_remove_if(hfp->cmd_handlers, match_handler_prefix,
> +								lookup_prefix);
> +	free(lookup_prefix);
> +
> +	if (!handler)
> +		return false;
> +
> +	destroy_cmd_handler(handler);
> +
> +	return true;
> +}
> +
>  static void disconnect_watch_destroy(void *user_data)
>  {
>  	struct hfp_gw *hfp = user_data;
> diff --git a/src/shared/hfp.h b/src/shared/hfp.h
> index b0bd934..0755a46 100644
> --- a/src/shared/hfp.h
> +++ b/src/shared/hfp.h
> @@ -60,6 +60,18 @@ enum hfp_error {
>  	HFP_ERROR_NETWORK_NOT_ALLOWED		= 32,
>  };
> 
> +enum hfp_gw_cmd_type {
> +	HFP_GW_CMD_TYPE_READ,
> +	HFP_GW_CMD_TYPE_SET,
> +	HFP_GW_CMD_TYPE_TEST,
> +	HFP_GW_CMD_TYPE_COMMAND
> +};
> +
> +struct hfp_gw_result;
> +
> +typedef void (*hfp_result_func_t)(struct hfp_gw_result *result,
> +				enum hfp_gw_cmd_type type, void *user_data);
> +
>  typedef void (*hfp_destroy_func_t)(void *user_data);
>  typedef void (*hfp_debug_func_t)(const char *str, void *user_data);
> 
> @@ -94,3 +106,9 @@ bool hfp_gw_set_disconnect_handler(struct hfp_gw *hfp,
>  					hfp_destroy_func_t destroy);
> 
>  bool hfp_gw_disconnect(struct hfp_gw *hfp);
> +
> +bool hfp_gw_register(struct hfp_gw *hfp, hfp_result_func_t callback,
> +						const char *prefix,
> +						void *user_data,
> +						hfp_destroy_func_t destroy);
> +bool hfp_gw_unregister(struct hfp_gw *hfp, const char *prefix);

All patches in this set are now applied, thanks.

-- 
BR
Szymon Janc

^ permalink raw reply

* [PATCHv4 6/6] shared/hfp: Add function to get unquoted string
From: Marcin Kraglak @ 2014-03-01 20:00 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1393704046-4317-1-git-send-email-marcin.kraglak@tieto.com>

---
 src/shared/hfp.c | 28 ++++++++++++++++++++++++++++
 src/shared/hfp.h |  2 ++
 2 files changed, 30 insertions(+)

diff --git a/src/shared/hfp.c b/src/shared/hfp.c
index 10734fc..b10316e 100644
--- a/src/shared/hfp.c
+++ b/src/shared/hfp.c
@@ -308,6 +308,34 @@ bool hfp_gw_result_get_string(struct hfp_gw_result *result, char *buf,
 	return true;
 }
 
+bool hfp_gw_result_get_unquoted_string(struct hfp_gw_result *result, char *buf,
+								uint8_t len)
+{
+	const char *data = result->data;
+	int i = 0;
+	char c;
+
+	skip_whitespace(result);
+
+	c = data[result->offset];
+	if (c == '"' || c == ')' || c == '(')
+		return false;
+
+	while (data[result->offset] != '\0' && data[result->offset] != ','
+					&& data[result->offset] != ')') {
+		if (i < len)
+			buf[i++] = data[result->offset];
+		result->offset++;
+	}
+
+	if (i < len)
+		buf[i++] = '\0';
+
+	next_field(result);
+
+	return true;
+}
+
 static void process_input(struct hfp_gw *hfp)
 {
 	char *str, *ptr;
diff --git a/src/shared/hfp.h b/src/shared/hfp.h
index 3313d91..649b77e 100644
--- a/src/shared/hfp.h
+++ b/src/shared/hfp.h
@@ -118,3 +118,5 @@ bool hfp_gw_result_open_container(struct hfp_gw_result *result);
 bool hfp_gw_result_close_container(struct hfp_gw_result *result);
 bool hfp_gw_result_get_string(struct hfp_gw_result *result, char *buf,
 								uint8_t len);
+bool hfp_gw_result_get_unquoted_string(struct hfp_gw_result *result, char *buf,
+								uint8_t len);
-- 
1.8.3.1


^ permalink raw reply related

* [PATCHv4 5/6] shared/hfp: Add function to get string
From: Marcin Kraglak @ 2014-03-01 20:00 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1393704046-4317-1-git-send-email-marcin.kraglak@tieto.com>

It will look for quoted sting and write it to given buffer.
---
 src/shared/hfp.c | 33 +++++++++++++++++++++++++++++++++
 src/shared/hfp.h |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/src/shared/hfp.c b/src/shared/hfp.c
index d7cca32..10734fc 100644
--- a/src/shared/hfp.c
+++ b/src/shared/hfp.c
@@ -275,6 +275,39 @@ bool hfp_gw_result_close_container(struct hfp_gw_result *result)
 	return true;
 }
 
+bool hfp_gw_result_get_string(struct hfp_gw_result *result, char *buf,
+								uint8_t len)
+{
+	int i = 0;
+	const char *data = result->data;
+
+	skip_whitespace(result);
+
+	if (data[result->offset] != '"')
+		return false;
+
+	result->offset++;
+
+	while (data[result->offset] != '\0' && data[result->offset] != '"') {
+		if (i < len)
+			buf[i++] = data[result->offset];
+		result->offset++;
+	}
+
+	if (i < len)
+		buf[i++] = '\0';
+
+	if (data[result->offset] == '"')
+		result->offset++;
+	else
+		return false;
+
+	skip_whitespace(result);
+	next_field(result);
+
+	return true;
+}
+
 static void process_input(struct hfp_gw *hfp)
 {
 	char *str, *ptr;
diff --git a/src/shared/hfp.h b/src/shared/hfp.h
index 00905de..3313d91 100644
--- a/src/shared/hfp.h
+++ b/src/shared/hfp.h
@@ -116,3 +116,5 @@ bool hfp_gw_unregister(struct hfp_gw *hfp, const char *prefix);
 bool hfp_gw_result_get_number(struct hfp_gw_result *result, unsigned int *val);
 bool hfp_gw_result_open_container(struct hfp_gw_result *result);
 bool hfp_gw_result_close_container(struct hfp_gw_result *result);
+bool hfp_gw_result_get_string(struct hfp_gw_result *result, char *buf,
+								uint8_t len);
-- 
1.8.3.1


^ permalink raw reply related

* [PATCHv4 4/6] shared/hfp: Add open/close container function
From: Marcin Kraglak @ 2014-03-01 20:00 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1393704046-4317-1-git-send-email-marcin.kraglak@tieto.com>

It will look for "(" or ")" parenthesis and skip it if found.
---
 src/shared/hfp.c | 26 ++++++++++++++++++++++++++
 src/shared/hfp.h |  2 ++
 2 files changed, 28 insertions(+)

diff --git a/src/shared/hfp.c b/src/shared/hfp.c
index 7cc9a30..d7cca32 100644
--- a/src/shared/hfp.c
+++ b/src/shared/hfp.c
@@ -249,6 +249,32 @@ bool hfp_gw_result_get_number(struct hfp_gw_result *result, unsigned int *val)
 	return true;
 }
 
+bool hfp_gw_result_open_container(struct hfp_gw_result *result)
+{
+	skip_whitespace(result);
+
+	/* The list shall be preceded by a left parenthesis "(") */
+	if (result->data[result->offset] != '(')
+		return false;
+
+	result->offset++;
+
+	return true;
+}
+
+bool hfp_gw_result_close_container(struct hfp_gw_result *result)
+{
+	skip_whitespace(result);
+
+	/* The list shall be followed by a right parenthesis (")" V250 5.7.3.1*/
+	if (result->data[result->offset] != ')')
+		return false;
+
+	result->offset++;
+
+	return true;
+}
+
 static void process_input(struct hfp_gw *hfp)
 {
 	char *str, *ptr;
diff --git a/src/shared/hfp.h b/src/shared/hfp.h
index 40adcb6..00905de 100644
--- a/src/shared/hfp.h
+++ b/src/shared/hfp.h
@@ -114,3 +114,5 @@ bool hfp_gw_register(struct hfp_gw *hfp, hfp_result_func_t callback,
 bool hfp_gw_unregister(struct hfp_gw *hfp, const char *prefix);
 
 bool hfp_gw_result_get_number(struct hfp_gw_result *result, unsigned int *val);
+bool hfp_gw_result_open_container(struct hfp_gw_result *result);
+bool hfp_gw_result_close_container(struct hfp_gw_result *result);
-- 
1.8.3.1


^ permalink raw reply related

* [PATCHv4 3/6] shared/hfp: Add get_number implementation
From: Marcin Kraglak @ 2014-03-01 20:00 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1393704046-4317-1-git-send-email-marcin.kraglak@tieto.com>

User can call this function with hfp_gw_result, which will
be passed to prefix handler.
---
 src/shared/hfp.c | 31 +++++++++++++++++++++++++++++++
 src/shared/hfp.h |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/src/shared/hfp.c b/src/shared/hfp.c
index 945f36e..7cc9a30 100644
--- a/src/shared/hfp.c
+++ b/src/shared/hfp.c
@@ -218,6 +218,37 @@ done:
 	return true;
 }
 
+static void next_field(struct hfp_gw_result *result)
+{
+	if (result->data[result->offset] == ',')
+		result->offset++;
+}
+
+bool hfp_gw_result_get_number(struct hfp_gw_result *result, unsigned int *val)
+{
+	int tmp = 0;
+	int i;
+
+	skip_whitespace(result);
+
+	i = result->offset;
+
+	while (result->data[i] >= '0' && result->data[i] <= '9')
+		tmp = tmp * 10 + result->data[i++] - '0';
+
+	if (i == result->offset)
+		return false;
+
+	if (val)
+		*val = tmp;
+	result->offset = i;
+
+	skip_whitespace(result);
+	next_field(result);
+
+	return true;
+}
+
 static void process_input(struct hfp_gw *hfp)
 {
 	char *str, *ptr;
diff --git a/src/shared/hfp.h b/src/shared/hfp.h
index 0755a46..40adcb6 100644
--- a/src/shared/hfp.h
+++ b/src/shared/hfp.h
@@ -112,3 +112,5 @@ bool hfp_gw_register(struct hfp_gw *hfp, hfp_result_func_t callback,
 						void *user_data,
 						hfp_destroy_func_t destroy);
 bool hfp_gw_unregister(struct hfp_gw *hfp, const char *prefix);
+
+bool hfp_gw_result_get_number(struct hfp_gw_result *result, unsigned int *val);
-- 
1.8.3.1


^ permalink raw reply related

* [PATCHv4 2/6] shared/hfp: Add implementiation of processing commands
From: Marcin Kraglak @ 2014-03-01 20:00 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1393704046-4317-1-git-send-email-marcin.kraglak@tieto.com>

It will look for prefix handler for given cammand.
---
 src/shared/hfp.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 94 insertions(+), 4 deletions(-)

diff --git a/src/shared/hfp.c b/src/shared/hfp.c
index 4bf993d..945f36e 100644
--- a/src/shared/hfp.c
+++ b/src/shared/hfp.c
@@ -29,6 +29,7 @@
 #include <unistd.h>
 #include <string.h>
 #include <stdarg.h>
+#include <ctype.h>
 
 #include "src/shared/util.h"
 #include "src/shared/ringbuf.h"
@@ -69,6 +70,11 @@ struct cmd_handler {
 	hfp_result_func_t callback;
 };
 
+struct hfp_gw_result {
+	const char *data;
+	int offset;
+};
+
 static void destroy_cmd_handler(void *data)
 {
 	struct cmd_handler *handler = data;
@@ -130,6 +136,88 @@ static void wakeup_writer(struct hfp_gw *hfp)
 	hfp->writer_active = true;
 }
 
+static void skip_whitespace(struct hfp_gw_result *result)
+{
+	while (result->data[result->offset] == ' ')
+		result->offset++;
+}
+
+static bool call_prefix_handler(struct hfp_gw *hfp, const char *data)
+{
+	struct cmd_handler *handler;
+	const char *separators = ";?=\0";
+	struct hfp_gw_result result;
+	enum hfp_gw_cmd_type type;
+	char lookup_prefix[18];
+	uint8_t pref_len = 0;
+	const char *prefix;
+	int i;
+
+	result.offset = 0;
+	result.data = data;
+
+	skip_whitespace(&result);
+
+	if (strlen(data + result.offset) < 3)
+		return false;
+
+	if (strncmp(data + result.offset, "AT", 2))
+		if (strncmp(data + result.offset, "at", 2))
+			return false;
+
+	result.offset += 2;
+	prefix = data + result.offset;
+
+	if (isalpha(prefix[0])) {
+		lookup_prefix[pref_len++] = toupper(prefix[0]);
+	} else {
+		pref_len = strcspn(prefix, separators);
+		if (pref_len > 17 || pref_len < 2)
+			return false;
+
+		for (i = 0; i < pref_len; i++)
+			lookup_prefix[i] = toupper(prefix[i]);
+	}
+
+	lookup_prefix[pref_len] = '\0';
+	result.offset += pref_len;
+
+	if (lookup_prefix[0] == 'D') {
+		type = HFP_GW_CMD_TYPE_SET;
+		goto done;
+	}
+
+	if (data[result.offset] == '=') {
+		result.offset++;
+		if (data[result.offset] == '?') {
+			result.offset++;
+			type = HFP_GW_CMD_TYPE_TEST;
+		} else {
+			type = HFP_GW_CMD_TYPE_SET;
+		}
+		goto done;
+	}
+
+	if (data[result.offset] == '?') {
+		result.offset++;
+		type = HFP_GW_CMD_TYPE_READ;
+		goto done;
+	}
+
+	type = HFP_GW_CMD_TYPE_COMMAND;
+
+done:
+
+	handler = queue_find(hfp->cmd_handlers, match_handler_prefix,
+								lookup_prefix);
+	if (!handler)
+		return false;
+
+	handler->callback(&result, type, handler->user_data);
+
+	return true;
+}
+
 static void process_input(struct hfp_gw *hfp)
 {
 	char *str, *ptr;
@@ -162,10 +250,12 @@ static void process_input(struct hfp_gw *hfp)
 
 	hfp->result_pending = true;
 
-	if (hfp->command_callback)
-		hfp->command_callback(str, hfp->command_data);
-	else
-		hfp_gw_send_result(hfp, HFP_RESULT_ERROR);
+	if (!call_prefix_handler(hfp, str)) {
+		if (hfp->command_callback)
+			hfp->command_callback(str, hfp->command_data);
+		else
+			hfp_gw_send_result(hfp, HFP_RESULT_ERROR);
+	}
 
 	len = ringbuf_drain(hfp->read_buf, count + 1);
 
-- 
1.8.3.1


^ permalink raw reply related

* [PATCHv4 1/6] shared/hfp: Add prefix handlers functionality
From: Marcin Kraglak @ 2014-03-01 20:00 UTC (permalink / raw)
  To: linux-bluetooth

Add two functions: hfp_gw_register_prefix_handler() and
hfp_gw_unregister_prefix_handler(). It will allow user to register for
specific command. Current implementation just put/remove handler data
from queue.
---
 Makefile.tools   |  1 +
 src/shared/hfp.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/hfp.h | 18 +++++++++++
 3 files changed, 118 insertions(+)

diff --git a/Makefile.tools b/Makefile.tools
index 9f7ba9f..31e1093 100644
--- a/Makefile.tools
+++ b/Makefile.tools
@@ -63,6 +63,7 @@ emulator_hfp_SOURCES = emulator/hfp.c \
 				monitor/mainloop.h monitor/mainloop.c \
 				src/shared/io.h src/shared/io-mainloop.c \
 				src/shared/util.h src/shared/util.c \
+				src/shared/queue.h src/shared/queue.c \
 				src/shared/ringbuf.h src/shared/ringbuf.c \
 				src/shared/hfp.h src/shared/hfp.c
 
diff --git a/src/shared/hfp.c b/src/shared/hfp.c
index 854cf46..4bf993d 100644
--- a/src/shared/hfp.c
+++ b/src/shared/hfp.c
@@ -32,6 +32,7 @@
 
 #include "src/shared/util.h"
 #include "src/shared/ringbuf.h"
+#include "src/shared/queue.h"
 #include "src/shared/io.h"
 #include "src/shared/hfp.h"
 
@@ -42,6 +43,7 @@ struct hfp_gw {
 	struct io *io;
 	struct ringbuf *read_buf;
 	struct ringbuf *write_buf;
+	struct queue *cmd_handlers;
 	bool writer_active;
 	bool permissive_syntax;
 	bool result_pending;
@@ -60,6 +62,37 @@ struct hfp_gw {
 	bool destroyed;
 };
 
+struct cmd_handler {
+	char *prefix;
+	void *user_data;
+	hfp_destroy_func_t destroy;
+	hfp_result_func_t callback;
+};
+
+static void destroy_cmd_handler(void *data)
+{
+	struct cmd_handler *handler = data;
+
+	if (handler->destroy)
+		handler->destroy(handler->user_data);
+
+	free(handler);
+}
+
+static bool match_handler_prefix(const void *a, const void *b)
+{
+	const struct cmd_handler *handler = a;
+	const char *prefix = b;
+
+	if (strlen(handler->prefix) != strlen(prefix))
+		return false;
+
+	if (memcmp(handler->prefix, prefix, strlen(prefix)))
+		return false;
+
+	return true;
+}
+
 static void write_watch_destroy(void *user_data)
 {
 	struct hfp_gw *hfp = user_data;
@@ -196,8 +229,19 @@ struct hfp_gw *hfp_gw_new(int fd)
 		return NULL;
 	}
 
+	hfp->cmd_handlers = queue_new();
+	if (!hfp->cmd_handlers) {
+		io_destroy(hfp->io);
+		ringbuf_free(hfp->write_buf);
+		ringbuf_free(hfp->read_buf);
+		free(hfp);
+		return NULL;
+	}
+
 	if (!io_set_read_handler(hfp->io, can_read_data,
 					hfp, read_watch_destroy)) {
+		queue_destroy(hfp->cmd_handlers,
+						destroy_cmd_handler);
 		io_destroy(hfp->io);
 		ringbuf_free(hfp->write_buf);
 		ringbuf_free(hfp->read_buf);
@@ -250,6 +294,9 @@ void hfp_gw_unref(struct hfp_gw *hfp)
 	ringbuf_free(hfp->write_buf);
 	hfp->write_buf = NULL;
 
+	queue_destroy(hfp->cmd_handlers, destroy_cmd_handler);
+	hfp->cmd_handlers = NULL;
+
 	if (!hfp->in_disconnect) {
 		free(hfp);
 		return;
@@ -405,6 +452,58 @@ bool hfp_gw_set_command_handler(struct hfp_gw *hfp,
 	return true;
 }
 
+bool hfp_gw_register(struct hfp_gw *hfp, hfp_result_func_t callback,
+						const char *prefix,
+						void *user_data,
+						hfp_destroy_func_t destroy)
+{
+	struct cmd_handler *handler;
+
+	handler = new0(struct cmd_handler, 1);
+	if (!handler)
+		return false;
+
+	handler->callback = callback;
+	handler->user_data = user_data;
+
+	handler->prefix = strdup(prefix);
+	if (!handler->prefix) {
+		free(handler);
+		return false;
+	}
+
+	if (queue_find(hfp->cmd_handlers, match_handler_prefix,
+							handler->prefix)) {
+		destroy_cmd_handler(handler);
+		return false;
+	}
+
+	handler->destroy = destroy;
+
+	return queue_push_tail(hfp->cmd_handlers, handler);
+}
+
+bool hfp_gw_unregister(struct hfp_gw *hfp, const char *prefix)
+{
+	struct cmd_handler *handler;
+	char *lookup_prefix;
+
+	lookup_prefix = strdup(prefix);
+	if (!lookup_prefix)
+		return false;
+
+	handler = queue_remove_if(hfp->cmd_handlers, match_handler_prefix,
+								lookup_prefix);
+	free(lookup_prefix);
+
+	if (!handler)
+		return false;
+
+	destroy_cmd_handler(handler);
+
+	return true;
+}
+
 static void disconnect_watch_destroy(void *user_data)
 {
 	struct hfp_gw *hfp = user_data;
diff --git a/src/shared/hfp.h b/src/shared/hfp.h
index b0bd934..0755a46 100644
--- a/src/shared/hfp.h
+++ b/src/shared/hfp.h
@@ -60,6 +60,18 @@ enum hfp_error {
 	HFP_ERROR_NETWORK_NOT_ALLOWED		= 32,
 };
 
+enum hfp_gw_cmd_type {
+	HFP_GW_CMD_TYPE_READ,
+	HFP_GW_CMD_TYPE_SET,
+	HFP_GW_CMD_TYPE_TEST,
+	HFP_GW_CMD_TYPE_COMMAND
+};
+
+struct hfp_gw_result;
+
+typedef void (*hfp_result_func_t)(struct hfp_gw_result *result,
+				enum hfp_gw_cmd_type type, void *user_data);
+
 typedef void (*hfp_destroy_func_t)(void *user_data);
 typedef void (*hfp_debug_func_t)(const char *str, void *user_data);
 
@@ -94,3 +106,9 @@ bool hfp_gw_set_disconnect_handler(struct hfp_gw *hfp,
 					hfp_destroy_func_t destroy);
 
 bool hfp_gw_disconnect(struct hfp_gw *hfp);
+
+bool hfp_gw_register(struct hfp_gw *hfp, hfp_result_func_t callback,
+						const char *prefix,
+						void *user_data,
+						hfp_destroy_func_t destroy);
+bool hfp_gw_unregister(struct hfp_gw *hfp, const char *prefix);
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH BlueZ] monitor: Validate HCI event/command parameter length when parsing
From: Anderson Lizardo @ 2014-03-01 19:23 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo

Print an error message if the parameter length for HCI commands/events
does not match the actual remaining packet size.

This mainly avoids using garbage bytes when parsing corrupted packets.
The check was inspired on those used when parsing SCO/ACL packets.
---
 monitor/packet.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/monitor/packet.c b/monitor/packet.c
index 78ecfd7..58a75db 100644
--- a/monitor/packet.c
+++ b/monitor/packet.c
@@ -7533,6 +7533,13 @@ void packet_hci_command(struct timeval *tv, uint16_t index,
 		return;
 	}
 
+	if (size != hdr->plen) {
+		print_text(COLOR_ERROR, "invalid packet size (%d != %d)", size,
+								hdr->plen);
+		packet_hexdump(data, size);
+		return;
+	}
+
 	if (opcode_data->cmd_fixed) {
 		if (hdr->plen != opcode_data->cmd_size) {
 			print_text(COLOR_ERROR, "invalid packet size");
@@ -7598,6 +7605,13 @@ void packet_hci_event(struct timeval *tv, uint16_t index,
 		return;
 	}
 
+	if (size != hdr->plen) {
+		print_text(COLOR_ERROR, "invalid packet size (%d != %d)", size,
+								hdr->plen);
+		packet_hexdump(data, size);
+		return;
+	}
+
 	if (event_data->fixed) {
 		if (hdr->plen != event_data->size) {
 			print_text(COLOR_ERROR, "invalid packet size");
-- 
1.8.3.2


^ permalink raw reply related

* Re: Passive scanning of iBeacons results in a "Data Buffer Overflow"
From: Anderson Lizardo @ 2014-03-01 19:11 UTC (permalink / raw)
  To: Adam Warski; +Cc: BlueZ development
In-Reply-To: <A4DDF6F3-8DD4-4CF7-A5C0-205F5B4EE489@warski.org>

Hi Adam,

On Sat, Mar 1, 2014 at 5:34 AM, Adam Warski <adam@warski.org> wrote:
>> Can you save the raw dump using "hcidump -w output.dump"  (or using
>> btmon -w) and send to the list? It is easier to analyze, as the parser
>> may be bogus.
>
> Sure. The problems start at 63.120548.
> The dump is here: http://www.warski.org/btmon_ibeacons.dat

Maybe you already did your own investigation, but here goes my findings:

When opening the dump file on a hex editor, we can see that this first
bogus packet starts (skipping BTSnoop's own header) at offset 0x45a4
(with bytes "3E 2A 02 01 ..."). At offset 0x45b4 we can see that the
packet becomes truncated by missing these bytes (based on previous
correct packets):

1A FF 4C 00 02 15 B9 40 7F 30 F5 F8 46 6E AF F9

>From now on, the packets become garbage because they are "shifted" due
to these missing bytes.

I think we can rule out any problems on the hcidump/btmon tools (as
the raw logs come directly from the kernel), so I see two
possibilities here:

1) There might be a bug in HCI packet reassembly logic (either in
hci_core.c or btusb.c).

2) (most likely) your BT dongle may be broken (i.e. generating
truncated USB packets from time to time)

So my suggestion would be to try with a different dongle, preferably
of a different chipset vendor. Note that "TP-Link" is just the brand
name, you can confirm the chipset using lsusb or (even better)
"hciconfig hci0 -a" (check "Manufacturer" field).

Can you also provide the "lsusb" and "hciconfig hci0 -a" output (feel
free to modify the BT address for privacy reasons)? I think it is
important to have this archived so similar reports can be traced to
this hardware and firmware.

Another good source of information is the kernel debug logs. Collect
it by enabling dynamic debugging:

# dmesg -c  # just to clear the kernel log buffer
# echo 'module bluetooth +pf' | cat > /sys/kernel/debug/dynamic_debug/control
# echo 'module btusb +pf' | cat > /sys/kernel/debug/dynamic_debug/control
# btmon -w lescan.dump # So we can correlate the kernel log with the
actual packets
# hcitool lescan --duplicates # Run on another terminal, until packets
become corrupt
# dmesg > dmesg.txt
# echo 'module bluetooth -pf' | cat > /sys/kernel/debug/dynamic_debug/control
# echo 'module btusb -pf' | cat > /sys/kernel/debug/dynamic_debug/control
# gzip dmesg.txt (if it is too big)

Then send dmesg.txt.gz + lescan.dump to the list. You may want to mask
out your BT address on the log as well.

>
> Thanks,
> Adam

Best Regards,
-- 
Anderson Lizardo
http://www.indt.org/?lang=en
INdT - Manaus - Brazil

^ permalink raw reply

* [PATCH 3/3] android/avrcp-lib: Fix destroy callback handling
From: Szymon Janc @ 2014-03-01 19:04 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1393700658-14323-1-git-send-email-szymon.janc@tieto.com>

AVRCP destroy callback was directly passed as AVCTP destroy callback
resulting in struct avrcp not being freed on remote disconnect.

This fix following Valgrind report:

931 (36 direct, 895 indirect) bytes in 1 blocks are definitely lost in
     loss record 53 of 55
   at 0x4896DC8: calloc (in /system/lib/valgrind/
     vgpreload_memcheck-arm-linux.so)
   by 0x48C5DB7: g_malloc0 (gmem.c:189)
   by 0x118671: avrcp_new (avrcp-lib.c:219)
   by 0x117D4F: connect_cb (avrcp.c:346)
   by 0x12068B: connect_cb (btio.c:232)
   by 0x48BD9C7: g_io_unix_dispatch (giounix.c:166)
   by 0x48C2CCB: g_main_context_dispatch (gmain.c:2539)
   by 0x48C2ED9: g_main_context_iterate.isra.19 (gmain.c:3146)
   by 0x48C3167: g_main_loop_run (gmain.c:3340)
   by 0x10BA03: main (main.c:490)
---
 android/avrcp-lib.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index feab303..bdc85ed 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -112,6 +112,9 @@ struct avrcp {
 	const struct avrcp_passthrough_handler *passthrough_handlers;
 	void *passthrough_data;
 	unsigned int passthrough_id;
+
+	avrcp_destroy_cb_t destroy;
+	void *destroy_data;
 };
 
 void avrcp_shutdown(struct avrcp *session)
@@ -126,6 +129,9 @@ void avrcp_shutdown(struct avrcp *session)
 		avctp_shutdown(session->conn);
 	}
 
+	if (session->destroy)
+		session->destroy(session->destroy_data);
+
 	g_free(session->tx_buf);
 	g_free(session);
 }
@@ -213,6 +219,15 @@ static bool handle_passthrough_pdu(struct avctp *conn, uint8_t op,
 	return handler->func(session);
 }
 
+static void disconnect_cb(void *data)
+{
+	struct avrcp *session = data;
+
+	session->conn = NULL;
+
+	avrcp_shutdown(session);
+}
+
 struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 {
 	struct avrcp *session;
@@ -237,13 +252,16 @@ struct avrcp *avrcp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
 							handle_vendordep_pdu,
 							session);
 
+	avctp_set_destroy_cb(session->conn, disconnect_cb, session);
+
 	return session;
 }
 
 void avrcp_set_destroy_cb(struct avrcp *session, avrcp_destroy_cb_t cb,
 							void *user_data)
 {
-	avctp_set_destroy_cb(session->conn, cb, user_data);
+	session->destroy = cb;
+	session->destroy_data = user_data;
 }
 
 void avrcp_set_control_handlers(struct avrcp *session,
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 2/3] android/avrcp-lib: Fix not freeing tx buffer on AVRCP shutdown
From: Szymon Janc @ 2014-03-01 19:04 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1393700658-14323-1-git-send-email-szymon.janc@tieto.com>

---
 android/avrcp-lib.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
index e71f95c..feab303 100644
--- a/android/avrcp-lib.c
+++ b/android/avrcp-lib.c
@@ -126,6 +126,7 @@ void avrcp_shutdown(struct avrcp *session)
 		avctp_shutdown(session->conn);
 	}
 
+	g_free(session->tx_buf);
 	g_free(session);
 }
 
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH 1/3] android/avctp: Fix memory leak on disconnect
From: Szymon Janc @ 2014-03-01 19:04 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

Free passthrough handler if it was not unregistered.

This fix following Valgrind report:

943 (12 direct, 931 indirect) bytes in 1 blocks are definitely lost in
     loss record 55 of 57
   at 0x48993CC: malloc (in /system/lib/valgrind/
     vgpreload_memcheck-arm-linux.so)
   by 0x48C5D77: g_malloc (gmem.c:159)
   by 0x1177E3: avctp_register_passthrough_handler (avctp.c:1293)
   by 0x1186BF: avrcp_new (avrcp-lib.c:230)
   by 0x117D67: connect_cb (avrcp.c:346)
   by 0x1206DB: connect_cb (btio.c:232)
   by 0x48BD9C7: g_io_unix_dispatch (giounix.c:166)
   by 0x48C2CCB: g_main_context_dispatch (gmain.c:2539)
   by 0x48C2ED9: g_main_context_iterate.isra.19 (gmain.c:3146)
   by 0x48C3167: g_main_loop_run (gmain.c:3340)
   by 0x10BA1F: main (main.c:491)
---
 android/avctp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/android/avctp.c b/android/avctp.c
index bc1bd80..b901371 100644
--- a/android/avctp.c
+++ b/android/avctp.c
@@ -1501,5 +1501,6 @@ void avctp_shutdown(struct avctp *session)
 		session->uinput = -1;
 	}
 
+	g_free(session->handler);
 	g_free(session);
 }
-- 
1.8.5.3


^ permalink raw reply related

* [PATCH BlueZ] emulator/btdev: Add dummy support for clearing the LE white list
From: Anderson Lizardo @ 2014-03-01 14:25 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Anderson Lizardo

Add just enough support for being able to bring up LE capable virtual
controllers using "btvirt".

Fixes this error on "btvirt -l2":

Unsupported command 0x2010

And on "hciconfig hci0 up":

Can't init device hci0: Invalid request code (56)
---
 emulator/btdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/emulator/btdev.c b/emulator/btdev.c
index 4a54a23..b54f91e 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -2353,6 +2353,13 @@ static void default_cmd(struct btdev *btdev, uint16_t opcode,
 		cmd_complete(btdev, opcode, &lrwls, sizeof(lrwls));
 		break;
 
+	case BT_HCI_CMD_LE_CLEAR_WHITE_LIST:
+		if (btdev->type == BTDEV_TYPE_BREDR)
+			goto unsupported;
+		status = BT_HCI_ERR_SUCCESS;
+		cmd_complete(btdev, opcode, &status, sizeof(status));
+		break;
+
 	case BT_HCI_CMD_LE_READ_SUPPORTED_STATES:
 		if (btdev->type == BTDEV_TYPE_BREDR)
 			goto unsupported;
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCH] android/socket: Use same RFCOMM channels for services as Linux daemon
From: Szymon Janc @ 2014-03-01 13:06 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1393622608-22965-1-git-send-email-szymon.janc@tieto.com>

On Friday 28 of February 2014 22:23:28 Szymon Janc wrote:
> Use numbers from doc/assigned-numbers.txt.
> ---
>  android/socket.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/android/socket.c b/android/socket.c
> index ee98b54..9aab345 100644
> --- a/android/socket.c
> +++ b/android/socket.c
> @@ -47,9 +47,10 @@
> 
>  #define RFCOMM_CHANNEL_MAX 30
> 
> -#define OPP_DEFAULT_CHANNEL	12
> +#define OPP_DEFAULT_CHANNEL	9
>  #define HFAG_DEFAULT_CHANNEL	13
> -#define PBAP_DEFAULT_CHANNEL	19
> +#define PBAP_DEFAULT_CHANNEL	15
> +#define MAP_MAS_DEFAULT_CHANNEL	16
> 
>  #define SVC_HINT_OBEX 0x10
> 
> @@ -403,7 +404,7 @@ static const struct profile_info {
>  			0x00, 0x00, 0x11, 0x32, 0x00, 0x00, 0x10, 0x00,
>  			0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB
>  		},
> -		.channel = 0,
> +		.channel = MAP_MAS_DEFAULT_CHANNEL,
>  		.svc_hint = SVC_HINT_OBEX,
>  		.sec_level = BT_IO_SEC_MEDIUM,
>  		.create_record = create_mas_record

Pushed.

-- 
BR
Szymon Janc

^ permalink raw reply

* Re: [PATCH 3/5] android/hal-audio: Write and sync in common code
From: Szymon Janc @ 2014-03-01 12:00 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1393610480-5775-4-git-send-email-andrzej.kaczmarek@tieto.com>

Hi Andrzej,

On Friday 28 of February 2014 19:01:18 Andrzej Kaczmarek wrote:
> There's no need for codec abstraction to take care of writing and
> synchronizing actual media stream since this is something that common
> code can do regardless of codec used.
> 
> Data are synchronized based on number of samples consumed from input
> stream instead of frames encoded to output stream which is also more
> reliable since it's not affected by encoder errors.
> ---
>  android/hal-audio.c | 188
> +++++++++++++++++++++++++--------------------------- 1 file changed, 91
> insertions(+), 97 deletions(-)
> 
> diff --git a/android/hal-audio.c b/android/hal-audio.c
> index 78889e5..b4d78ca 100644
> --- a/android/hal-audio.c
> +++ b/android/hal-audio.c
> @@ -130,17 +130,9 @@ struct sbc_data {
>  	size_t in_buf_size;
> 
>  	size_t out_frame_len;
> -	size_t out_buf_size;
> -	uint8_t *out_buf;
> 
>  	unsigned frame_duration;
>  	unsigned frames_per_packet;
> -
> -	struct timespec start;
> -	unsigned frames_sent;
> -	uint32_t timestamp;
> -
> -	uint16_t seq;
>  };
> 
>  static inline void timespec_diff(struct timespec *a, struct timespec *b,
> @@ -162,9 +154,9 @@ static int sbc_cleanup(void *codec_data);
>  static int sbc_get_config(void *codec_data, struct audio_input_config
> *config); static size_t sbc_get_buffer_size(void *codec_data);
>  static size_t sbc_get_mediapacket_duration(void *codec_data);
> -static void sbc_resume(void *codec_data);
> -static ssize_t sbc_write_data(void *codec_data, const void *buffer,
> -							size_t bytes, int fd);
> +static ssize_t sbc_encode_mediapacket(void *codec_data, const uint8_t
> *buffer, +					size_t len, struct media_packet *mp,
> +					size_t mp_data_len, size_t *written);
> 
>  struct audio_codec {
>  	uint8_t type;
> @@ -178,9 +170,9 @@ struct audio_codec {
>  					struct audio_input_config *config);
>  	size_t (*get_buffer_size) (void *codec_data);
>  	size_t (*get_mediapacket_duration) (void *codec_data);
> -	void (*resume) (void *codec_data);
> -	ssize_t (*write_data) (void *codec_data, const void *buffer,
> -							size_t bytes, int fd);
> +	ssize_t (*encode_mediapacket) (void *codec_data, const uint8_t *buffer,
> +					size_t len, struct media_packet *mp,
> +					size_t mp_data_len, size_t *written);
>  };
> 
>  static const struct audio_codec audio_codecs[] = {
> @@ -194,8 +186,7 @@ static const struct audio_codec audio_codecs[] = {
>  		.get_config = sbc_get_config,
>  		.get_buffer_size = sbc_get_buffer_size,
>  		.get_mediapacket_duration = sbc_get_mediapacket_duration,
> -		.resume = sbc_resume,
> -		.write_data = sbc_write_data,
> +		.encode_mediapacket = sbc_encode_mediapacket,
>  	}
>  };
> 
> @@ -208,6 +199,13 @@ struct audio_endpoint {
>  	const struct audio_codec *codec;
>  	void *codec_data;
>  	int fd;
> +
> +	struct media_packet *mp;
> +	size_t mp_data_len;
> +
> +	uint16_t seq;
> +	uint32_t samples;
> +	struct timespec start;
>  };
> 
>  static struct audio_endpoint audio_endpoints[MAX_AUDIO_ENDPOINTS];
> @@ -419,8 +417,6 @@ static int sbc_codec_init(struct audio_preset *preset,
> uint16_t mtu, sbc_data->in_buf_size = num_frames * in_frame_len;
> 
>  	sbc_data->out_frame_len = out_frame_len;
> -	sbc_data->out_buf_size = hdr_len + num_frames * out_frame_len;
> -	sbc_data->out_buf = calloc(1, sbc_data->out_buf_size);
> 
>  	sbc_data->frame_duration = sbc_get_frame_duration(&sbc_data->enc);
>  	sbc_data->frames_per_packet = num_frames;
> @@ -438,7 +434,6 @@ static int sbc_cleanup(void *codec_data)
>  	struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
> 
>  	sbc_finish(&sbc_data->enc);
> -	free(sbc_data->out_buf);
>  	free(codec_data);
> 
>  	return AUDIO_STATUS_SUCCESS;
> @@ -486,28 +481,18 @@ static size_t sbc_get_mediapacket_duration(void
> *codec_data) return sbc_data->frame_duration * sbc_data->frames_per_packet;
>  }
> 
> -static void sbc_resume(void *codec_data)
> -{
> -	struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
> -
> -	DBG("");
> -
> -	clock_gettime(CLOCK_MONOTONIC, &sbc_data->start);
> -
> -	sbc_data->frames_sent = 0;
> -	sbc_data->timestamp = 0;
> -}
> -
> -static int write_media_packet(int fd, struct sbc_data *sbc_data,
> -				struct media_packet *mp, size_t data_len)
> +static int write_media_packet(struct a2dp_stream_out *out, size_t
> mp_data_len, +							uint32_t input_samples)
>  {
> +	struct audio_endpoint *ep = out->ep;
> +	struct media_packet *mp = ep->mp;
>  	struct timespec cur;
>  	struct timespec diff;
> -	unsigned expected_frames;
> +	uint32_t expected_samples;
>  	int ret;
> 
>  	while (true) {
> -		ret = write(fd, mp, sizeof(*mp) + data_len);
> +		ret = write(ep->fd, mp, sizeof(*mp) + mp_data_len);
>  		if (ret >= 0)
>  			break;
> 
> @@ -515,12 +500,10 @@ static int write_media_packet(int fd, struct sbc_data
> *sbc_data, return -errno;
>  	}
> 
> -	sbc_data->frames_sent += mp->payload.frame_count;
> -
>  	clock_gettime(CLOCK_MONOTONIC, &cur);
> -	timespec_diff(&cur, &sbc_data->start, &diff);
> -	expected_frames = (diff.tv_sec * 1000000 + diff.tv_nsec / 1000) /
> -						sbc_data->frame_duration;
> +	timespec_diff(&cur, &ep->start, &diff);
> +	expected_samples = (diff.tv_sec * 1000000ll + diff.tv_nsec / 1000ll) *
> +						out->cfg.rate / 1000000ll;
> 
>  	/* AudioFlinger does not seem to provide any *working*
>  	 * API to provide data in some interval and will just
> @@ -531,9 +514,8 @@ static int write_media_packet(int fd, struct sbc_data
> *sbc_data, * lagging behind audio stream, we can sleep for
>  	 * duration of single media packet.
>  	 */
> -	if (sbc_data->frames_sent >= expected_frames)
> -		usleep(sbc_data->frame_duration *
> -				mp->payload.frame_count);
> +	if (ep->samples >= expected_samples)
> +		usleep(input_samples * 1000000 / out->cfg.rate);
> 
>  	return ret;
>  }
> @@ -574,53 +556,6 @@ static ssize_t sbc_encode_mediapacket(void *codec_data,
> const uint8_t *buffer, return consumed;
>  }
> 
> -static ssize_t sbc_write_data(void *codec_data, const void *buffer,
> -							size_t bytes, int fd)
> -{
> -	struct sbc_data *sbc_data = (struct sbc_data *) codec_data;
> -	size_t consumed = 0;
> -	struct media_packet *mp = (struct media_packet *) sbc_data->out_buf;
> -	size_t free_space = sbc_data->out_buf_size - sizeof(*mp);
> -
> -	mp->hdr.v = 2;
> -	mp->hdr.pt = 1;
> -	mp->hdr.ssrc = htonl(1);
> -
> -	while (consumed < bytes) {
> -		size_t written = 0;
> -		ssize_t read;
> -		int ret;
> -
> -		read = sbc_encode_mediapacket(codec_data, buffer + consumed,
> -						bytes - consumed, mp,
> -						free_space,
> -						&written);
> -
> -		if (read <= 0) {
> -			error("sbc_encode_mediapacket failed (%zd)", read);
> -			goto done;
> -		}
> -
> -		consumed += read;
> -
> -		mp->hdr.sequence_number = htons(sbc_data->seq++);
> -		mp->hdr.timestamp = htonl(sbc_data->timestamp);
> -
> -		/* AudioFlinger provides PCM 16bit stereo only, thus sample size
> -		 * is always 4 bytes
> -		 */
> -		sbc_data->timestamp += (read / 4);
> -
> -		ret = write_media_packet(fd, sbc_data, mp, written);
> -		if (ret < 0)
> -			return ret;
> -	}
> -
> -done:
> -	/* we always assume that all data was processed and sent */
> -	return bytes;
> -}
> -
>  static int audio_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len,
>  			void *param, size_t *rsp_len, void *rsp, int *fd)
>  {
> @@ -970,6 +905,13 @@ static bool open_endpoint(struct audio_endpoint *ep,
>  	codec->init(preset, mtu, &ep->codec_data);
>  	codec->get_config(ep->codec_data, cfg);
> 
> +	ep->mp = calloc(mtu, 1);
> +	ep->mp->hdr.v = 2;
> +	ep->mp->hdr.pt = 1;
> +	ep->mp->hdr.ssrc = htonl(1);
> +
> +	ep->mp_data_len = mtu - sizeof(*ep->mp);
> +
>  	free(preset);
> 
>  	return true;
> @@ -983,6 +925,8 @@ static void close_endpoint(struct audio_endpoint *ep)
>  		ep->fd = -1;
>  	}
> 
> +	free(ep->mp);
> +
>  	ep->codec->cleanup(ep->codec_data);
>  	ep->codec_data = NULL;
>  }
> @@ -1002,10 +946,59 @@ static void downmix_to_mono(struct a2dp_stream_out
> *out, const uint8_t *buffer, }
>  }
> 
> +static bool write_data(struct a2dp_stream_out *out, const void *buffer,
> +								size_t bytes)
> +{
> +	struct audio_endpoint *ep = out->ep;
> +	struct media_packet *mp = (struct media_packet *) ep->mp;
> +	size_t free_space = ep->mp_data_len;
> +	size_t consumed = 0;
> +
> +	while (consumed < bytes) {
> +		size_t written = 0;
> +		ssize_t read;
> +		uint32_t samples;
> +		int ret;
> +
> +		read = ep->codec->encode_mediapacket(ep->codec_data,
> +							buffer + consumed,
> +							bytes - consumed, mp,
> +							free_space, &written);
> +
> +		/* This is non-fatal and we can just assume buffer was processed
> +		 * properly and wait for next one.
> +		 */
> +		if (read <= 0)
> +			goto done;

I'd just return true here.

> +
> +		consumed += read;
> +
> +		mp->hdr.sequence_number = htons(ep->seq++);
> +		mp->hdr.timestamp = htonl(ep->samples);
> +
> +		/* AudioFlinger provides 16bit PCM, so sample size is 2 bytes
> +		 * multipled by number of channels. Number of channels is simply
> +		 * number of bits set in channels mask.
> +		 */
> +		samples = read / (2 * popcount(out->cfg.channels));
> +		ep->samples += samples;
> +
> +		ret = write_media_packet(out, written, samples);
> +		if (ret < 0)
> +			return false;
> +	}
> +
> +done:
> +	return true;
> +}
> +
> +

minor: double empty line here.

>  static ssize_t out_write(struct audio_stream_out *stream, const void
> *buffer, size_t bytes)
>  {
>  	struct a2dp_stream_out *out = (struct a2dp_stream_out *) stream;
> +	const void *in_buf = buffer;
> +	size_t in_len = bytes;
> 
>  	/* just return in case we're closing */
>  	if (out->audio_state == AUDIO_A2DP_STATE_NONE)
> @@ -1018,7 +1011,8 @@ static ssize_t out_write(struct audio_stream_out
> *stream, const void *buffer, if (ipc_resume_stream_cmd(out->ep->id) !=
> AUDIO_STATUS_SUCCESS) return -1;
> 
> -		out->ep->codec->resume(out->ep->codec_data);
> +		clock_gettime(CLOCK_MONOTONIC, &out->ep->start);
> +		out->ep->samples = 0;
> 
>  		out->audio_state = AUDIO_A2DP_STATE_STARTED;
>  	}
> @@ -1048,14 +1042,14 @@ static ssize_t out_write(struct audio_stream_out
> *stream, const void *buffer,
> 
>  		downmix_to_mono(out, buffer, bytes);
> 
> -		return out->ep->codec->write_data(out->ep->codec_data,
> -							out->downmix_buf,
> -							bytes / 2,
> -							out->ep->fd) * 2;
> +		in_buf = out->downmix_buf;
> +		in_len = bytes / 2;
>  	}
> 
> -	return out->ep->codec->write_data(out->ep->codec_data, buffer,
> -							bytes, out->ep->fd);
> +	if (!write_data(out, in_buf, in_len))
> +		return -1;
> +
> +	return bytes;
>  }
> 
>  static uint32_t out_get_sample_rate(const struct audio_stream *stream)

-- 
BR
Szymon Janc

^ permalink raw reply

* Re: [PATCH 1/5] android/hal-audio: Add open/close_endpoint helpers
From: Szymon Janc @ 2014-03-01 11:51 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1393610480-5775-2-git-send-email-andrzej.kaczmarek@tieto.com>

Hi Andrzej,

On Friday 28 of February 2014 19:01:16 Andrzej Kaczmarek wrote:
> ---
>  android/hal-audio.c | 112
> +++++++++++++++++++++++++++++----------------------- 1 file changed, 63
> insertions(+), 49 deletions(-)
> 
> diff --git a/android/hal-audio.c b/android/hal-audio.c
> index e1f3f0d..36e8d2e 100644
> --- a/android/hal-audio.c
> +++ b/android/hal-audio.c
> @@ -914,6 +914,67 @@ static void unregister_endpoints(void)
>  	}
>  }
> 
> +static int set_blocking(int fd)
> +{
> +	int flags;
> +
> +	flags = fcntl(fd, F_GETFL, 0);
> +	if (flags < 0) {
> +		error("fcntl(F_GETFL): %s (%d)", strerror(errno), errno);
> +		return -errno;

I'm aware that this function is just moved up but just to not forget it :)

error() can modify errno so it should be saved before call.
(this should be fixed in multiple places)

Also strerror() is not thread safe, we should probably start using 
strerror_r() (eg. have wrapping macros that we could use in multithreaded 
code). BT HAL has same problem so I guess this can be done outside of this 
patchset.

> +	}
> +
> +	if (fcntl(fd, F_SETFL, flags & ~O_NONBLOCK) < 0) {
> +		error("fcntl(F_SETFL): %s (%d)", strerror(errno), errno);
> +		return -errno;
> +	}
> +
> +	return 0;
> +}
> +
> +static bool open_endpoint(struct audio_endpoint *ep,
> +						struct audio_input_config *cfg)
> +{
> +	struct audio_preset *preset;
> +	const struct audio_codec *codec;
> +	uint16_t mtu;
> +	int fd;
> +
> +	if (ipc_open_stream_cmd(ep->id, &mtu, &fd, &preset) !=
> +							AUDIO_STATUS_SUCCESS)
> +		return false;
> +
> +	if (!preset || fd < 0)

If this can happen despite ipc_open_stream_cmd() returned success then freeing 
preset or closing fd should be handled here.

> +		return false;
> +
> +	if (set_blocking(fd) < 0) {
> +		free(preset);

Shouldn't fd be closed here?

> +		return false;
> +	}
> +
> +	ep->fd = fd;
> +
> +	codec = ep->codec;
> +	codec->init(preset, mtu, &ep->codec_data);
> +	codec->get_config(ep->codec_data, cfg);
> +
> +	free(preset);
> +
> +	return true;
> +}
> +
> +static void close_endpoint(struct audio_endpoint *ep)
> +{
> +	ipc_close_stream_cmd(ep->id);
> +	if (ep->fd >= 0) {
> +		close(ep->fd);
> +		ep->fd = -1;
> +	}
> +
> +	ep->codec->cleanup(ep->codec_data);
> +	ep->codec_data = NULL;
> +}
> +
>  static void downmix_to_mono(struct a2dp_stream_out *out, const uint8_t
> *buffer, size_t bytes)
>  {
> @@ -1260,24 +1321,6 @@ static int in_remove_audio_effect(const struct
> audio_stream *stream, return -ENOSYS;
>  }
> 
> -static int set_blocking(int fd)
> -{
> -	int flags;
> -
> -	flags = fcntl(fd, F_GETFL, 0);
> -	if (flags < 0) {
> -		error("fcntl(F_GETFL): %s (%d)", strerror(errno), errno);
> -		return -errno;
> -	}
> -
> -	if (fcntl(fd, F_SETFL, flags & ~O_NONBLOCK) < 0) {
> -		error("fcntl(F_SETFL): %s (%d)", strerror(errno), errno);
> -		return -errno;
> -	}
> -
> -	return 0;
> -}
> -
>  static int audio_open_output_stream(struct audio_hw_device *dev,
>  					audio_io_handle_t handle,
>  					audio_devices_t devices,
> @@ -1288,10 +1331,6 @@ static int audio_open_output_stream(struct
> audio_hw_device *dev, {
>  	struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *) dev;
>  	struct a2dp_stream_out *out;
> -	struct audio_preset *preset;
> -	const struct audio_codec *codec;
> -	uint16_t mtu;
> -	int fd;
> 
>  	out = calloc(1, sizeof(struct a2dp_stream_out));
>  	if (!out)
> @@ -1319,29 +1358,12 @@ static int audio_open_output_stream(struct
> audio_hw_device *dev, /* TODO: for now we always use endpoint 0 */
>  	out->ep = &audio_endpoints[0];
> 
> -	if (ipc_open_stream_cmd(out->ep->id, &mtu, &fd, &preset) !=
> -							AUDIO_STATUS_SUCCESS)
> -		goto fail;
> -
> -	if (!preset || fd < 0)
> -		goto fail;
> -
> -	if (set_blocking(fd) < 0) {
> -		free(preset);
> +	if (!open_endpoint(out->ep, &out->cfg))
>  		goto fail;
> -	}
> -
> -	out->ep->fd = fd;
> -	codec = out->ep->codec;
> -
> -	codec->init(preset, mtu, &out->ep->codec_data);
> -	codec->get_config(out->ep->codec_data, &out->cfg);
> 
>  	DBG("rate=%d channels=%d format=%d", out->cfg.rate,
>  					out->cfg.channels, out->cfg.format);
> 
> -	free(preset);
> -
>  	if (out->cfg.channels == AUDIO_CHANNEL_OUT_MONO) {
>  		out->downmix_buf = malloc(FIXED_BUFFER_SIZE / 2);
>  		if (!out->downmix_buf)
> @@ -1367,18 +1389,10 @@ static void audio_close_output_stream(struct
> audio_hw_device *dev, {
>  	struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *) dev;
>  	struct a2dp_stream_out *out = (struct a2dp_stream_out *) stream;
> -	struct audio_endpoint *ep = a2dp_dev->out->ep;
> 
>  	DBG("");
> 
> -	ipc_close_stream_cmd(ep->id);
> -	if (ep->fd >= 0) {
> -		close(ep->fd);
> -		ep->fd = -1;
> -	}
> -
> -	ep->codec->cleanup(ep->codec_data);
> -	ep->codec_data = NULL;
> +	close_endpoint(a2dp_dev->out->ep);
> 
>  	free(out->downmix_buf);

-- 
BR
Szymon Janc

^ permalink raw reply

* Re: Passive scanning of iBeacons results in a "Data Buffer Overflow"
From: Adam Warski @ 2014-03-01  9:34 UTC (permalink / raw)
  To: Anderson Lizardo; +Cc: BlueZ development
In-Reply-To: <CAJdJm_PMAVyiY-=13puBHAucCzObUWOSOXZ+maOL2z11YwOh-g@mail.gmail.com>


> For me looks like something got confused on the HCI packet parsing:
> either the kernel, hcidump, or something else. The HCI events after
> the LE meta event make no sense (they seem garbage). Did you try with
> btmon instead of hcidump ?

Yes, with btmon the same is happening. Also when just running “hcitool lescan --passive —duplicates” it stops getting advertisements after a while (without hcidump or btmon running in the background).

> Can you save the raw dump using "hcidump -w output.dump"  (or using
> btmon -w) and send to the list? It is easier to analyze, as the parser
> may be bogus.

Sure. The problems start at 63.120548.
The dump is here: http://www.warski.org/btmon_ibeacons.dat

Thanks,
Adam

-- 
Adam Warski

http://twitter.com/#!/adamwarski
http://www.softwaremill.com
http://www.warski.org


^ permalink raw reply


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