* obex-client: support authentication response?
From: Mike @ 2012-10-17 21:13 UTC (permalink / raw)
To: linux-bluetooth
I'm trying to pass TP/SSM/BV-6-C (respond to authentication requests)
for PBAP. As far as I can see, there is code in the obex server to
respond to challenge requests, but I don't think I see any for the
client. As for when it runs against PTS, obex-client initiates a
disconnect when it gets the unauthorized response code. Am I missing
something?
Thanks,
Mike
^ permalink raw reply
* Re: [PATCH BlueZ 2/2] core: Update gdbus function calls
From: Lucas De Marchi @ 2012-10-17 18:33 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <CAJdJm_OqOs4-U4E+nN9VQ362F0aWe+qJnyxWVF_ABscwQE+5wA@mail.gmail.com>
On Wed, Oct 17, 2012 at 1:25 PM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> Hi again,
>
> On Wed, Oct 17, 2012 at 1:20 PM, Anderson Lizardo
> <anderson.lizardo@openbossa.org> wrote:
>> Hi Lucas,
>>
>> On Wed, Oct 17, 2012 at 12:14 PM, Lucas De Marchi
>> <lucas.demarchi@profusion.mobi> wrote:
>>> @@ -827,24 +824,21 @@ static void set_name(struct btd_adapter *adapter, const char *name,
>>> int ret;
>>>
>>> if (adapter->allow_name_changes == FALSE)
>>> - return g_dbus_pending_property_error(btd_get_dbus_connection(),
>>> - id, ERROR_INTERFACE ".Failed",
>>> + return g_dbus_pending_property_error(id,
>>> + ERROR_INTERFACE ".Failed",
>>> strerror(EPERM));
>>>
>>> ret = adapter_set_name(adapter, name);
>>> - if (ret >= 0) {
>>> - g_dbus_pending_property_success(btd_get_dbus_connection(), id);
>>> - return;
>>> - }
>>> + if (ret >= 0)
>>> + return g_dbus_pending_property_success(id);
>>
>> Out of curiosity, is this style of returning "void" (instead of just
>> "return;") common in gdbus?
>
> Just realized that this is BlueZ specific code. Anyway, just found it
> odd (but answering my question, seems to be common).
It may be common because I used it a lot in the first conversions :-).
I don't know about other places using it. In gdbus-related code it's
very convenient because the function usually returns void as well, so
we can shortcut it with the g_dbus_pending_property_* calls.
Lucas De Marchi
^ permalink raw reply
* Re: [PATCHv2 17/19] Bluetooth: Send create channel request instead of connect for AMP
From: Marcel Holtmann @ 2012-10-17 17:24 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk
In-Reply-To: <1350430593-23565-18-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> When the channel policy is set to prefer AMP, then an L2CAP channel is
> set up using the "create channel" command rather than the "connect"
> command. A physical link is also set up before sending "create
> channel".
>
> Behavior is unchanged if enable_hs is false.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap_core.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 7e8fe84..17d917b 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -58,6 +58,9 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn,
> static void l2cap_tx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
> struct sk_buff_head *skbs, u8 event);
>
> +static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
> + u8 status);
> +
any chance to avoid this forward declaration by moving some functions
around?
> /* ---- L2CAP channels ---- */
>
> static struct l2cap_chan *__l2cap_get_chan_by_dcid(struct l2cap_conn *conn,
> @@ -576,6 +579,13 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
> mgr->bredr_chan = NULL;
> }
>
> + if (chan->hs_hchan) {
> + chan->hs_hchan = NULL;
> + chan->hs_hcon = NULL;
> +
> + /* Placeholder - free logical link */
> + }
> +
> chan->ops->teardown(chan, err);
>
> if (test_bit(CONF_NOT_COMPLETE, &chan->conf_state))
> @@ -1126,6 +1136,7 @@ static void l2cap_start_connection(struct l2cap_chan *chan)
> {
> if (__amp_capable(chan)) {
> BT_DBG("chan %p AMP capable: discover AMPs", chan);
> + set_bit(CONF_CONNECT_PEND, &chan->conf_state);
> a2mp_discover_amp(chan);
> } else {
> l2cap_send_conn_req(chan);
> @@ -1271,6 +1282,16 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
> rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHEN_PEND);
> }
>
> + if (rsp.result == __constant_cpu_to_le16(L2CAP_CR_SUCCESS) &&
> + chan->local_amp_id) {
> + /* Placeholder - uncomment when amp functions
> + * are available
> + amp_accept_physical(chan, chan->local_amp_id);
> + */
> + l2cap_chan_unlock(chan);
> + continue;
> + }
> +
> l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_RSP,
> sizeof(rsp), &rsp);
>
> @@ -3359,6 +3380,18 @@ done:
> rfc.mode = chan->mode;
> }
>
> + if (test_bit(CONF_LOC_CONF_PEND, &chan->conf_state) &&
> + chan->local_amp_id) {
> + struct hci_chan *hchan = NULL;
> +
> + /* Placeholder - get hci_chan for logical link */
> +
> + if (hchan && hchan->state == BT_CONNECTED) {
> + l2cap_logical_cfm(chan, hchan,
> + L2CAP_MR_SUCCESS);
> + }
> + }
> +
> if (result == L2CAP_CONF_SUCCESS)
> set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
> }
> @@ -6367,6 +6400,15 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> stat = L2CAP_CS_AUTHOR_PEND;
> chan->ops->defer(chan);
> } else {
> + if (chan->local_amp_id) {
> + /* Placeholder - accept physical
> + * link
> + amp_accept_physical(chan,
> + chan->local_amp_id);
> + */
> + continue;
> + }
> +
> __l2cap_state_change(chan, BT_CONFIG);
> res = L2CAP_CR_SUCCESS;
> stat = L2CAP_CS_NO_INFO;
Check the forward declaration and fix it if possible. Otherwise this is
fine.
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv2 10/19] Bluetooth: Add logical link confirm
From: Marcel Holtmann @ 2012-10-17 17:22 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk
In-Reply-To: <1350430593-23565-11-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> The logical link confirm callback is executed when the AMP controller
> completes its logical link setup. During a channel move, a newly
> formed logical link allows a move responder to send a move channel
> response. A move initiator will send a move channel confirm. A
> failed logical link will end the channel move and send an appropriate
> response or confirm command indicating a failure.
>
> If the channel is being created on an AMP controller, L2CAP
> configuration is started after the logical link is set up.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap_core.c | 114 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 112 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 5e4796c..ddd8c72 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -3799,6 +3799,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn,
> goto unlock;
> }
>
> + chan->ident = cmd->ident;
> l2cap_send_cmd(conn, cmd->ident, L2CAP_CONF_RSP, len, rsp);
> chan->num_conf_rsp++;
>
> @@ -4241,11 +4242,120 @@ static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
> l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
> }
>
> +/* Call with chan locked */
> static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
> u8 status)
> {
> - /* Placeholder */
> - return;
> + BT_DBG("chan %p, hchan %p, status %d", chan, hchan, status);
> +
> + if (status) {
> + /* Logical link setup failed */
> + if (chan->state != BT_CONNECTED) {
> + /* Create channel failure, disconnect */
> + l2cap_send_disconn_req(chan->conn, chan, ECONNRESET);
> + } else if (chan->move_role == L2CAP_MOVE_ROLE_RESPONDER) {
> + l2cap_move_revert(chan);
> + chan->move_role = L2CAP_MOVE_ROLE_NONE;
> + chan->move_state = L2CAP_MOVE_STABLE;
> + l2cap_send_move_chan_rsp(chan->conn, chan->ident,
> + chan->dcid, L2CAP_MR_NOT_SUPP);
> + } else if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
> + if (chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_COMP ||
> + chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_CFM) {
> + /* Remote has only sent pending or
> + * success responses, clean up
> + */
> + l2cap_move_revert(chan);
> + chan->move_role = L2CAP_MOVE_ROLE_NONE;
> + chan->move_state = L2CAP_MOVE_STABLE;
> + }
> +
> + /* Other amp move states imply that the move
> + * has already aborted
> + */
> + l2cap_send_move_chan_cfm(chan->conn, chan, chan->scid,
> + L2CAP_MC_UNCONFIRMED);
> + __set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
> + }
> +
> + chan->hs_hchan = NULL;
> + chan->hs_hcon = NULL;
> +
> + /* Placeholder - free logical link */
> +
> + } else if (chan->state != BT_CONNECTED) {
> + struct l2cap_conf_rsp rsp;
> + u8 code;
> +
> + /* Create channel complete */
> +
> + /* Ignore logical link if channel is on BR/EDR */
> + if (!chan->local_amp_id)
> + return;
> +
> + chan->hs_hcon = hchan->conn;
> + chan->hs_hcon->l2cap_data = chan->conn;
> +
> + code = l2cap_build_conf_rsp(chan, &rsp,
> + L2CAP_CONF_SUCCESS, 0);
> + l2cap_send_cmd(chan->conn, chan->ident, L2CAP_CONF_RSP, code,
> + &rsp);
> + set_bit(CONF_OUTPUT_DONE, &chan->conf_state);
> +
> + if (test_bit(CONF_INPUT_DONE, &chan->conf_state)) {
> + int err = 0;
> +
> + set_default_fcs(chan);
> +
> + err = l2cap_ertm_init(chan);
> + if (err < 0)
> + l2cap_send_disconn_req(chan->conn, chan, -err);
> + else
> + l2cap_chan_ready(chan);
> + }
> + } else {
> + /* Channel move */
> + chan->hs_hcon = hchan->conn;
> + chan->hs_hcon->l2cap_data = chan->conn;
> +
> + BT_DBG("move_state %d", chan->move_state);
> +
> + switch (chan->move_state) {
> + case L2CAP_MOVE_WAIT_LOGICAL_COMP:
> + /* Move confirm will be sent after a success
> + * response is received
> + */
> + chan->move_state = L2CAP_MOVE_WAIT_RSP_SUCCESS;
> + break;
> + case L2CAP_MOVE_WAIT_LOGICAL_CFM:
> + if (test_bit(CONN_LOCAL_BUSY, &chan->conn_state)) {
> + chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
> + } else if (chan->move_role ==
> + L2CAP_MOVE_ROLE_INITIATOR) {
> + chan->move_state = L2CAP_MOVE_WAIT_CONFIRM_RSP;
> + l2cap_send_move_chan_cfm(chan->conn, chan,
> + chan->scid,
> + L2CAP_MR_SUCCESS);
> + __set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
> + } else if (chan->move_role ==
> + L2CAP_MOVE_ROLE_RESPONDER) {
> + chan->move_state = L2CAP_MOVE_WAIT_CONFIRM;
> + l2cap_send_move_chan_rsp(chan->conn,
> + chan->ident,
> + chan->dcid,
> + L2CAP_MR_SUCCESS);
> + }
> + break;
> + default:
> + /* Move was not in expected state, free the channel */
> + chan->hs_hchan = NULL;
> + chan->hs_hcon = NULL;
> +
> + /* Placeholder - free the logical link */
> +
> + chan->move_state = L2CAP_MOVE_STABLE;
> + }
> + }
> }
>
I find this this function still a little bit too complex. Any chance we
can split into more logical pieces. It is fine if not, but we should at
least give it another try.
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv2 09/19] Bluetooth: Move channel response
From: Marcel Holtmann @ 2012-10-17 17:20 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk
In-Reply-To: <1350430593-23565-10-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> The move response command includes a result code indicationg
> "pending", "success", or "failure" status. A pending result is
> received when the remote address is still setting up a physical link,
> and will be followed by success or failure. On success, logical link
> setup will proceed. On failure, the move is stopped. The receiver of
> a move channel response must always follow up by sending a move
> channel confirm command.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> include/net/bluetooth/l2cap.h | 2 +
> net/bluetooth/l2cap_core.c | 161 ++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 158 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 6d3615e..b4c3c65 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -52,6 +52,8 @@
> #define L2CAP_ENC_TIMEOUT msecs_to_jiffies(5000)
> #define L2CAP_CONN_TIMEOUT msecs_to_jiffies(40000)
> #define L2CAP_INFO_TIMEOUT msecs_to_jiffies(4000)
> +#define L2CAP_MOVE_TIMEOUT msecs_to_jiffies(4000)
> +#define L2CAP_MOVE_ERTX_TIMEOUT msecs_to_jiffies(60000)
>
> #define L2CAP_A2MP_DEFAULT_MTU 670
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index ed2c23f..5e4796c 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -128,6 +128,20 @@ static struct l2cap_chan *__l2cap_get_chan_by_ident(struct l2cap_conn *conn,
> return NULL;
> }
>
> +static struct l2cap_chan *l2cap_get_chan_by_ident(struct l2cap_conn *conn,
> + u8 ident)
> +{
> + struct l2cap_chan *c;
> +
> + mutex_lock(&conn->chan_lock);
> + c = __l2cap_get_chan_by_ident(conn, ident);
> + if (c)
> + l2cap_chan_lock(c);
> + mutex_unlock(&conn->chan_lock);
> +
> + return c;
> +}
> +
> static struct l2cap_chan *__l2cap_global_chan_by_addr(__le16 psm, bdaddr_t *src)
> {
> struct l2cap_chan *c;
> @@ -4227,6 +4241,13 @@ static void l2cap_send_move_chan_cfm_rsp(struct l2cap_conn *conn, u8 ident,
> l2cap_send_cmd(conn, ident, L2CAP_MOVE_CHAN_CFM_RSP, sizeof(rsp), &rsp);
> }
>
> +static void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
> + u8 status)
> +{
> + /* Placeholder */
> + return;
> +}
> +
> static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
> struct l2cap_cmd_hdr *cmd,
> u16 cmd_len, void *data)
> @@ -4317,9 +4338,137 @@ send_move_response:
> return 0;
> }
>
> -static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
> - struct l2cap_cmd_hdr *cmd,
> - u16 cmd_len, void *data)
> +static void l2cap_move_continue(struct l2cap_conn *conn, u16 icid, u16 result)
> +{
> + struct l2cap_chan *chan;
> +
> + chan = l2cap_get_chan_by_scid(conn, icid);
> + if (!chan) {
> + l2cap_send_move_chan_cfm(conn, NULL, icid,
> + L2CAP_MC_UNCONFIRMED);
> + return;
> + }
> +
> + __clear_chan_timer(chan);
> + if (result == L2CAP_MR_PEND)
> + __set_chan_timer(chan, L2CAP_MOVE_ERTX_TIMEOUT);
> +
> + if (chan->move_state == L2CAP_MOVE_WAIT_LOGICAL_COMP) {
> + /* Move confirm will be sent when logical link
> + * is complete.
> + */
> + chan->move_state = L2CAP_MOVE_WAIT_LOGICAL_CFM;
> + } else if (chan->move_state == L2CAP_MOVE_WAIT_RSP_SUCCESS) {
> + if (result == L2CAP_MR_PEND) {
> + goto done;
> + } else if (test_bit(CONN_LOCAL_BUSY,
> + &chan->conn_state)) {
> + chan->move_state = L2CAP_MOVE_WAIT_LOCAL_BUSY;
> + } else {
> + /* Logical link is up or moving to BR/EDR,
> + * proceed with move */
> + chan->move_state = L2CAP_MOVE_WAIT_CONFIRM_RSP;
> + l2cap_send_move_chan_cfm(conn, chan, chan->scid,
> + L2CAP_MC_CONFIRMED);
> + __set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
> + }
> + } else if (chan->move_state == L2CAP_MOVE_WAIT_RSP) {
> + struct hci_chan *hchan = NULL;
> + /* Moving to AMP */
> + if (result == L2CAP_MR_SUCCESS) {
> + /* Remote is ready, send confirm immediately
> + * after logical link is ready
> + */
> + chan->move_state = L2CAP_MOVE_WAIT_LOGICAL_CFM;
> + } else {
> + /* Both logical link and move success
> + * are required to confirm
> + */
> + chan->move_state = L2CAP_MOVE_WAIT_LOGICAL_COMP;
> + }
> +
> + /* Placeholder - get hci_chan for logical link */
> + if (!hchan) {
> + /* Logical link not available */
> + l2cap_send_move_chan_cfm(conn, chan, chan->scid,
> + L2CAP_MC_UNCONFIRMED);
> + goto done;
> + }
> +
> + /* If the logical link is not yet connected, do not
> + * send confirmation.
> + */
> + if (hchan->state != BT_CONNECTED)
> + goto done;
> +
> + /* Logical link is already ready to go */
> +
> + chan->hs_hcon = hchan->conn;
> + chan->hs_hcon->l2cap_data = chan->conn;
> +
> + if (result == L2CAP_MR_SUCCESS) {
> + /* Can confirm now */
> + l2cap_send_move_chan_cfm(conn, chan, chan->scid,
> + L2CAP_MC_CONFIRMED);
> + } else {
> + /* Now only need move success
> + * to confirm
> + */
> + chan->move_state = L2CAP_MOVE_WAIT_RSP_SUCCESS;
> + }
> +
> + l2cap_logical_cfm(chan, hchan, L2CAP_MR_SUCCESS);
> + } else {
> + /* Any other amp move state means the move failed. */
> + chan->move_id = chan->local_amp_id;
> + chan->move_state = L2CAP_MOVE_STABLE;
> + l2cap_move_revert(chan);
> + chan->move_role = L2CAP_MOVE_ROLE_NONE;
> + l2cap_send_move_chan_cfm(conn, chan, chan->scid,
> + L2CAP_MC_UNCONFIRMED);
> + __set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
> + }
can you just use a switch statement here.
> +
> +done:
> + l2cap_chan_unlock(chan);
> +}
> +
> +static void l2cap_move_fail(struct l2cap_conn *conn, u8 ident, u16 icid,
> + u16 result)
> +{
> + struct l2cap_chan *chan;
> +
> + chan = l2cap_get_chan_by_ident(conn, ident);
> + if (!chan) {
> + /* Could not locate channel, icid is best guess */
> + l2cap_send_move_chan_cfm(conn, NULL, icid,
> + L2CAP_MC_UNCONFIRMED);
> + return;
> + }
> +
> + __clear_chan_timer(chan);
> +
> + if (chan->move_role == L2CAP_MOVE_ROLE_INITIATOR) {
> + if (result == L2CAP_MR_COLLISION) {
> + chan->move_role = L2CAP_MOVE_ROLE_RESPONDER;
> + } else {
> + /* Cleanup - cancel move */
> + chan->move_id = chan->local_amp_id;
> + chan->move_state = L2CAP_MOVE_STABLE;
> + l2cap_move_revert(chan);
> + chan->move_role = L2CAP_MOVE_ROLE_NONE;
> + }
> + }
> +
> + l2cap_send_move_chan_cfm(conn, chan, chan->scid, L2CAP_MC_UNCONFIRMED);
> + __set_chan_timer(chan, L2CAP_MOVE_TIMEOUT);
> +
> + l2cap_chan_unlock(chan);
> +}
> +
> +static int l2cap_move_channel_rsp(struct l2cap_conn *conn,
> + struct l2cap_cmd_hdr *cmd,
> + u16 cmd_len, void *data)
> {
> struct l2cap_move_chan_rsp *rsp = data;
> u16 icid, result;
> @@ -4332,8 +4481,10 @@ static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
>
> BT_DBG("icid 0x%4.4x, result 0x%4.4x", icid, result);
>
> - /* Placeholder: Always unconfirmed */
> - l2cap_send_move_chan_cfm(conn, NULL, icid, L2CAP_MC_UNCONFIRMED);
> + if (result == L2CAP_MR_SUCCESS || result == L2CAP_MR_PEND)
> + l2cap_move_continue(conn, icid, result);
> + else
> + l2cap_move_fail(conn, cmd->ident, icid, result);
>
> return 0;
> }
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv2 07/19] Bluetooth: Add move channel confirm handling
From: Marcel Holtmann @ 2012-10-17 17:16 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk
In-Reply-To: <1350430593-23565-8-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> After sending a move channel response, a move responder waits for a
> move channel confirm command. If the received command has a
> "confirmed" result the move is proceeding, and "unconfirmed" means the
> move has failed and the channel will not change controllers.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap_core.c | 70 ++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 67 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 9663292..ed2c23f 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -1036,6 +1036,42 @@ static void l2cap_move_setup(struct l2cap_chan *chan)
> set_bit(CONN_REMOTE_BUSY, &chan->conn_state);
> }
>
> +static void l2cap_move_success(struct l2cap_chan *chan)
> +{
> + BT_DBG("chan %p", chan);
> +
> + if (chan->mode != L2CAP_MODE_ERTM)
> + return;
> +
> + switch (chan->move_role) {
> + case L2CAP_MOVE_ROLE_INITIATOR:
> + l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
> + chan->rx_state = L2CAP_RX_STATE_WAIT_F;
> + break;
> + case L2CAP_MOVE_ROLE_RESPONDER:
> + chan->rx_state = L2CAP_RX_STATE_WAIT_P;
> + break;
> + }
> +}
> +
> +static void l2cap_move_revert(struct l2cap_chan *chan)
> +{
> + BT_DBG("chan %p", chan);
> +
> + if (chan->mode != L2CAP_MODE_ERTM)
> + return;
> +
> + switch (chan->move_role) {
> + case L2CAP_MOVE_ROLE_INITIATOR:
> + l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
> + chan->rx_state = L2CAP_RX_STATE_WAIT_F;
> + break;
> + case L2CAP_MOVE_ROLE_RESPONDER:
> + chan->rx_state = L2CAP_RX_STATE_WAIT_P;
> + break;
> + }
> +}
> +
> static void l2cap_chan_ready(struct l2cap_chan *chan)
> {
> /* This clears all conf flags, including CONF_NOT_COMPLETE */
> @@ -4302,11 +4338,12 @@ static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
> return 0;
> }
>
> -static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
> - struct l2cap_cmd_hdr *cmd,
> - u16 cmd_len, void *data)
> +static int l2cap_move_channel_confirm(struct l2cap_conn *conn,
> + struct l2cap_cmd_hdr *cmd,
> + u16 cmd_len, void *data)
> {
> struct l2cap_move_chan_cfm *cfm = data;
> + struct l2cap_chan *chan;
> u16 icid, result;
>
> if (cmd_len != sizeof(*cfm))
> @@ -4317,8 +4354,35 @@ static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
>
> BT_DBG("icid 0x%4.4x, result 0x%4.4x", icid, result);
>
> + chan = l2cap_get_chan_by_dcid(conn, icid);
> + if (!chan)
> + goto send_move_confirm_response;
> +
> + if (chan->move_state == L2CAP_MOVE_WAIT_CONFIRM) {
> + chan->move_state = L2CAP_MOVE_STABLE;
> + if (result == L2CAP_MC_CONFIRMED) {
> + chan->local_amp_id = chan->move_id;
> + if (!chan->local_amp_id) {
> + /* Have moved off of AMP, free the channel */
> + chan->hs_hchan = NULL;
> + chan->hs_hcon = NULL;
> +
> + /* Placeholder - free the logical link */
> + }
> + l2cap_move_success(chan);
> + } else {
> + chan->move_id = chan->local_amp_id;
> + l2cap_move_revert(chan);
> + }
> + chan->move_role = L2CAP_MOVE_ROLE_NONE;
> + }
> +
> +send_move_confirm_response:
> l2cap_send_move_chan_cfm_rsp(conn, cmd->ident, icid);
>
> + if (chan)
> + l2cap_chan_unlock(chan);
> +
still not a big fan of the if (chan) check before the unlock. This way
of dealing with locks makes my brain hurt ;)
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv2 06/19] Bluetooth: Add new ERTM receive states for channel move
From: Marcel Holtmann @ 2012-10-17 17:13 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk
In-Reply-To: <1350430593-23565-7-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> Two new states are required to implement channel moves with the ERTM
> receive state machine.
>
> The "WAIT_P" state is used by a move responder to wait for a "poll"
> flag after a move is completed (success or failure). "WAIT_F" is
> similarly used by a move initiator to wait for a "final" flag when the
> move is completing. In either state, the reqseq value in the
> poll/final frame tells the state machine exactly which frame should be
> expected next.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap_core.c | 104 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 104 insertions(+)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv2 04/19] Bluetooth: Lookup channel structure based on DCID
From: Marcel Holtmann @ 2012-10-17 17:12 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk
In-Reply-To: <1350430593-23565-5-git-send-email-mathewm@codeaurora.org>
Hi Mat,
> Processing a move channel request involves getting the channel
> structure using the destination channel ID. Previous code could only
> look up using the source channel ID.
>
> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> ---
> net/bluetooth/l2cap_core.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 07/19] Bluetooth: Add move channel confirm handling
From: Marcel Holtmann @ 2012-10-17 17:10 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, gustavo, sunnyk
In-Reply-To: <alpine.DEB.2.02.1210161020250.13129@mathewm-linux>
Hi Mat,
> >> After sending a move channel response, a move responder waits for a
> >> move channel confirm command. If the received command has a
> >> "confirmed" result the move is proceeding, and "unconfirmed" means the
> >> move has failed and the channel will not change controllers.
> >>
> >> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
> >> ---
> >> net/bluetooth/l2cap_core.c | 70 ++++++++++++++++++++++++++++++++++++++++++++--
> >> 1 file changed, 67 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> >> index aab7f79..ef744a9 100644
> >> --- a/net/bluetooth/l2cap_core.c
> >> +++ b/net/bluetooth/l2cap_core.c
> >> @@ -1030,6 +1030,42 @@ static void l2cap_move_setup(struct l2cap_chan *chan)
> >> set_bit(CONN_REMOTE_BUSY, &chan->conn_state);
> >> }
> >>
> >> +static void l2cap_move_success(struct l2cap_chan *chan)
> >> +{
> >> + BT_DBG("chan %p", chan);
> >> +
> >> + if (chan->mode != L2CAP_MODE_ERTM)
> >> + return;
> >> +
> >> + switch (chan->move_role) {
> >> + case L2CAP_MOVE_ROLE_INITIATOR:
> >> + l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
> >> + chan->rx_state = L2CAP_RX_STATE_WAIT_F;
> >> + break;
> >> + case L2CAP_MOVE_ROLE_RESPONDER:
> >> + chan->rx_state = L2CAP_RX_STATE_WAIT_P;
> >> + break;
> >> + }
> >> +}
> >> +
> >> +static void l2cap_move_revert(struct l2cap_chan *chan)
> >> +{
> >> + BT_DBG("chan %p", chan);
> >> +
> >> + if (chan->mode != L2CAP_MODE_ERTM)
> >> + return;
> >> +
> >> + switch (chan->move_role) {
> >> + case L2CAP_MOVE_ROLE_INITIATOR:
> >> + l2cap_tx(chan, NULL, NULL, L2CAP_EV_EXPLICIT_POLL);
> >> + chan->rx_state = L2CAP_RX_STATE_WAIT_F;
> >> + break;
> >> + case L2CAP_MOVE_ROLE_RESPONDER:
> >> + chan->rx_state = L2CAP_RX_STATE_WAIT_P;
> >> + break;
> >> + }
> >> +}
> >> +
> >> static void l2cap_chan_ready(struct l2cap_chan *chan)
> >> {
> >> /* This clears all conf flags, including CONF_NOT_COMPLETE */
> >> @@ -4297,11 +4333,12 @@ static inline int l2cap_move_channel_rsp(struct l2cap_conn *conn,
> >> return 0;
> >> }
> >>
> >> -static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
> >> - struct l2cap_cmd_hdr *cmd,
> >> - u16 cmd_len, void *data)
> >> +static int l2cap_move_channel_confirm(struct l2cap_conn *conn,
> >> + struct l2cap_cmd_hdr *cmd,
> >> + u16 cmd_len, void *data)
> >> {
> >> struct l2cap_move_chan_cfm *cfm = data;
> >> + struct l2cap_chan *chan;
> >> u16 icid, result;
> >>
> >> if (cmd_len != sizeof(*cfm))
> >> @@ -4312,8 +4349,35 @@ static inline int l2cap_move_channel_confirm(struct l2cap_conn *conn,
> >>
> >> BT_DBG("icid 0x%4.4x, result 0x%4.4x", icid, result);
> >>
> >> + chan = l2cap_get_chan_by_dcid(conn, icid);
> >> + if (!chan)
> >> + goto send_move_confirm_response;
> >> +
> >> + if (chan->move_state == L2CAP_MOVE_WAIT_CONFIRM) {
> >> + chan->move_state = L2CAP_MOVE_STABLE;
> >> + if (result == L2CAP_MC_CONFIRMED) {
> >> + chan->local_amp_id = chan->move_id;
> >> + if (!chan->local_amp_id) {
> >> + /* Have moved off of AMP, free the channel */
> >> + chan->hs_hchan = NULL;
> >> + chan->hs_hcon = NULL;
> >> +
> >> + /* Placeholder - free the logical link */
> >> + }
> >> + l2cap_move_success(chan);
> >> + } else {
> >> + chan->move_id = chan->local_amp_id;
> >> + l2cap_move_revert(chan);
> >> + }
> >> + chan->move_role = L2CAP_MOVE_ROLE_NONE;
> >> + }
> >> +
> >> +send_move_confirm_response:
> >> l2cap_send_move_chan_cfm_rsp(conn, cmd->ident, icid);
> >>
> >> + if (chan)
> >> + l2cap_chan_unlock(chan);
> >> +
> >> return 0;
> >> }
> >>
> >
> > the more I read into this one, can we not have a clean exit when (!chan)
> > for this one. Or am I missing something.
>
> Are you thinking this should send a COMMAND_REJ if it can't find the
> matching channel?
>
> I think sending the confirm response is the appropriate thing to do.
> The spec says "When a device receives a Move Channel Confirmation
> packet it shall send a Move Channel Confirmation response packet".
> Sending this response doesn't indicate that the move was successful.
I am fine with sending a response packet. However we might have to make
it explicit a response with success and response with failure. My
concern is that the code becomes complex and hard to follow.
Regards
Marcel
^ permalink raw reply
* Re: [RFCv0 0/8] Handling physical and logical link
From: Marcel Holtmann @ 2012-10-17 17:07 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1350399670-12418-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
> Set of patches adding handling for physical and logical link
> complete events.
>
> Andrei Emeltchenko (8):
> Bluetooth: AMP: Process Physical Link Complete evt
> Bluetooth: AMP: Process Logical Link complete evt
> Bluetooth: Add logical link create function
> Bluetooth: AMP: Process Disc Logical Link
> Bluetooth: AMP: Process Disc Physical Link complete evt
> Bluetooth: AMP: Remove hci_conn receiving error command status
> Bluetooth: Disconnect logical link when deleteing chan
> Bluetooth: Add put(hcon) when deleting hchan
I looked over the patches and the look fine to me.
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Regards
Marcel
^ permalink raw reply
* Re: [PATCH BlueZ 2/2] core: Update gdbus function calls
From: Anderson Lizardo @ 2012-10-17 16:25 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
In-Reply-To: <CAJdJm_OoM=Ku7pESgqvXEOJO3jB21bdGHfA368uiTnfoxb=bJw@mail.gmail.com>
Hi again,
On Wed, Oct 17, 2012 at 1:20 PM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> Hi Lucas,
>
> On Wed, Oct 17, 2012 at 12:14 PM, Lucas De Marchi
> <lucas.demarchi@profusion.mobi> wrote:
>> @@ -827,24 +824,21 @@ static void set_name(struct btd_adapter *adapter, const char *name,
>> int ret;
>>
>> if (adapter->allow_name_changes == FALSE)
>> - return g_dbus_pending_property_error(btd_get_dbus_connection(),
>> - id, ERROR_INTERFACE ".Failed",
>> + return g_dbus_pending_property_error(id,
>> + ERROR_INTERFACE ".Failed",
>> strerror(EPERM));
>>
>> ret = adapter_set_name(adapter, name);
>> - if (ret >= 0) {
>> - g_dbus_pending_property_success(btd_get_dbus_connection(), id);
>> - return;
>> - }
>> + if (ret >= 0)
>> + return g_dbus_pending_property_success(id);
>
> Out of curiosity, is this style of returning "void" (instead of just
> "return;") common in gdbus?
Just realized that this is BlueZ specific code. Anyway, just found it
odd (but answering my question, seems to be common).
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH BlueZ 2/2] core: Update gdbus function calls
From: Anderson Lizardo @ 2012-10-17 16:20 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
In-Reply-To: <1350486869-5020-2-git-send-email-lucas.demarchi@profusion.mobi>
Hi Lucas,
On Wed, Oct 17, 2012 at 12:14 PM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> wrote:
> @@ -827,24 +824,21 @@ static void set_name(struct btd_adapter *adapter, const char *name,
> int ret;
>
> if (adapter->allow_name_changes == FALSE)
> - return g_dbus_pending_property_error(btd_get_dbus_connection(),
> - id, ERROR_INTERFACE ".Failed",
> + return g_dbus_pending_property_error(id,
> + ERROR_INTERFACE ".Failed",
> strerror(EPERM));
>
> ret = adapter_set_name(adapter, name);
> - if (ret >= 0) {
> - g_dbus_pending_property_success(btd_get_dbus_connection(), id);
> - return;
> - }
> + if (ret >= 0)
> + return g_dbus_pending_property_success(id);
Out of curiosity, is this style of returning "void" (instead of just
"return;") common in gdbus?
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* [PATCH BlueZ 2/2] core: Update gdbus function calls
From: Lucas De Marchi @ 2012-10-17 15:14 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
In-Reply-To: <1350486869-5020-1-git-send-email-lucas.demarchi@profusion.mobi>
Done by the following semantic patch, with manual tweaks afterwards due
to changes in 80-chars line breaks:
// <smpl>
@r1 @
expression E1;
@@
g_dbus_pending_property_success(
- E1,
...)
@r2 @
expression E1;
@@
g_dbus_pending_property_error(
- E1,
...)
@r3 @
expression E1;
@@
g_dbus_pending_property_error_valist(
- E1,
...)
// </smpl>
---
src/adapter.c | 91 +++++++++++++++++++++++++++--------------------------------
src/device.c | 72 ++++++++++++++++++----------------------------
2 files changed, 69 insertions(+), 94 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index 7c2aec0..e5d4cf7 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -346,7 +346,6 @@ static void set_session_pending_mode(struct btd_adapter *adapter,
static void set_discoverable(struct btd_adapter *adapter,
gboolean discoverable, GDBusPendingPropertySet id)
{
- DBusConnection *conn = btd_get_dbus_connection();
uint8_t mode;
int err;
@@ -354,12 +353,12 @@ static void set_discoverable(struct btd_adapter *adapter,
if (mode == adapter->mode) {
adapter->global_mode = mode;
- return g_dbus_pending_property_success(conn, id);
+ return g_dbus_pending_property_success(id);
}
err = set_mode(adapter, mode);
if (err < 0)
- return g_dbus_pending_property_error(conn, id,
+ return g_dbus_pending_property_error(id,
ERROR_INTERFACE ".Failed",
strerror(-err));
@@ -371,7 +370,6 @@ static void set_discoverable(struct btd_adapter *adapter,
static void set_powered(struct btd_adapter *adapter, gboolean powered,
GDBusPendingPropertySet id)
{
- DBusConnection *conn = btd_get_dbus_connection();
uint8_t mode;
int err;
@@ -385,12 +383,12 @@ static void set_powered(struct btd_adapter *adapter, gboolean powered,
if (mode == adapter->mode) {
adapter->global_mode = mode;
- return g_dbus_pending_property_success(conn, id);
+ return g_dbus_pending_property_success(id);
}
err = set_mode(adapter, mode);
if (err < 0)
- return g_dbus_pending_property_error(conn, id,
+ return g_dbus_pending_property_error(id,
ERROR_INTERFACE ".Failed",
strerror(-err));
@@ -402,11 +400,10 @@ static void set_powered(struct btd_adapter *adapter, gboolean powered,
static void set_pairable(struct btd_adapter *adapter, gboolean pairable,
bool reply, GDBusPendingPropertySet id)
{
- DBusConnection *conn = btd_get_dbus_connection();
int err;
if (adapter->scan_mode == SCAN_DISABLED)
- return g_dbus_pending_property_error(conn, id,
+ return g_dbus_pending_property_error(id,
ERROR_INTERFACE ".NotReady",
"Resource Not Ready");
@@ -419,7 +416,7 @@ static void set_pairable(struct btd_adapter *adapter, gboolean pairable,
err = set_mode(adapter, MODE_DISCOVERABLE);
if (err < 0) {
if (reply)
- g_dbus_pending_property_error(conn, id,
+ g_dbus_pending_property_error(id,
ERROR_INTERFACE ".Failed",
strerror(-err));
return;
@@ -430,7 +427,7 @@ store:
done:
if (reply)
- g_dbus_pending_property_success(conn, id);
+ g_dbus_pending_property_success(id);
}
static gboolean pairable_timeout_handler(void *data)
@@ -716,7 +713,7 @@ static void set_discoverable_timeout(struct btd_adapter *adapter,
DBusConnection *conn = btd_get_dbus_connection();
if (adapter->discov_timeout == timeout && timeout == 0)
- return g_dbus_pending_property_success(conn, id);
+ return g_dbus_pending_property_success(id);
if (adapter->scan_mode & SCAN_INQUIRY)
mgmt_set_discoverable(adapter->dev_id, TRUE, timeout);
@@ -727,7 +724,7 @@ static void set_discoverable_timeout(struct btd_adapter *adapter,
g_dbus_emit_property_changed(conn, adapter->path, ADAPTER_INTERFACE,
"DiscoverableTimeout");
- g_dbus_pending_property_success(conn, id);
+ g_dbus_pending_property_success(id);
}
static void set_pairable_timeout(struct btd_adapter *adapter,
@@ -736,7 +733,7 @@ static void set_pairable_timeout(struct btd_adapter *adapter,
DBusConnection *conn = btd_get_dbus_connection();
if (adapter->pairable_timeout == timeout && timeout == 0)
- return g_dbus_pending_property_success(conn, id);
+ return g_dbus_pending_property_success(id);
if (adapter->pairable)
adapter_set_pairable_timeout(adapter, timeout);
@@ -747,7 +744,7 @@ static void set_pairable_timeout(struct btd_adapter *adapter,
g_dbus_emit_property_changed(conn, adapter->path, ADAPTER_INTERFACE,
"PairableTimeout");
- g_dbus_pending_property_success(conn, id);
+ g_dbus_pending_property_success(id);
}
void btd_adapter_class_changed(struct btd_adapter *adapter, uint8_t *new_class)
@@ -827,24 +824,21 @@ static void set_name(struct btd_adapter *adapter, const char *name,
int ret;
if (adapter->allow_name_changes == FALSE)
- return g_dbus_pending_property_error(btd_get_dbus_connection(),
- id, ERROR_INTERFACE ".Failed",
+ return g_dbus_pending_property_error(id,
+ ERROR_INTERFACE ".Failed",
strerror(EPERM));
ret = adapter_set_name(adapter, name);
- if (ret >= 0) {
- g_dbus_pending_property_success(btd_get_dbus_connection(), id);
- return;
- }
+ if (ret >= 0)
+ return g_dbus_pending_property_success(id);
if (ret == -EINVAL)
- g_dbus_pending_property_error(btd_get_dbus_connection(),
- id, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
+ g_dbus_pending_property_error(id,
+ ERROR_INTERFACE ".InvalidArguments",
+ "Invalid arguments in method call");
else
- g_dbus_pending_property_error(btd_get_dbus_connection(),
- id, ERROR_INTERFACE ".Failed",
- strerror(-ret));
+ g_dbus_pending_property_error(id, ERROR_INTERFACE ".Failed",
+ strerror(-ret));
}
struct btd_device *adapter_find_device(struct btd_adapter *adapter,
@@ -1173,12 +1167,10 @@ static void adapter_property_set_name(const GDBusPropertyTable *property,
{
const char *name;
- if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_STRING) {
- g_dbus_pending_property_error(btd_get_dbus_connection(),
- id, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
- return;
- }
+ if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_STRING)
+ return g_dbus_pending_property_error(id,
+ ERROR_INTERFACE ".InvalidArguments",
+ "Invalid arguments in method call");
dbus_message_iter_get_basic(value, &name);
@@ -1217,9 +1209,9 @@ static void adapter_property_set_powered(
dbus_bool_t powered;
if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN)
- return g_dbus_pending_property_error(btd_get_dbus_connection(),
- id, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
+ return g_dbus_pending_property_error(id,
+ ERROR_INTERFACE ".InvalidArguments",
+ "Invalid arguments in method call");
dbus_message_iter_get_basic(value, &powered);
@@ -1247,9 +1239,9 @@ static void adapter_property_set_discoverable(
dbus_bool_t discoverable;
if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN)
- return g_dbus_pending_property_error(btd_get_dbus_connection(),
- id, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
+ return g_dbus_pending_property_error(id,
+ ERROR_INTERFACE ".InvalidArguments",
+ "Invalid arguments in method call");
dbus_message_iter_get_basic(value, &discoverable);
@@ -1274,9 +1266,9 @@ static void adapter_property_set_pairable(const GDBusPropertyTable *property,
dbus_bool_t pairable;
if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN)
- return g_dbus_pending_property_error(btd_get_dbus_connection(),
- id, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
+ return g_dbus_pending_property_error(id,
+ ERROR_INTERFACE ".InvalidArguments",
+ "Invalid arguments in method call");
dbus_message_iter_get_basic(value, &pairable);
@@ -1303,9 +1295,9 @@ static void adapter_property_set_discoverable_timeout(
uint32_t timeout;
if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_UINT32)
- return g_dbus_pending_property_error(btd_get_dbus_connection(),
- id, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
+ return g_dbus_pending_property_error(id,
+ ERROR_INTERFACE ".InvalidArguments",
+ "Invalid arguments in method call");
dbus_message_iter_get_basic(value, &timeout);
set_discoverable_timeout(data, timeout, id);
@@ -1330,9 +1322,9 @@ static void adapter_property_set_pairable_timeout(
uint32_t timeout;
if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_UINT32)
- return g_dbus_pending_property_error(btd_get_dbus_connection(),
- id, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
+ return g_dbus_pending_property_error(id,
+ ERROR_INTERFACE ".InvalidArguments",
+ "Invalid arguments in method call");
dbus_message_iter_get_basic(value, &timeout);
set_pairable_timeout(data, timeout, id);
@@ -2361,13 +2353,12 @@ static void set_mode_complete(struct btd_adapter *adapter)
if (pending->type == SESSION_TYPE_MODE_GLOBAL) {
if (err < 0)
- g_dbus_pending_property_error(conn, pending->prop_id,
+ g_dbus_pending_property_error(pending->prop_id,
ERROR_INTERFACE ".Failed",
strerror(-err));
else {
adapter->global_mode = adapter->mode;
- g_dbus_pending_property_success(conn,
- pending->prop_id);
+ g_dbus_pending_property_success(pending->prop_id);
}
} else if (pending->msg != NULL) {
DBusMessage *msg = pending->msg;
diff --git a/src/device.c b/src/device.c
index bb0f890..bc7f8dd 100644
--- a/src/device.c
+++ b/src/device.c
@@ -394,10 +394,8 @@ static void set_alias(GDBusPendingPropertySet id, const char *alias,
/* No change */
if ((device->alias == NULL && g_str_equal(alias, "")) ||
- g_strcmp0(device->alias, alias) == 0) {
- g_dbus_pending_property_success(btd_get_dbus_connection(), id);
- return;
- }
+ g_strcmp0(device->alias, alias) == 0)
+ return g_dbus_pending_property_success(id);
ba2str(adapter_get_address(adapter), srcaddr);
ba2str(&device->bdaddr, dstaddr);
@@ -405,11 +403,9 @@ static void set_alias(GDBusPendingPropertySet id, const char *alias,
/* Remove alias if empty string */
err = write_device_alias(srcaddr, dstaddr, device->bdaddr_type,
g_str_equal(alias, "") ? NULL : alias);
- if (err < 0) {
- g_dbus_pending_property_error(btd_get_dbus_connection(),
- id, ERROR_INTERFACE ".Failed", strerror(-err));
- return;
- }
+ if (err < 0)
+ return g_dbus_pending_property_error(id,
+ ERROR_INTERFACE ".Failed", strerror(-err));
g_free(device->alias);
device->alias = g_str_equal(alias, "") ? NULL : g_strdup(alias);
@@ -417,7 +413,7 @@ static void set_alias(GDBusPendingPropertySet id, const char *alias,
g_dbus_emit_property_changed(btd_get_dbus_connection(),
device->path, DEVICE_INTERFACE, "Alias");
- g_dbus_pending_property_success(btd_get_dbus_connection(), id);
+ g_dbus_pending_property_success(id);
}
static void dev_property_set_alias(const GDBusPropertyTable *property,
@@ -426,12 +422,10 @@ static void dev_property_set_alias(const GDBusPropertyTable *property,
{
const char *alias;
- if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_STRING) {
- g_dbus_pending_property_error(btd_get_dbus_connection(),
- id, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
- return;
- }
+ if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_STRING)
+ return g_dbus_pending_property_error(id,
+ ERROR_INTERFACE ".InvalidArguments",
+ "Invalid arguments in method call");
dbus_message_iter_get_basic(value, &alias);
@@ -686,27 +680,23 @@ static void set_trust(GDBusPendingPropertySet id, gboolean value, void *data)
char srcaddr[18], dstaddr[18];
int err;
- if (device->trusted == value) {
- g_dbus_pending_property_success(btd_get_dbus_connection(), id);
- return;
- }
+ if (device->trusted == value)
+ return g_dbus_pending_property_success(id);
ba2str(adapter_get_address(adapter), srcaddr);
ba2str(&device->bdaddr, dstaddr);
err = write_trust(srcaddr, dstaddr, device->bdaddr_type, value);
- if (err < 0) {
- g_dbus_pending_property_error(btd_get_dbus_connection(),
- id, ERROR_INTERFACE ".Failed", strerror(-err));
- return;
- }
+ if (err < 0)
+ return g_dbus_pending_property_error(id,
+ ERROR_INTERFACE ".Failed", strerror(-err));
device->trusted = value;
g_dbus_emit_property_changed(btd_get_dbus_connection(),
device->path, DEVICE_INTERFACE, "Trusted");
- g_dbus_pending_property_success(btd_get_dbus_connection(), id);
+ g_dbus_pending_property_success(id);
}
static void dev_property_set_trusted(const GDBusPropertyTable *property,
@@ -715,12 +705,10 @@ static void dev_property_set_trusted(const GDBusPropertyTable *property,
{
dbus_bool_t b;
- if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN) {
- g_dbus_pending_property_error(btd_get_dbus_connection(),
- id, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
- return;
- }
+ if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN)
+ return g_dbus_pending_property_error(id,
+ ERROR_INTERFACE ".InvalidArguments",
+ "Invalid arguments in method call");
dbus_message_iter_get_basic(value, &b);
@@ -750,17 +738,15 @@ static void set_blocked(GDBusPendingPropertySet id, gboolean value, void *data)
switch (-err) {
case 0:
- g_dbus_pending_property_success(btd_get_dbus_connection(), id);
+ g_dbus_pending_property_success(id);
break;
case EINVAL:
- g_dbus_pending_property_error(btd_get_dbus_connection(), id,
- ERROR_INTERFACE ".Failed",
+ g_dbus_pending_property_error(id, ERROR_INTERFACE ".Failed",
"Kernel lacks blacklist support");
break;
default:
- g_dbus_pending_property_error(btd_get_dbus_connection(), id,
- ERROR_INTERFACE ".Failed",
- strerror(-err));
+ g_dbus_pending_property_error(id, ERROR_INTERFACE ".Failed",
+ strerror(-err));
break;
}
}
@@ -772,12 +758,10 @@ static void dev_property_set_blocked(const GDBusPropertyTable *property,
{
dbus_bool_t b;
- if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN) {
- g_dbus_pending_property_error(btd_get_dbus_connection(),
- id, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
- return;
- }
+ if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BOOLEAN)
+ return g_dbus_pending_property_error(id,
+ ERROR_INTERFACE ".InvalidArguments",
+ "Invalid arguments in method call");
dbus_message_iter_get_basic(value, &b);
--
1.7.12.3
^ permalink raw reply related
* [PATCH BlueZ 1/2] gdbus: Remove connection from pending_property functions
From: Lucas De Marchi @ 2012-10-17 15:14 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lucas De Marchi
The reply to a DBus.Properties.Set() method call should go through the
same D-Bus connection. Thus remove the DBusConnection parameter from the
following functions:
- g_dbus_pending_property_success()
- g_dbus_pending_property_error_valist()
- g_dbus_pending_property_error()
---
gdbus/gdbus.h | 13 +++++--------
gdbus/object.c | 24 ++++++++++++------------
2 files changed, 17 insertions(+), 20 deletions(-)
diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index 8251947..ba49621 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -246,14 +246,11 @@ guint g_dbus_add_signal_watch(DBusConnection *connection,
gboolean g_dbus_remove_watch(DBusConnection *connection, guint tag);
void g_dbus_remove_all_watches(DBusConnection *connection);
-void g_dbus_pending_property_success(DBusConnection *connection,
- GDBusPendingPropertySet id);
-void g_dbus_pending_property_error_valist(DBusConnection *connection,
- GDBusPendingReply id, const char *name,
- const char *format, va_list args);
-void g_dbus_pending_property_error(DBusConnection *connection,
- GDBusPendingReply id, const char *name,
- const char *format, ...);
+void g_dbus_pending_property_success(GDBusPendingPropertySet id);
+void g_dbus_pending_property_error_valist(GDBusPendingReply id,
+ const char *name, const char *format, va_list args);
+void g_dbus_pending_property_error(GDBusPendingReply id, const char *name,
+ const char *format, ...);
void g_dbus_emit_property_changed(DBusConnection *connection,
const char *path, const char *interface,
const char *name);
diff --git a/gdbus/object.c b/gdbus/object.c
index 4147361..ec98c2c 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -71,6 +71,7 @@ struct security_data {
};
struct property_data {
+ DBusConnection *conn;
GDBusPendingPropertySet id;
DBusMessage *message;
};
@@ -445,8 +446,7 @@ static struct property_data *remove_pending_property_data(
return propdata;
}
-void g_dbus_pending_property_success(DBusConnection *connection,
- GDBusPendingPropertySet id)
+void g_dbus_pending_property_success(GDBusPendingPropertySet id)
{
struct property_data *propdata;
@@ -454,14 +454,15 @@ void g_dbus_pending_property_success(DBusConnection *connection,
if (propdata == NULL)
return;
- g_dbus_send_reply(connection, propdata->message, DBUS_TYPE_INVALID);
+ g_dbus_send_reply(propdata->conn, propdata->message,
+ DBUS_TYPE_INVALID);
dbus_message_unref(propdata->message);
g_free(propdata);
}
-void g_dbus_pending_property_error_valist(DBusConnection *connection,
- GDBusPendingReply id, const char *name,
- const char *format, va_list args)
+void g_dbus_pending_property_error_valist(GDBusPendingReply id,
+ const char *name, const char *format,
+ va_list args)
{
struct property_data *propdata;
DBusMessage *reply;
@@ -473,7 +474,7 @@ void g_dbus_pending_property_error_valist(DBusConnection *connection,
reply = g_dbus_create_error_valist(propdata->message, name, format,
args);
if (reply != NULL) {
- dbus_connection_send(connection, reply, NULL);
+ dbus_connection_send(propdata->conn, reply, NULL);
dbus_message_unref(reply);
}
@@ -481,16 +482,14 @@ void g_dbus_pending_property_error_valist(DBusConnection *connection,
g_free(propdata);
}
-void g_dbus_pending_property_error(DBusConnection *connection,
- GDBusPendingReply id, const char *name,
- const char *format, ...)
+void g_dbus_pending_property_error(GDBusPendingReply id, const char *name,
+ const char *format, ...)
{
va_list args;
va_start(args, format);
- g_dbus_pending_property_error_valist(connection, id, name, format,
- args);
+ g_dbus_pending_property_error_valist(id, name, format, args);
va_end(args);
}
@@ -891,6 +890,7 @@ static DBusMessage *properties_set(DBusConnection *connection,
propdata = g_new(struct property_data, 1);
propdata->id = next_pending_property++;
propdata->message = dbus_message_ref(message);
+ propdata->conn = connection;
pending_property_set = g_slist_prepend(pending_property_set, propdata);
property->set(property, &sub, propdata->id, iface->user_data);
--
1.7.12.3
^ permalink raw reply related
* Re: REMINDER: Linux wireless networking summit, Barcelona 8-9 November
From: John W. Linville @ 2012-10-17 14:46 UTC (permalink / raw)
To: Daniel Wagner; +Cc: linux-wireless, linux-bluetooth
In-Reply-To: <507EB186.40506@monom.org>
On Wed, Oct 17, 2012 at 03:24:22PM +0200, Daniel Wagner wrote:
> Hi John,
>
> On 10/15/2012 09:36 PM, John W. Linville wrote:
> >Just a reminder...there will be a Linux wireless networking mini-summit
> >event coming soon. It will be on 8-9 November in Barcelona, just
> >after LinuxCon Europe. If you haven't already made plans, you had
> >better get moving!
> >
> >Attendees of the wireless mini-summit must also register for LinuxCon
> >Europe. More information is available here:
> >
> > http://wireless.kernel.org/en/developers/Summits/Barcelona-2012
> >
> >Please notice that there is a wiki there with schedules for discussion
> >topics. If you have suggestions or want other changes, please make
> >them and/or let us know here.
>
> I changed the "a short overview what we do in ConnMan" to "ConnMan
> and wpa_supplicant" which is probably a more attractive title.
> Unfortunately, Patrik Flykt wont be there.
Awesome, thanks!
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: Memory leak introduced in commit f8619bef3406a2134082dc41c208105fe028c09f
From: Anderson Lizardo @ 2012-10-17 14:08 UTC (permalink / raw)
To: Ludek Finstrle; +Cc: linux-bluetooth
In-Reply-To: <20121017124931.GA32194@pzkagis.cz>
Hi Ludek,
On Wed, Oct 17, 2012 at 9:49 AM, Ludek Finstrle <luf@pzkagis.cz> wrote:
> Hello,
>
> I see memory leak which was introduced in commit f8619bef3406a2134082dc41c208105fe028c09f:
> attrib: Fix not checking if att_data_list_alloc fails
>
> It returns (in src/attrib-server.c) from functions when adl local variable is NULL but
> it doesn't free local variables (read_by_group: group; read_by_type: type; find_info: info).
>
> The patch is very easy but I can't test it with head so I don't want to send possible
> crapy patch.
Please feel free to send it marked as RFC on the subject (e.g. "[RFC]
bla bla") and rebased against master, so we can review the fix.
Best Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: Memory leak introduced in commit f8619bef3406a2134082dc41c208105fe028c09f
From: Vinicius Costa Gomes @ 2012-10-17 14:03 UTC (permalink / raw)
To: Ludek Finstrle; +Cc: linux-bluetooth
In-Reply-To: <20121017124931.GA32194@pzkagis.cz>
Hi Luf,
On 14:49 Wed 17 Oct, Ludek Finstrle wrote:
> Hello,
>
> I see memory leak which was introduced in commit f8619bef3406a2134082dc41c208105fe028c09f:
> attrib: Fix not checking if att_data_list_alloc fails
>
> It returns (in src/attrib-server.c) from functions when adl local variable is NULL but
> it doesn't free local variables (read_by_group: group; read_by_type: type; find_info: info).
Thanks a lot for the report.
Ugh, I will fix it.
>
> The patch is very easy but I can't test it with head so I don't want to send possible
> crapy patch.
>
> Luf
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers,
--
Vinicius
^ permalink raw reply
* Re: REMINDER: Linux wireless networking summit, Barcelona 8-9 November
From: Daniel Wagner @ 2012-10-17 13:24 UTC (permalink / raw)
To: John W. Linville; +Cc: linux-wireless, linux-bluetooth
In-Reply-To: <20121015193648.GF16817@tuxdriver.com>
Hi John,
On 10/15/2012 09:36 PM, John W. Linville wrote:
> Just a reminder...there will be a Linux wireless networking mini-summit
> event coming soon. It will be on 8-9 November in Barcelona, just
> after LinuxCon Europe. If you haven't already made plans, you had
> better get moving!
>
> Attendees of the wireless mini-summit must also register for LinuxCon
> Europe. More information is available here:
>
> http://wireless.kernel.org/en/developers/Summits/Barcelona-2012
>
> Please notice that there is a wiki there with schedules for discussion
> topics. If you have suggestions or want other changes, please make
> them and/or let us know here.
I changed the "a short overview what we do in ConnMan" to "ConnMan and
wpa_supplicant" which is probably a more attractive title.
Unfortunately, Patrik Flykt wont be there.
cheers,
daniel
^ permalink raw reply
* Re: gatttool and multiple devices connected
From: Anderson Lizardo @ 2012-10-17 13:02 UTC (permalink / raw)
To: Anderson Lizardo, Kim Schulz, linux-bluetooth
In-Reply-To: <20121017113515.GA16879@x220.ger.corp.intel.com>
Hi Johan,
On Wed, Oct 17, 2012 at 8:35 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> This kernel is actually not that old (as long as it is based on the
>> official 3.4.0 one). Did you have hcidump logs for the attempt to
>> discover primary services on device B while connected to device A?
>
> Actually I'd say that it is too old for LE usage. E.g. the address type
> field to the L2CAP socket address was introduced in 3.5 which is quite
> important from an LE standpoint.
Ok, my mistake. I thought it was introduced still on 3.4.0. In this
case, Better use the latest mainline (3.6.*).
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Memory leak introduced in commit f8619bef3406a2134082dc41c208105fe028c09f
From: Ludek Finstrle @ 2012-10-17 12:49 UTC (permalink / raw)
To: linux-bluetooth
Hello,
I see memory leak which was introduced in commit f8619bef3406a2134082dc41c208105fe028c09f:
attrib: Fix not checking if att_data_list_alloc fails
It returns (in src/attrib-server.c) from functions when adl local variable is NULL but
it doesn't free local variables (read_by_group: group; read_by_type: type; find_info: info).
The patch is very easy but I can't test it with head so I don't want to send possible
crapy patch.
Luf
^ permalink raw reply
* [PATCH BlueZ] AVDTP: Do not keep a internal reference
From: Luiz Augusto von Dentz @ 2012-10-17 11:49 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Don't initialize reference with 1, instead always start disconnect timer
when reference drops to 0, so in case nobody reclaims the session it
automatically disconnect after 1 second and frees the memory.
---
audio/avdtp.c | 220 ++++++++++++++++++++++++++--------------------------------
1 file changed, 97 insertions(+), 123 deletions(-)
diff --git a/audio/avdtp.c b/audio/avdtp.c
index bd91cb6..6bfeb9b 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -387,8 +387,7 @@ struct avdtp_stream {
/* Structure describing an AVDTP connection between two devices */
struct avdtp {
- int ref;
- int free_lock;
+ unsigned int ref;
uint16_t version;
@@ -657,50 +656,6 @@ static gboolean stream_open_timeout(gpointer user_data)
return FALSE;
}
-static gboolean disconnect_timeout(gpointer user_data)
-{
- struct avdtp *session = user_data;
- struct audio_device *dev;
- gboolean stream_setup;
-
- session->dc_timer = 0;
- stream_setup = session->stream_setup;
- session->stream_setup = FALSE;
-
- dev = manager_get_device(&session->server->src, &session->dst, FALSE);
-
- if (dev && dev->sink && stream_setup)
- sink_setup_stream(dev->sink, session);
- else if (dev && dev->source && stream_setup)
- source_setup_stream(dev->source, session);
- else
- connection_lost(session, ETIMEDOUT);
-
- return FALSE;
-}
-
-static void remove_disconnect_timer(struct avdtp *session)
-{
- g_source_remove(session->dc_timer);
- session->dc_timer = 0;
- session->stream_setup = FALSE;
-}
-
-static void set_disconnect_timer(struct avdtp *session)
-{
- if (session->dc_timer)
- remove_disconnect_timer(session);
-
- if (session->device_disconnect) {
- session->dc_timer = g_idle_add(disconnect_timeout, session);
- return;
- }
-
- session->dc_timer = g_timeout_add_seconds(DISCONNECT_TIMEOUT,
- disconnect_timeout,
- session);
-}
-
void avdtp_error_init(struct avdtp_error *err, uint8_t category, int id)
{
err->category = category;
@@ -780,8 +735,9 @@ static void avdtp_set_state(struct avdtp *session,
}
}
-static void stream_free(struct avdtp_stream *stream)
+static void stream_free(void *data)
{
+ struct avdtp_stream *stream = data;
struct avdtp_remote_sep *rsep;
stream->lsep->info.inuse = 0;
@@ -1144,37 +1100,42 @@ static int avdtp_cancel_authorization(struct avdtp *session)
return err;
session->auth_id = 0;
+ avdtp_unref(session);
return 0;
}
-static void connection_lost(struct avdtp *session, int err)
+static void sep_free(gpointer data)
{
- char address[18];
+ struct avdtp_remote_sep *sep = data;
- ba2str(&session->dst, address);
- DBG("Disconnected from %s", address);
+ g_slist_free_full(sep->caps, g_free);
+ g_free(sep);
+}
- if (err != EACCES)
- avdtp_cancel_authorization(session);
+static void remove_disconnect_timer(struct avdtp *session)
+{
+ g_source_remove(session->dc_timer);
+ session->dc_timer = 0;
+ session->stream_setup = FALSE;
+}
- session->free_lock = 1;
+static void avdtp_free(void *data)
+{
+ struct avdtp *session = data;
- finalize_discovery(session, err);
+ DBG("%p", session);
- g_slist_foreach(session->streams, (GFunc) release_stream, session);
- session->streams = NULL;
+ g_slist_free_full(session->streams, stream_free);
- session->free_lock = 0;
+ if (session->state != AVDTP_SESSION_STATE_DISCONNECTED)
+ avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
if (session->io) {
g_io_channel_shutdown(session->io, FALSE, NULL);
g_io_channel_unref(session->io);
- session->io = NULL;
}
- avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
-
if (session->io_id) {
g_source_remove(session->io_id);
session->io_id = 0;
@@ -1183,69 +1144,95 @@ static void connection_lost(struct avdtp *session, int err)
if (session->dc_timer)
remove_disconnect_timer(session);
- if (session->ref != 1)
- error("connection_lost: ref count not 1 after all callbacks");
- else
- avdtp_unref(session);
+ avdtp_cancel_authorization(session);
+
+ if (session->req)
+ pending_req_free(session->req);
+
+ g_slist_free_full(session->seps, sep_free);
+
+ g_free(session->buf);
+
+ g_free(session);
}
-static void sep_free(gpointer data)
+static gboolean disconnect_timeout(gpointer user_data)
{
- struct avdtp_remote_sep *sep = data;
+ struct avdtp *session = user_data;
+ struct audio_device *dev;
+ gboolean stream_setup;
- g_slist_free_full(sep->caps, g_free);
- g_free(sep);
+ session->dc_timer = 0;
+
+ if (session->ref == 0) {
+ struct avdtp_server *server = session->server;
+
+ server->sessions = g_slist_remove(server->sessions, session);
+ avdtp_free(session);
+ return FALSE;
+ }
+
+ stream_setup = session->stream_setup;
+ session->stream_setup = FALSE;
+
+ dev = manager_get_device(&session->server->src, &session->dst, FALSE);
+
+ if (dev && dev->sink && stream_setup)
+ sink_setup_stream(dev->sink, session);
+ else if (dev && dev->source && stream_setup)
+ source_setup_stream(dev->source, session);
+ else
+ connection_lost(session, ETIMEDOUT);
+
+ return FALSE;
}
-void avdtp_unref(struct avdtp *session)
+static void set_disconnect_timer(struct avdtp *session)
{
- struct avdtp_server *server;
+ if (session->dc_timer)
+ remove_disconnect_timer(session);
- if (!session)
+ if (session->device_disconnect) {
+ session->dc_timer = g_idle_add(disconnect_timeout, session);
return;
+ }
- session->ref--;
-
- DBG("%p: ref=%d", session, session->ref);
+ session->dc_timer = g_timeout_add_seconds(DISCONNECT_TIMEOUT,
+ disconnect_timeout,
+ session);
+}
- if (session->ref == 1) {
- if (session->state == AVDTP_SESSION_STATE_CONNECTING &&
- session->io) {
- avdtp_cancel_authorization(session);
- g_io_channel_shutdown(session->io, TRUE, NULL);
- g_io_channel_unref(session->io);
- session->io = NULL;
- avdtp_set_state(session,
- AVDTP_SESSION_STATE_DISCONNECTED);
- }
+static void connection_lost(struct avdtp *session, int err)
+{
+ char address[18];
- if (session->io)
- set_disconnect_timer(session);
- else if (!session->free_lock) /* Drop the local ref if we
- aren't connected */
- session->ref--;
- }
+ ba2str(&session->dst, address);
+ DBG("Disconnected from %s", address);
- if (session->ref > 0)
- return;
+ if (err != EACCES)
+ avdtp_cancel_authorization(session);
- server = session->server;
+ g_slist_foreach(session->streams, (GFunc) release_stream, session);
+ session->streams = NULL;
- DBG("%p: freeing session and removing from list", session);
+ finalize_discovery(session, err);
- if (session->dc_timer)
- remove_disconnect_timer(session);
+ avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
+}
- server->sessions = g_slist_remove(server->sessions, session);
+void avdtp_unref(struct avdtp *session)
+{
+ if (!session)
+ return;
- if (session->req)
- pending_req_free(session->req);
+ session->ref--;
- g_slist_free_full(session->seps, sep_free);
+ DBG("%p: ref=%d", session, session->ref);
- g_free(session->buf);
+ if (session->ref > 0)
+ return;
- g_free(session);
+ set_disconnect_timer(session);
}
struct avdtp *avdtp_ref(struct avdtp *session)
@@ -2231,12 +2218,6 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
goto failed;
}
- if (session->ref == 1 && !session->streams && !session->req)
- set_disconnect_timer(session);
-
- if (session->streams && session->dc_timer)
- remove_disconnect_timer(session);
-
if (session->req && session->req->collided) {
DBG("Collision detected");
goto next;
@@ -2383,7 +2364,7 @@ static struct avdtp *avdtp_get_internal(const bdaddr_t *src, const bdaddr_t *dst
session->server = server;
bacpy(&session->dst, dst);
- session->ref = 1;
+ set_disconnect_timer(session);
/* We don't use avdtp_set_state() here since this isn't a state change
* but just setting of the initial state */
session->state = AVDTP_SESSION_STATE_DISCONNECTED;
@@ -2490,6 +2471,8 @@ static void auth_cb(DBusError *derr, void *user_data)
struct avdtp *session = user_data;
GError *err = NULL;
+ avdtp_unref(session);
+
if (derr && dbus_error_is_set(derr)) {
error("Access denied: %s", derr->message);
connection_lost(session, EACCES);
@@ -2578,10 +2561,12 @@ static void avdtp_confirm_cb(GIOChannel *chan, gpointer data)
auth_cb, session);
if (session->auth_id == 0) {
avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
- avdtp_unref(session);
goto drop;
}
+ /* Disable disconnect timer while authorizing */
+ avdtp_ref(session);
+
dev->auto_connect = auto_connect;
return;
@@ -3949,23 +3934,12 @@ proceed:
void avdtp_exit(const bdaddr_t *src)
{
struct avdtp_server *server;
- GSList *l;
server = find_server(servers, src);
if (!server)
return;
- l = server->sessions;
- while (l) {
- struct avdtp *session = l->data;
-
- l = l->next;
- /* value of l pointer should be updated before invoking
- * connection_lost since it internally uses avdtp_unref
- * which operates on server->session list as well
- */
- connection_lost(session, -ECONNABORTED);
- }
+ g_slist_free_full(server->sessions, avdtp_free);
servers = g_slist_remove(servers, server);
--
1.7.11.4
^ permalink raw reply related
* Re: gatttool and multiple devices connected
From: Johan Hedberg @ 2012-10-17 11:35 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: Kim Schulz, linux-bluetooth
In-Reply-To: <CAJdJm_P+udqJta3K-FhzYEO4bOm+W=u9koV953etN8TmO=eo4w@mail.gmail.com>
Hi Lizardo,
On Wed, Oct 17, 2012, Anderson Lizardo wrote:
> On Wed, Oct 17, 2012 at 6:00 AM, Kim Schulz <kim@schulz.dk> wrote:
> > Turns out that the guy compiling our kernel choose a version
> > 3.4.0.030400 so not really the
> > bleeding edge version.
> > Do you have any recommendation on what version of the kernel we should
> > aim for?
>
> This kernel is actually not that old (as long as it is based on the
> official 3.4.0 one). Did you have hcidump logs for the attempt to
> discover primary services on device B while connected to device A?
Actually I'd say that it is too old for LE usage. E.g. the address type
field to the L2CAP socket address was introduced in 3.5 which is quite
important from an LE standpoint.
Johan
^ permalink raw reply
* Re: gatttool and multiple devices connected
From: Anderson Lizardo @ 2012-10-17 11:21 UTC (permalink / raw)
To: Kim Schulz; +Cc: linux-bluetooth
In-Reply-To: <005601cdac45$e1ee8ac0$a5cba040$@dk>
Hi Kim,
On Wed, Oct 17, 2012 at 6:00 AM, Kim Schulz <kim@schulz.dk> wrote:
> Turns out that the guy compiling our kernel choose a version
> 3.4.0.030400 so not really the
> bleeding edge version.
> Do you have any recommendation on what version of the kernel we should
> aim for?
This kernel is actually not that old (as long as it is based on the
official 3.4.0 one). Did you have hcidump logs for the attempt to
discover primary services on device B while connected to device A?
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH] device: Fix modifying list while iterating
From: Johan Hedberg @ 2012-10-17 10:42 UTC (permalink / raw)
To: Mikel Astiz; +Cc: linux-bluetooth, Mikel Astiz
In-Reply-To: <1350468429-30882-1-git-send-email-mikel.astiz.oss@gmail.com>
Hi Mikel,
On Wed, Oct 17, 2012, Mikel Astiz wrote:
> A list should not be modified while iterating on it, and in this case
> the solution is trivial: the code is just trying to free the whole list
> with a previous call to profile->device_remove() per list item.
> ---
> src/device.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] avdtp: Fix crash on connection lost
From: Luiz Augusto von Dentz @ 2012-10-17 10:32 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <5925869.iVRXt7vIOW@uw000953>
Hi Szymon,
On Wed, Oct 17, 2012 at 12:17 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> On Wednesday 17 of October 2012 11:48:36 Luiz Augusto von Dentz wrote:
>> Hi Szymon,
>
> Hi Luiz,
>
>> >> We should probably get rid of the free_lock as well.
>> >
>> > Well, this is what this patch does, doesn't it?:)
>>
>> What about the attached patch, it should fix the references for good
>> by using the dc_timer when the reference drops to 0, I just need to
>> check if 1 second is enough to cover all cases including the ones
>> where the remote device connects but doesn't setup any stream, with
>> the devices I have 1 second seems enough.
>
> Yeap, this fix crash I've observed.
>
> However, avdtp_get_internal() now returns newly created session with ref=0
> and in avdtp_confirm_cb() there is unref called when btd_request_authorization
> failed. Due to ref being signed int (why?) this could lead to not-nice-to-debug
> bug if that session is reclaimed before dc timer fires (ref would go from -1 to
> 0).
Good catch, I should be able to fix this, perhaps we can free
immediately or let the timeout handle this as well.
> Also I'm having trouble with tracking lifetime of session object.. I think
> only avdtp_ref/unref function should really care about ref count. That would
> require other function to only use ref/unref to get/free session object.
> e.g. avdtp_free should be only called from disconnect_timeout() and
> connection_lost() should only trigger other session users to drop their
> references. Same goes to set_disconnect_timer() which should only be called
> from avdtp_unref. There should be no need to call connection_lost from
> disconnect_timeout as this timer should only run when there is no references
> held to that session, right?
Well that involves changing some other logic behind the timeout, such as this:
if (session->ref == 0 && !session->streams && !session->req)
set_disconnect_timer(session);
if (session->streams && session->dc_timer)
remove_disconnect_timer(session);
IMO those should be removed in favor of the ref/unref handling if the
disconnect timer should be started/removed, if there is a request the
user should be holding a reference and the same for the stream, this
should actually simplify even further the code. Also the reason I did
avdtp_free in the connection_lost was not to much too much code around
as avdtp_free call a lot of other functions, but you are probably
right and this should actually be called from disconnect_timeout.
--
Luiz Augusto von Dentz
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox