Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH] Fix unnecessary checking for empty phones list
From: Radoslaw Jablonski @ 2010-08-13  6:35 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Radoslaw Jablonski

Removed not needed anymore checking for empty list of phone numbers.
---
 plugins/vcard.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/plugins/vcard.c b/plugins/vcard.c
index 9beb62a..80f8265 100644
--- a/plugins/vcard.c
+++ b/plugins/vcard.c
@@ -295,11 +295,6 @@ static void vcard_printf_end(GString *vcards)
 void phonebook_add_contact(GString *vcards, struct phonebook_contact *contact,
 					uint64_t filter, uint8_t format)
 {
-	/* There's really nothing to do */
-	if ((contact->numbers == NULL && (contact->fullname == NULL ||
-						contact->fullname[0] == '\0')))
-		return;
-
 	if (format == FORMAT_VCARD30)
 		filter |= (FILTER_VERSION | FILTER_FN | FILTER_N | FILTER_TEL);
 	else if (format == FORMAT_VCARD21)
-- 
1.7.0.4


^ permalink raw reply related

* bluetooth profile source code
From: Viral Mehta @ 2010-08-13  4:34 UTC (permalink / raw)
  To: linux-bluetooth@vger.kernel.org

Hi,

I am looking for some BT profile source code.
I was able to find out for OBEX source code,

But, I could not find any source cod for
Headset Profile
A2DP profile
HandsFree Profile

Surprisingly, is there no open source code for all these profiles ?
Or I am just not able to google properly !!??

Thanks,
Viral

This Email may contain confidential or privileged information for the intended recipient (s) If you are not the intended recipient, please do not use or disseminate the information, notify the sender and delete it from your system.

______________________________________________________________________

^ permalink raw reply

* Re: why is hciops a plugin ?
From: Pavan Savoy @ 2010-08-13  0:35 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Johan Hedberg, linux-bluetooth@vger.kernel.org
In-Reply-To: <1281613392.12579.267.camel@localhost.localdomain>

Marcel, Johan,

On Thu, Aug 12, 2010 at 5:13 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Johan,
>
>> > When you say raw HCI access do you mean doing the
>> > socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI) and bind/ioctl, writev+poll/read
>> > lacks few things ?
>> > but all of these are in lib/hci isn't it ?
>>
>> Yes, that's the type of socket that I meant. Direct access to the HCI
>> messages would largely go away from the userspace side. Those
>> libbluetooth functions you're referring to are just convenience wrappers
>> for sending HCI commands through raw HCI sockets.
>>
>> > So even if it was netlink only something like hci_open_dev() would
>> > have changed to socket(PF_NETLINK, SOCK_RAW, BT_NETLINK ); or
>> > something right ?
>>
>> I'm not really familiar enough with netlink to comment on this. Marcel
>> (whose idea it originally was) would have to comment.
>>
>> > any pointers out there ? references for such things ?
>> > I am just curious, don;t want anything specific ...
>>
>> Right now, not really. The only stack internal messages there are at the
>> moment are things like HCI_DEV_REG, HCI_DEV_UNREG, HCI_DEV_UP and
>> HCI_DEV_DOWN and they only go one way (kernel->userspace). This category
>> of messages will grow in the future and it'll be possible to send them
>> both ways.

Yes, this explains why hciops is a a plugin now, thanks...

>> Some HCI messages for which it's already clear now that there will be a
>> benefit from a higher level abstraction on the userspace side are things
>> like name resolving and pairing related requests. Also, if I understood
>> correctly from Marcel, removing userspace processing of HCI events will
>> result in a considreable reduction of context switches since there wont
>> anymore be a promiscuous userspace-side socket that needs to
>> handle/filter all HCI data (Marcel, please correct me if this is
>> inaccurate or wrong).
>
> this is correct. We will not require userspace to do HCI event
> processing and filtering anymore. It will be a dedicated side channel.

Side channel ? But there would still be some way to bring out events
onto user-space right ?
As in application which do and ONLY do HCI-VS commands, and are
interested in HCI-VS events...

> Regards
>
> Marcel
>
>
>

^ permalink raw reply

* Re: [RFC 6/7] Bluetooth: Reassemble enhanced L2CAP PDUs using skb fragments.
From: Mat Martineau @ 2010-08-13  0:11 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth, marcel, rshaffer
In-Reply-To: <20100811052433.GA10773@vigoh>


Gustavo -

On Wed, 11 Aug 2010, Gustavo F. Padovan wrote:

> Hi Mat,
>
> * Mat Martineau <mathewm@codeaurora.org> [2010-08-10 12:15:03 -0700]:
>
>> As enhanced L2CAP PDUs arrive, it is not necessary to copy them
>> in to a separate skbuff.  Instead, the skbuffs can be linked
>> together as fragments, only being copied in to a linear buffer
>> when the data is copied to userspace.  This avoids the need
>> to allocate additional buffers for incoming data, and
>> eliminates copying of data payloads during SDU reassembly.
>> This is of greater concern with high-speed AMP links than
>> with BR/EDR.
>>
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>>  include/net/bluetooth/l2cap.h |    1 +
>>  net/bluetooth/l2cap.c         |   66 +++++++++++++++++++++-------------------
>>  2 files changed, 36 insertions(+), 31 deletions(-)
>>
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index 2f3222f..9384e87 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -357,6 +357,7 @@ struct l2cap_pinfo {
>>  	__u16		sdu_len;
>>  	__u16		partial_sdu_len;
>>  	struct sk_buff	*sdu;
>> +	struct sk_buff	*sdu_last_frag;
>>
>>  	__u8		ident;
>>
>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>> index b485c4a..0212035 100644
>> --- a/net/bluetooth/l2cap.c
>> +++ b/net/bluetooth/l2cap.c
>> @@ -290,6 +290,9 @@ static void l2cap_chan_del(struct sock *sk, int err)
>>  		skb_queue_purge(SREJ_QUEUE(sk));
>>  		skb_queue_purge(BUSY_QUEUE(sk));
>>
>> +		if (l2cap_pi(sk)->sdu)
>> +			kfree_skb(l2cap_pi(sk)->sdu);
>> +
>>  		list_for_each_entry_safe(l, tmp, SREJ_LIST(sk), list) {
>>  			list_del(&l->list);
>>  			kfree(l);
>> @@ -3635,6 +3638,27 @@ static int l2cap_add_to_srej_queue(struct sock *sk, struct sk_buff *skb, u8 tx_s
>>  	return 0;
>>  }
>>
>> +static inline void append_skb_frag(struct sk_buff *skb,
>> +			struct sk_buff *new_frag, struct sk_buff **last_frag)
>
> Call this l2cap_append_skb_frag()

Ok.

>
>> +{
>> +	/* skb->len reflects data in skb as well as all fragments
>> +	   skb->data_len reflects only data in fragments
>> +	 */
>> +	BT_DBG("skb %p, new_frag %p, *last_frag %p", skb, new_frag, *last_frag);
>> +
>> +	if (!skb_has_frags(skb))
>> +		skb_shinfo(skb)->frag_list = new_frag;
>> +
>> +	new_frag->next = NULL;
>> +
>> +	(*last_frag)->next = new_frag;
>> +	*last_frag = new_frag;
>> +
>> +	skb->len += new_frag->len;
>> +	skb->data_len += new_frag->len;
>> +	skb->truesize += new_frag->truesize;
>> +}
>> +
>
> Wondering if it is possible to do that append more simple, but I looked
> through the kernel code it's not possible, we have to keep last frag
> pointer.
>
> This change should go to l2cap_streaming_reassembly_sdu() as well, then
> you can get ride of partial_sdu_len in the struct l2cap_pinfo.

I haven't had a chance to test streaming mode, so I was avoiding 
changes to it so far.  I agree that the same technique can be applied 
there.

>
>>  static int l2cap_ertm_reassembly_sdu(struct sock *sk, struct sk_buff *skb, u16 control)
>>  {
>>  	struct l2cap_pinfo *pi = l2cap_pi(sk);
>> @@ -3643,7 +3667,7 @@ static int l2cap_ertm_reassembly_sdu(struct sock *sk, struct sk_buff *skb, u16 c
>>
>>  	switch (control & L2CAP_CTRL_SAR) {
>>  	case L2CAP_SDU_UNSEGMENTED:
>> -		if (pi->conn_state & L2CAP_CONN_SAR_SDU)
>> +		if (pi->sdu)
>
> pi->sdu can do the work work of pi->conn_state & L2CAP_CONN_SAR_SDU, so
> a separated patch for that change sounds better (you can change that for
> the Streaming mode at the same time, it's a similar code, but more simple).
>

Ok.

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

^ permalink raw reply

* Re: [RFC 5/7] Bluetooth: Handle fragmented skbs in bt_sock_stream_recvmsg()
From: Mat Martineau @ 2010-08-13  0:07 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth, marcel, rshaffer
In-Reply-To: <20100811042543.GA9428@vigoh>



On Wed, 11 Aug 2010, Gustavo F. Padovan wrote:

> Hi Mat,
>
> * Mat Martineau <mathewm@codeaurora.org> [2010-08-10 12:15:02 -0700]:
>
>> When reading L2CAP skbuffs, add the ability to copy from
>> fragmented skbuffs generated during ERTM or streaming mode
>> reassembly.  This defers extra copying of L2CAP payloads
>> until the final, unavoidable copy to userspace.
>>
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>>  net/bluetooth/af_bluetooth.c |   30 ++++++++++++++++++++++++++++--
>>  1 files changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
>> index 77a26fe..5e0375b 100644
>> --- a/net/bluetooth/af_bluetooth.c
>> +++ b/net/bluetooth/af_bluetooth.c
>> @@ -342,7 +342,7 @@ int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
>>  		}
>>
>>  		chunk = min_t(unsigned int, skb->len, size);
>> -		if (memcpy_toiovec(msg->msg_iov, skb->data, chunk)) {
>> +		if (skb_copy_datagram_iovec(skb, 0, msg->msg_iov, chunk)) {
>>  			skb_queue_head(&sk->sk_receive_queue, skb);
>>  			if (!copied)
>>  				copied = -EFAULT;
>> @@ -354,7 +354,33 @@ int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
>>  		sock_recv_ts_and_drops(msg, sk, skb);
>>
>>  		if (!(flags & MSG_PEEK)) {
>> -			skb_pull(skb, chunk);
>> +			int skb_len = skb_headlen(skb);
>
> Why are you using the header length here?

skb_headlen() returns the length of the first fragment.


>> +
>> +			if (chunk <= skb_len) {
>> +				__skb_pull(skb, chunk);
>> +			} else {
>> +				struct sk_buff *frag;
>> +
>> +				__skb_pull(skb, skb_len);
>> +				chunk -= skb_len;
>
> Why do we have this __skb_pull() here? I think skb_walk_frags() can
> handle everything.

That first __skb_pull() is necessary to deal with data in the first 
skbuff, and the skb_walk_frags() deals with anything in 
skb_shinfo(skb)->frag_list.  The linked skb list looks roughly like 
this:

skb
  unsigned char *data -> (L2CAP data)
  struct sk_buff *frag_list -> 2nd frag
                                 unsigned char *data -> (more data)
                                 struct sk_buff *next -> 3rd frag
                                                           (etc.)

So the first frag is special - the pointer to the fragment is 
frag_list.  Each linked fragment after that uses the "next" pointer in 
the sk_buff struct.  The skb_walk_frags() macro starts with the 
sk_buff pointed to by frag_list, then follows the "next" links.



>> +
>> +				skb_walk_frags(skb, frag) {
>> +					if (chunk <= frag->len) {
>> +						/* Pulling partial data */
>> +						skb->len -= chunk;
>> +						skb->data_len -= chunk;
>> +						__skb_pull(frag, chunk);
>> +						break;
>
> If we break here what will happen whit the rest of the data to be
> pulled.

The data is left to be pulled later.  The skb is not freed until all 
the fragments are empty, and this is handled by existing code.  I've 
added the next couple of lines of context below to help explain.

>
>> +					} else if (frag->len) {
>> +						/* Pulling all frag data */
>> +						chunk -= frag->len;
>> +						skb->len -= frag->len;
>> +						skb->data_len -= frag->len;
>> +						__skb_pull(frag, frag->len);
>> +					}
>> +				}
>> +			}
>> +
>>  			if (skb->len) {
>>  				skb_queue_head(&sk->sk_receive_queue, skb);
>>  				break;
                         }
                         kfree_skb(skb);

If the skb is not empty, it's pushed back on the head of the receive 
queue.  If it is empty, it's freed.

I based this approach on some code I found in the SCTP protocol.

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum



^ permalink raw reply

* Re: [RFC 3/7] Bluetooth: Add FCS awareness to L2CAP HCI fragmentation.
From: Mat Martineau @ 2010-08-12 23:36 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth, marcel, rshaffer
In-Reply-To: <20100811035654.GA9293@vigoh>



On Wed, 11 Aug 2010, Gustavo F. Padovan wrote:

> Hi Mat,
>
> * Gustavo F. Padovan <gustavo@padovan.org> [2010-08-11 00:35:41 -0300]:
>
>> * Mat Martineau <mathewm@codeaurora.org> [2010-08-10 12:15:00 -0700]:
>>
>>> In order to not limit ERTM and streaming mode PDUs to the HCI MTU
>>> size, L2CAP must be able to split PDUs in to multple HCI fragments.
>>> This is done by allocating space for the FCS in the last fragment.
>>>
>>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>>> ---
>>>  net/bluetooth/l2cap.c |   39 ++++++++++++++++++++++++++++++++++-----
>>>  1 files changed, 34 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>>> index aa69c84..b485c4a 100644
>>> --- a/net/bluetooth/l2cap.c
>>> +++ b/net/bluetooth/l2cap.c
>>> @@ -1664,31 +1664,63 @@ static inline int l2cap_skbuff_fromiovec(struct sock *sk, struct msghdr *msg, in
>>>  {
>>>  	struct l2cap_conn *conn = l2cap_pi(sk)->conn;
>>>  	struct sk_buff **frag;
>>> +	struct sk_buff *final;
>>>  	int err, sent = 0;
>>>
>>> +	BT_DBG("sk %p, msg %p, len %d, count %d, skb %p", sk,
>>> +		msg, (int)len, (int)count, skb);
>>> +
>>>  	if (memcpy_fromiovec(skb_put(skb, count), msg->msg_iov, count))
>>>  		return -EFAULT;
>>>
>>>  	sent += count;
>>>  	len  -= count;
>>> +	final = skb;
>>>
>>>  	/* Continuation fragments (no L2CAP header) */
>>>  	frag = &skb_shinfo(skb)->frag_list;
>>>  	while (len) {
>>> +		int skblen;
>>>  		count = min_t(unsigned int, conn->mtu, len);
>>>
>>> -		*frag = bt_skb_send_alloc(sk, count, msg->msg_flags & MSG_DONTWAIT, &err);
>>> +		/* Add room for the FCS if it fits */
>>> +		if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 &&
>>> +		    len + L2CAP_FCS_SIZE <= conn->mtu)
>>
>> You don't need to check for (len + L2CAP_FCS_SIZE <= conn=mtu) here.
>> Section 5.1 point that:
>>
>> "Unlike the B-Frame length field, the I-frame length field may be greater
>> than the configured MTU because it includes the octet lengths of the
>> Control, L2CAP SDU Length (when present), and frame check sequence
>> fields as well as the Information octets."
>>
>> From that I understand "len <=" and not "len + L2CAP_FCS_SIZE <="
>
> So here you might want to the check if we support FCS, and then add 2 to
> skblen.

That's what the code does, without violating the HCI MTU.

>>> +			skblen = count + L2CAP_FCS_SIZE;
>>> +		else
>>> +			skblen = count;
>>> +
>>> +		*frag = bt_skb_send_alloc(sk, skblen,
>>> +					msg->msg_flags & MSG_DONTWAIT, &err);
>>>  		if (!*frag)
>>>  			return -EFAULT;
>>> -		if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov, count))
>>> +
>>> +		if (memcpy_fromiovec(skb_put(*frag, count),
>>> +				msg->msg_iov, count))
>>>  			return -EFAULT;
>>>
>>>  		sent += count;
>>>  		len  -= count;
>>>
>>> +		final = *frag;
>>> +
>>>  		frag = &(*frag)->next;
>>>  	}
>>>
>>> +	if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16) {
>>> +		if (skb_tailroom(final) < L2CAP_FCS_SIZE) {
>>> +			*frag = bt_skb_send_alloc(sk, L2CAP_FCS_SIZE,
>>> +						msg->msg_flags & MSG_DONTWAIT,
>>> +						&err);
>
> Why do we need to check for FCS again? We already added it required
> space to the last fragment.
>

If there was room in the HCI fragment, count+2 bytes were allocated 
with bt_skb_send_alloc.  However, only count bytes were used by 
skb_put.

This final block of code is doing two things:

  1. Allocating a final HCI fragment for the FCS if there was not room 
for the FCS in the last data fragment.

  2. Doing the skb_put for the FCS bytes.

I will add some comments to this code to help clarify what's going on.

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

^ permalink raw reply

* Re: [RFC 3/7] Bluetooth: Add FCS awareness to L2CAP HCI fragmentation.
From: Mat Martineau @ 2010-08-12 23:26 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth, marcel, rshaffer
In-Reply-To: <20100811033540.GA9151@vigoh>



On Wed, 11 Aug 2010, Gustavo F. Padovan wrote:

> * Mat Martineau <mathewm@codeaurora.org> [2010-08-10 12:15:00 -0700]:
>
>> In order to not limit ERTM and streaming mode PDUs to the HCI MTU
>> size, L2CAP must be able to split PDUs in to multple HCI fragments.
>> This is done by allocating space for the FCS in the last fragment.
>>
>> Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
>> ---
>>  net/bluetooth/l2cap.c |   39 ++++++++++++++++++++++++++++++++++-----
>>  1 files changed, 34 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>> index aa69c84..b485c4a 100644
>> --- a/net/bluetooth/l2cap.c
>> +++ b/net/bluetooth/l2cap.c
>> @@ -1664,31 +1664,63 @@ static inline int l2cap_skbuff_fromiovec(struct sock *sk, struct msghdr *msg, in
>>  {
>>  	struct l2cap_conn *conn = l2cap_pi(sk)->conn;
>>  	struct sk_buff **frag;
>> +	struct sk_buff *final;
>>  	int err, sent = 0;
>>
>> +	BT_DBG("sk %p, msg %p, len %d, count %d, skb %p", sk,
>> +		msg, (int)len, (int)count, skb);
>> +
>>  	if (memcpy_fromiovec(skb_put(skb, count), msg->msg_iov, count))
>>  		return -EFAULT;
>>
>>  	sent += count;
>>  	len  -= count;
>> +	final = skb;
>>
>>  	/* Continuation fragments (no L2CAP header) */
>>  	frag = &skb_shinfo(skb)->frag_list;
>>  	while (len) {
>> +		int skblen;
>>  		count = min_t(unsigned int, conn->mtu, len);
>>
>> -		*frag = bt_skb_send_alloc(sk, count, msg->msg_flags & MSG_DONTWAIT, &err);
>> +		/* Add room for the FCS if it fits */
>> +		if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16 &&
>> +		    len + L2CAP_FCS_SIZE <= conn->mtu)
>
> You don't need to check for (len + L2CAP_FCS_SIZE <= conn=mtu) here.
> Section 5.1 point that:
>
> "Unlike the B-Frame length field, the I-frame length field may be greater
> than the configured MTU because it includes the octet lengths of the
> Control, L2CAP SDU Length (when present), and frame check sequence
> fields as well as the Information octets."
>
> From that I understand "len <=" and not "len + L2CAP_FCS_SIZE <="

This is checking against the HCI MTU, not the L2CAP MTU.  Any L2CAP 
payload (headers, data, or FCS) needs to fit in the HCI fragment. 
The idea here is to see if len is short enough that the FCS can be 
included in this HCI fragment after the L2CAP data.

>> +			skblen = count + L2CAP_FCS_SIZE;
>> +		else
>> +			skblen = count;
>> +
>> +		*frag = bt_skb_send_alloc(sk, skblen,
>> +					msg->msg_flags & MSG_DONTWAIT, &err);
>>  		if (!*frag)
>>  			return -EFAULT;
>> -		if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov, count))
>> +
>> +		if (memcpy_fromiovec(skb_put(*frag, count),
>> +				msg->msg_iov, count))
>>  			return -EFAULT;
>>
>>  		sent += count;
>>  		len  -= count;
>>
>> +		final = *frag;
>> +
>>  		frag = &(*frag)->next;
>>  	}
>>
>> +	if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16) {
>> +		if (skb_tailroom(final) < L2CAP_FCS_SIZE) {
>> +			*frag = bt_skb_send_alloc(sk, L2CAP_FCS_SIZE,
>> +						msg->msg_flags & MSG_DONTWAIT,
>> +						&err);
>> +			if (!*frag)
>> +				return -EFAULT;
>> +
>> +			final = *frag;
>> +		}
>> +
>> +		skb_put(final, L2CAP_FCS_SIZE);
>> +	}
>> +
>>  	return sent;
>>  }
>>
>> @@ -1790,9 +1822,6 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct sock *sk, struct msghdr *m
>>  		return ERR_PTR(err);
>>  	}
>>
>> -	if (l2cap_pi(sk)->fcs == L2CAP_FCS_CRC16)
>> -		put_unaligned_le16(0, skb_put(skb, 2));
>> -
>>  	bt_cb(skb)->retries = 0;
>>  	return skb;
>>  }
>> --
>> 1.7.1

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum



^ permalink raw reply

* Re: [PATCH] Bluetooth: Process HCI events in a workqueue instead of a tasklet
From: David Vrabel @ 2010-08-12 22:20 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1281446085.12579.212.camel@localhost.localdomain>

Marcel Holtmann wrote:
> Hi David,
> 
>>>> Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
>>>> ---
>>>>  include/net/bluetooth/hci_core.h |    2 +
>>>>  net/bluetooth/hci_core.c         |   58 ++++++++++++++++++++++++++++++++-----
>>>>  2 files changed, 52 insertions(+), 8 deletions(-)
>>> so I stuffed this now into bluetooth-testing tree and would like to see
>>> some extra testing exposure. So far this has only been tested by myself.
>>>
>>> If there are no regression then this should make a lot of HCI and L2CAP
>>> handling a lot simple.
>> This may result in packets being processed in a different order to that 
>> which they were received in.
>>
>> e.g., what happens to an ACL packet processed before the connection 
>> complete event for that connection?
> 
> good point. So we would either a) need to disable the RX tasklet when we
> receive an event and schedule it for processing or b) process the ACL
> data also in a workqueue.

I've thought some more about this and I'm not sure disabling the tasklet
is sufficient to prevent packets being reordered.  Consider a transport
that submits (in the same interrupt handler call) an ACL packet and an
HCI event.  The tasklet will be scheduled and then disabled until the
event is processed in the workqueue.

On the other hand, USB transports do not ensure any ordering between HCI
event and ACL packets because they're received on different USB
endpoints which could be processed in any order.

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

^ permalink raw reply

* [PATCH 2nd version] IrMC sync server support
From: Marcel Mol @ 2010-08-12 21:09 UTC (permalink / raw)
  To: linux-bluetooth

A reasonable working IrMC SYNC server (only full phonebook sync support)
Support for cal and note by just returning nothing.
---
 Makefile.am    |    3 +
 plugins/irmc.c |  507 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/main.c     |   10 +-
 src/obex.h     |    3 +-
 4 files changed, 521 insertions(+), 2 deletions(-)
 create mode 100644 plugins/irmc.c

diff --git a/Makefile.am b/Makefile.am
index 73e2f28..a2873f9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -55,6 +55,9 @@ builtin_modules += pbap
 builtin_sources += plugins/pbap.c plugins/phonebook.h \
 			plugins/vcard.h plugins/vcard.c
 
+builtin_modules += irmc
+builtin_sources += plugins/irmc.c plugins/phonebook.h
+
 builtin_modules += syncevolution
 builtin_sources += plugins/syncevolution.c
 
diff --git a/plugins/irmc.c b/plugins/irmc.c
new file mode 100644
index 0000000..fa7f91d
--- /dev/null
+++ b/plugins/irmc.c
@@ -0,0 +1,507 @@
+/*
+ *
+ *  OBEX IrMC Sync Server
+ *
+ *  Copyright (C) 2010  Marcel Mol <marcel@mesa.nl>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include <glib.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+#include <openobex/obex.h>
+#include <openobex/obex_const.h>
+
+#include "plugin.h"
+#include "log.h"
+#include "obex.h"
+#include "service.h"
+#include "phonebook.h"
+#include "mimetype.h"
+#include "filesystem.h"
+#include "dbus.h"
+
+#define IRMC_CHANNEL	17
+
+#define IRMC_RECORD "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>	\
+<record>								\
+  <attribute id=\"0x0001\">						\
+    <sequence>								\
+      <uuid value=\"0x1104\"/>						\
+    </sequence>								\
+  </attribute>								\
+									\
+  <attribute id=\"0x0004\">						\
+    <sequence>								\
+      <sequence>							\
+        <uuid value=\"0x0100\"/>					\
+      </sequence>							\
+      <sequence>							\
+        <uuid value=\"0x0003\"/>					\
+        <uint8 value=\"%u\" name=\"channel\"/>				\
+      </sequence>							\
+      <sequence>							\
+        <uuid value=\"0x0008\"/>					\
+      </sequence>							\
+    </sequence>								\
+  </attribute>								\
+									\
+  <attribute id=\"0x0009\">						\
+    <sequence>								\
+      <sequence>							\
+        <uuid value=\"0x1104\"/>					\
+        <uint16 value=\"0x0100\" name=\"version\"/>			\
+      </sequence>							\
+    </sequence>								\
+  </attribute>								\
+									\
+  <attribute id=\"0x0100\">						\
+    <text value=\"%s\" name=\"name\"/>					\
+  </attribute>								\
+									\
+  <attribute id=\"0x0301\">						\
+    <sequence>								\
+      <uint8 value=\"0x01\"/>						\
+    </sequence>								\
+  </attribute>								\
+</record>"
+
+
+struct aparam_header {
+	uint8_t tag;
+	uint8_t len;
+	uint8_t val[0];
+} __attribute__ ((packed));
+
+#define DID_LEN 18
+
+struct irmc_session {
+	struct obex_session *os;
+	struct apparam_field *params;
+	uint16_t entries;
+	GString *buffer;
+	char sn[DID_LEN];
+	char did[DID_LEN];
+	char manu[DID_LEN];
+	char model[DID_LEN];
+};
+
+#define IRMC_TARGET_SIZE 9
+
+static const guint8 IRMC_TARGET[IRMC_TARGET_SIZE] = {
+			0x49, 0x52, 0x4d, 0x43,  0x2d, 0x53, 0x59, 0x4e, 0x43 };
+
+/*
+ * FIXME:
+ * the IrMC specs state the first vcard should be the owner
+ * vcard. As there is no simple way to collect ownerdetails
+ * just create an empty vcard (which is allowed according to the
+ * specs).
+ */
+static const char *owner_vcard =
+		"BEGIN:VCARD\r\n"
+		"VERSION:2.1\r\n"
+		"N:\r\n"
+		"TEL:\r\n"
+		"X-IRMX-LUID:0\r\n"
+		"END:VCARD\r\n";
+
+static void phonebook_size_result(const char *buffer, size_t bufsize,
+				int vcards, int missed, void *user_data)
+{
+	struct irmc_session *irmc = user_data;
+
+	DBG("vcards %d", vcards);
+
+	irmc->params->maxlistcount = vcards;
+}
+
+static void query_result(const char *buffer, size_t bufsize, int vcards,
+					int missed, void *user_data)
+{
+	struct irmc_session *irmc = user_data;
+	const char *s, *t;
+
+	DBG("bufsize %d vcards %d missed %d", bufsize, vcards, missed);
+
+	/* first add a 'owner' vcard */
+	if (!irmc->buffer)
+		irmc->buffer = g_string_new(owner_vcard);
+	else
+		irmc->buffer = g_string_append(irmc->buffer, owner_vcard);
+
+	/* loop around buffer and add X-IRMC-LUID attribs */
+	s = buffer;
+	while ((t = strstr(s, "UID:")) != NULL) {
+		/* add upto UID: into buffer */
+		irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
+		/*
+		 * add UID: line into buffer
+		 * Not sure if UID is still needed if X-IRMC-LUID is there
+		 */
+		s = t;
+		t = strstr(s, "\r\n");
+		t += 2;
+		irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
+		/* add X-IRMC-LUID with same number as UID */
+		irmc->buffer = g_string_append_len(irmc->buffer,
+							"X-IRMC-LUID:", 12);
+		s += 4; /* point to uid number */
+		irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
+		s = t;
+	}
+	/* add remaining bit of buffer */
+	irmc->buffer = g_string_append(irmc->buffer, s);
+
+	obex_object_set_io_flags(irmc, G_IO_IN, 0);
+}
+
+static void *irmc_connect(struct obex_session *os, int *err)
+{
+	struct irmc_session *irmc;
+	struct apparam_field *param;
+
+	DBG("");
+
+	manager_register_session(os);
+
+	irmc = g_new0(struct irmc_session, 1);
+	irmc->os = os;
+
+	/*
+	 * FIXME:
+	 * Ideally get capabilities info here and use that to define
+	 * IrMC DID and SN etc parameters.
+	 * For now lets used hostname and some 'random' value
+	 */
+	gethostname(irmc->did, DID_LEN);
+	strncpy(irmc->sn, "12345", DID_LEN);
+	strncpy(irmc->manu, "obex", DID_LEN);
+	strncpy(irmc->model, "mymodel", DID_LEN);
+
+	/*
+         *  We need to know the number of contact/cal/nt entries
+         *  somewhere so why not do it now.
+         */
+	param = g_new0(struct apparam_field, 1);
+	param->maxlistcount = 0; /* to count the number of vcards... */
+	param->filter = 0x200085; /* UID TEL N VERSION */
+	irmc->params = param;
+	phonebook_pull("telecom/pb.vcf", irmc->params, phonebook_size_result,
+									irmc);
+
+	if (err)
+		*err = 0;
+
+	return irmc;
+}
+
+static int irmc_get(struct obex_session *os, obex_object_t *obj,
+		gboolean *stream, void *user_data)
+{
+	struct irmc_session *irmc = user_data;
+	const char *type = obex_get_type(os);
+	const char *name = obex_get_name(os);
+	char *path;
+	int ret;
+
+	DBG("name %s type %s irmc %p", name, type ? type : "NA", irmc);
+
+	path = g_strdup(name);
+	*stream = TRUE;
+
+	ret = obex_get_stream_start(os, path);
+
+	g_free(path);
+
+	return ret;
+}
+
+static void irmc_disconnect(struct obex_session *os, void *user_data)
+{
+	struct irmc_session *irmc = user_data;
+
+	DBG("");
+
+	manager_unregister_session(os);
+
+        if (irmc->params) {
+                if (irmc->params->searchval)
+			g_free(irmc->params->searchval);
+                g_free(irmc->params);
+        }
+	if (irmc->buffer) {
+		string_free(irmc->buffer);
+		irmc->buffer = NULL;
+	}
+
+	g_free(irmc);
+}
+
+static int irmc_chkput(struct obex_session *os, void *user_data)
+{
+	DBG("");
+	/* Reject all PUTs */
+	return -EBADR;
+}
+
+static void *irmc_open_devinfo(struct irmc_session *irmc, int *err)
+{
+	if (!irmc->buffer)
+		irmc->buffer = g_string_new("");
+
+	g_string_append_printf(irmc->buffer,
+				"MANU:%s\r\n"
+				"MOD:%s\r\n"
+				"SN:%s\r\n"
+				"PB-TYPE-TX:VCARD2.1\r\n",
+				irmc->manu, irmc->model, irmc->sn);
+
+	return irmc;
+}
+
+static void *irmc_open_pb(const char *name,
+		 struct irmc_session *irmc, int *err)
+{
+	GString *mybuf;
+	int ret;
+
+	if (!g_strcmp0(name, ".vcf")) {
+		/* how can we tell if the vcard count call already finished? */
+		ret = phonebook_pull("telecom/pb.vcf", irmc->params,
+							query_result, irmc);
+		if (ret < 0) {
+			DBG("phonebook_pull failed...");
+			goto fail;
+		}
+		return irmc;
+	}
+
+	if (!g_strcmp0(name, "/info.log")) {
+		mybuf = g_string_new("");
+		g_string_printf(mybuf, "Total-Records:%d\r\n"
+				"Maximum-Records:%d\r\n"
+				"DID:%s\r\n",
+				irmc->params->maxlistcount,
+				irmc->params->maxlistcount, irmc->did);
+	}
+	else if (!strncmp(name, "/luid/", 6)) {
+		name += 6;
+		if (!g_strcmp0(name, "cc.log")) {
+			mybuf = g_string_new("");
+			g_string_printf(mybuf, "%d\r\n", irmc->params->maxlistcount);
+		}
+		else {
+			int l = strlen(name);
+			/* FIXME:
+			 * Reply the same to any *.log so we hopefully force a
+			 * full phonebook dump.
+			 * Is IEL:2 ok?
+			 */
+			if (l > 4 && !g_strcmp0(name + l - 4, ".log")) {
+				DBG("changelog request, force whole book");
+				mybuf = g_string_new("");
+				g_string_printf(mybuf, "SN:%s\r\n"
+							"IEL:2\r\n"
+							"DID:%s\r\n"
+							"Total-Records:%d\r\n"
+							"Maximum-Records:%d\r\n"
+							"*\r\n",
+						irmc->sn, irmc->did,
+						irmc->params->maxlistcount,
+						irmc->params->maxlistcount);
+			}
+			else {
+				ret = -EBADR;
+				goto fail;
+			}
+		}
+	}
+	else {
+		ret = -EBADR;
+		goto fail;
+	}
+
+	if (!irmc->buffer)
+		irmc->buffer = mybuf;
+	else {
+		irmc->buffer = g_string_append(irmc->buffer, mybuf->str);
+		string_free(mybuf);
+	}
+
+	return irmc;
+
+fail:
+	if (err)
+		*err = ret;
+
+	return NULL;
+}
+
+static void *irmc_open_cal(const char *name,
+		 struct irmc_session *irmc, int *err)
+{
+	/* no suport yet. Just return an empty buffer. cal.vcs */
+	DBG("unsupported, returning empty buffer");
+	if (!irmc->buffer)
+		irmc->buffer = g_string_new("");
+
+	return irmc;
+}
+
+static void *irmc_open_nt(const char *name,
+		 struct irmc_session *irmc, int *err)
+{
+	/* no suport yet. Just return an empty buffer. nt.vnt */
+	DBG("unsupported, returning empty buffer");
+	if (!irmc->buffer)
+		irmc->buffer = g_string_new("");
+
+	return irmc;
+}
+
+static void *irmc_open(const char *name, int oflag, mode_t mode,
+		void *context, size_t *size, int *err)
+{
+	struct irmc_session *irmc = context;
+	int ret;
+	const char *p;
+
+	DBG("name %s context %p", name, context);
+	if (oflag != O_RDONLY) {
+		ret = -EPERM;
+		goto fail;
+	}
+	if (strncmp(name, "telecom/", 8) != 0) {
+		ret = -EBADR;
+		goto fail;
+	}
+
+	p = name + 8;
+	if (!g_strcmp0(p, "devinfo.txt"))
+		return irmc_open_devinfo(irmc, err);
+	else if (!strncmp(p, "pb", 2))
+		return irmc_open_pb(p+2, irmc, err);
+	else if (!strncmp(p, "cal", 3))
+		return irmc_open_cal(p+3, irmc, err);
+	else if (!strncmp(p, "nt", 2))
+		return irmc_open_nt(p+2, irmc, err);
+
+fail:
+	if (err)
+		*err = ret;
+
+	return NULL;
+}
+
+static int irmc_close(void *object)
+{
+	struct irmc_session *irmc = object;
+
+	DBG("");
+	if (irmc->buffer) {
+		string_free(irmc->buffer);
+		irmc->buffer = NULL;
+	}
+
+	return 0;
+}
+
+static ssize_t irmc_read(void *object, void *buf, size_t count, uint8_t *hi)
+{
+	struct irmc_session *irmc = object;
+        int len;
+
+	DBG("buffer %p count %d", irmc->buffer, count);
+	if (!irmc->buffer)
+                return -EAGAIN;
+
+	*hi = OBEX_HDR_BODY;
+	len = string_read(irmc->buffer, buf, count);
+	DBG("returning %d bytes", len);
+	return len;
+}
+
+static struct obex_mime_type_driver irmc_driver = {
+	.target = IRMC_TARGET,
+	.target_size = IRMC_TARGET_SIZE,
+	.open = irmc_open,
+	.close = irmc_close,
+	.read = irmc_read,
+};
+
+static struct obex_service_driver irmc = {
+	.name = "IRMC Sync server",
+	.service = OBEX_IRMC,
+	.channel = IRMC_CHANNEL,
+	.record = IRMC_RECORD,
+	.target = IRMC_TARGET,
+	.target_size = IRMC_TARGET_SIZE,
+	.connect = irmc_connect,
+	.get = irmc_get,
+	.disconnect = irmc_disconnect,
+	.chkput = irmc_chkput
+};
+
+static int irmc_init(void)
+{
+	int err;
+
+	DBG("");
+	err = phonebook_init();
+	if (err < 0)
+		goto fail_pb_init;
+
+	err = obex_mime_type_driver_register(&irmc_driver);
+	if (err < 0)
+                goto fail_mime_irmc;
+
+	err = obex_service_driver_register(&irmc);
+	if (err < 0)
+		goto fail_irmc_reg;
+
+	return 0;
+
+fail_irmc_reg:
+	obex_mime_type_driver_unregister(&irmc_driver);
+fail_mime_irmc:
+	phonebook_exit();
+fail_pb_init:
+	return err;
+}
+
+static void irmc_exit(void)
+{
+	DBG("");
+	obex_service_driver_unregister(&irmc);
+	obex_mime_type_driver_unregister(&irmc_driver);
+	phonebook_exit();
+}
+
+OBEX_PLUGIN_DEFINE(irmc, irmc_init, irmc_exit)
diff --git a/src/main.c b/src/main.c
index 649acf9..ef894cf 100644
--- a/src/main.c
+++ b/src/main.c
@@ -80,6 +80,7 @@ static gboolean option_autoaccept = FALSE;
 static gboolean option_opp = FALSE;
 static gboolean option_ftp = FALSE;
 static gboolean option_pbap = FALSE;
+static gboolean option_irmc = FALSE;
 static gboolean option_pcsuite = FALSE;
 static gboolean option_symlinks = FALSE;
 static gboolean option_syncevolution = FALSE;
@@ -120,6 +121,8 @@ static GOptionEntry options[] = {
 				"Enable Phonebook Access server" },
 	{ "pcsuite", 's', 0, G_OPTION_ARG_NONE, &option_pcsuite,
 				"Enable PC Suite Services server" },
+	{ "irmc", 'i', 0, G_OPTION_ARG_NONE, &option_irmc,
+				"Enable IrMCSync server" },
 	{ "syncevolution", 'e', 0, G_OPTION_ARG_NONE, &option_syncevolution,
 				"Enable OBEX server for SyncEvolution" },
 	{ NULL },
@@ -208,9 +211,10 @@ int main(int argc, char *argv[])
 
 	if (option_opp == FALSE && option_ftp == FALSE &&
 				option_pbap == FALSE &&
+				option_irmc == FALSE &&
 				option_syncevolution == FALSE) {
 		fprintf(stderr, "No server selected (use either "
-				"--opp, --ftp, --pbap or --syncevolution)\n");
+				"--opp, --ftp, --pbap, --irmc or --syncevolution)\n");
 		exit(EXIT_FAILURE);
 	}
 
@@ -266,6 +270,10 @@ int main(int argc, char *argv[])
 				option_autoaccept, option_symlinks,
 				option_capability);
 
+	if (option_irmc == TRUE)
+		obex_server_init(OBEX_IRMC, NULL, TRUE, FALSE, FALSE,
+				option_capability);
+
 	if (option_syncevolution == TRUE)
 		obex_server_init(OBEX_SYNCEVOLUTION, NULL, TRUE, FALSE,
 							FALSE, NULL);
diff --git a/src/obex.h b/src/obex.h
index 9424b6b..35723ca 100644
--- a/src/obex.h
+++ b/src/obex.h
@@ -34,7 +34,8 @@
 #define OBEX_BIP	(1 << 3)
 #define OBEX_PBAP	(1 << 4)
 #define OBEX_PCSUITE	(1 << 5)
-#define OBEX_SYNCEVOLUTION	(1 << 6)
+#define OBEX_IRMC	(1 << 6)
+#define OBEX_SYNCEVOLUTION	(1 << 7)
 
 #define TARGET_SIZE 16
 
-- 
1.7.2


^ permalink raw reply related

* [PATCH 3th version] IrMC sync server support
From: Marcel Mol @ 2010-08-12 21:09 UTC (permalink / raw)
  To: linux-bluetooth

A reasonable working IrMC SYNC server (only full phonebook sync support)
Support for cal and note by just returning nothing.
---
 Makefile.am    |    3 +
 plugins/irmc.c |  515 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/main.c     |   10 +-
 src/obex.h     |    3 +-
 4 files changed, 529 insertions(+), 2 deletions(-)
 create mode 100644 plugins/irmc.c

diff --git a/Makefile.am b/Makefile.am
index 73e2f28..07fc27d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -55,6 +55,9 @@ builtin_modules += pbap
 builtin_sources += plugins/pbap.c plugins/phonebook.h \
 			plugins/vcard.h plugins/vcard.c
 
+builtin_modules += irmc
+builtin_sources += plugins/irmc.c
+
 builtin_modules += syncevolution
 builtin_sources += plugins/syncevolution.c
 
diff --git a/plugins/irmc.c b/plugins/irmc.c
new file mode 100644
index 0000000..056a3b9
--- /dev/null
+++ b/plugins/irmc.c
@@ -0,0 +1,515 @@
+/*
+ *
+ *  OBEX IrMC Sync Server
+ *
+ *  Copyright (C) 2010  Marcel Mol <marcel@mesa.nl>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include <glib.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <arpa/inet.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+#include <openobex/obex.h>
+#include <openobex/obex_const.h>
+
+#include "plugin.h"
+#include "log.h"
+#include "obex.h"
+#include "service.h"
+#include "phonebook.h"
+#include "mimetype.h"
+#include "filesystem.h"
+#include "dbus.h"
+
+#define IRMC_CHANNEL	17
+
+#define IRMC_RECORD "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>	\
+<record>								\
+  <attribute id=\"0x0001\">						\
+    <sequence>								\
+      <uuid value=\"0x1104\"/>						\
+    </sequence>								\
+  </attribute>								\
+									\
+  <attribute id=\"0x0004\">						\
+    <sequence>								\
+      <sequence>							\
+        <uuid value=\"0x0100\"/>					\
+      </sequence>							\
+      <sequence>							\
+        <uuid value=\"0x0003\"/>					\
+        <uint8 value=\"%u\" name=\"channel\"/>				\
+      </sequence>							\
+      <sequence>							\
+        <uuid value=\"0x0008\"/>					\
+      </sequence>							\
+    </sequence>								\
+  </attribute>								\
+									\
+  <attribute id=\"0x0009\">						\
+    <sequence>								\
+      <sequence>							\
+        <uuid value=\"0x1104\"/>					\
+        <uint16 value=\"0x0100\" name=\"version\"/>			\
+      </sequence>							\
+    </sequence>								\
+  </attribute>								\
+									\
+  <attribute id=\"0x0100\">						\
+    <text value=\"%s\" name=\"name\"/>					\
+  </attribute>								\
+									\
+  <attribute id=\"0x0301\">						\
+    <sequence>								\
+      <uint8 value=\"0x01\"/>						\
+    </sequence>								\
+  </attribute>								\
+</record>"
+
+
+struct aparam_header {
+	uint8_t tag;
+	uint8_t len;
+	uint8_t val[0];
+} __attribute__ ((packed));
+
+#define DID_LEN 18
+
+struct irmc_session {
+	struct obex_session *os;
+	struct apparam_field *params;
+	uint16_t entries;
+	GString *buffer;
+	char sn[DID_LEN];
+	char did[DID_LEN];
+	char manu[DID_LEN];
+	char model[DID_LEN];
+};
+
+#define IRMC_TARGET_SIZE 9
+
+static const guint8 IRMC_TARGET[IRMC_TARGET_SIZE] = {
+			0x49, 0x52, 0x4d, 0x43,  0x2d, 0x53, 0x59, 0x4e, 0x43 };
+
+/*
+ * FIXME:
+ * the IrMC specs state the first vcard should be the owner
+ * vcard. As there is no simple way to collect ownerdetails
+ * just create an empty vcard (which is allowed according to the
+ * specs).
+ */
+static const char *owner_vcard =
+		"BEGIN:VCARD\r\n"
+		"VERSION:2.1\r\n"
+		"N:\r\n"
+		"TEL:\r\n"
+		"X-IRMX-LUID:0\r\n"
+		"END:VCARD\r\n";
+
+static void phonebook_size_result(const char *buffer, size_t bufsize,
+				int vcards, int missed, void *user_data)
+{
+	struct irmc_session *irmc = user_data;
+
+	DBG("vcards %d", vcards);
+
+	irmc->params->maxlistcount = vcards;
+}
+
+static void query_result(const char *buffer, size_t bufsize, int vcards,
+					int missed, void *user_data)
+{
+	struct irmc_session *irmc = user_data;
+	const char *s, *t;
+
+	DBG("bufsize %d vcards %d missed %d", bufsize, vcards, missed);
+
+	/* first add a 'owner' vcard */
+	if (!irmc->buffer)
+		irmc->buffer = g_string_new(owner_vcard);
+	else
+		irmc->buffer = g_string_append(irmc->buffer, owner_vcard);
+
+	/* loop around buffer and add X-IRMC-LUID attribs */
+	s = buffer;
+	while ((t = strstr(s, "UID:")) != NULL) {
+		/* add upto UID: into buffer */
+		irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
+		/*
+		 * add UID: line into buffer
+		 * Not sure if UID is still needed if X-IRMC-LUID is there
+		 */
+		s = t;
+		t = strstr(s, "\r\n");
+		t += 2;
+		irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
+		/* add X-IRMC-LUID with same number as UID */
+		irmc->buffer = g_string_append_len(irmc->buffer,
+							"X-IRMC-LUID:", 12);
+		s += 4; /* point to uid number */
+		irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
+		s = t;
+	}
+	/* add remaining bit of buffer */
+	irmc->buffer = g_string_append(irmc->buffer, s);
+
+	obex_object_set_io_flags(irmc, G_IO_IN, 0);
+}
+
+static void *irmc_connect(struct obex_session *os, int *err)
+{
+	struct irmc_session *irmc;
+	struct apparam_field *param;
+
+	DBG("");
+
+	manager_register_session(os);
+
+	irmc = g_new0(struct irmc_session, 1);
+	irmc->os = os;
+
+	/*
+	 * FIXME:
+	 * Ideally get capabilities info here and use that to define
+	 * IrMC DID and SN etc parameters.
+	 * For now lets used hostname and some 'random' value
+	 */
+	gethostname(irmc->did, DID_LEN);
+	strncpy(irmc->sn, "12345", DID_LEN);
+	strncpy(irmc->manu, "obex", DID_LEN);
+	strncpy(irmc->model, "mymodel", DID_LEN);
+
+	/*
+         *  We need to know the number of contact/cal/nt entries
+         *  somewhere so why not do it now.
+         */
+	param = g_new0(struct apparam_field, 1);
+	param->maxlistcount = 0; /* to count the number of vcards... */
+	param->filter = 0x200085; /* UID TEL N VERSION */
+	irmc->params = param;
+	phonebook_pull("telecom/pb.vcf", irmc->params, phonebook_size_result,
+									irmc);
+
+	if (err)
+		*err = 0;
+
+	return irmc;
+}
+
+static int irmc_get(struct obex_session *os, obex_object_t *obj,
+		gboolean *stream, void *user_data)
+{
+	struct irmc_session *irmc = user_data;
+	const char *type = obex_get_type(os);
+	const char *name = obex_get_name(os);
+	char *path;
+	int ret;
+
+	DBG("name %s type %s irmc %p", name, type ? type : "NA", irmc);
+
+	path = g_strdup(name);
+	*stream = TRUE;
+
+	ret = obex_get_stream_start(os, path);
+
+	g_free(path);
+
+	return ret;
+}
+
+static void irmc_disconnect(struct obex_session *os, void *user_data)
+{
+	struct irmc_session *irmc = user_data;
+
+	DBG("");
+
+	manager_unregister_session(os);
+
+        if (irmc->params) {
+                if (irmc->params->searchval)
+			g_free(irmc->params->searchval);
+                g_free(irmc->params);
+        }
+	if (irmc->buffer) {
+		string_free(irmc->buffer);
+		irmc->buffer = NULL;
+	}
+
+	g_free(irmc);
+}
+
+static int irmc_chkput(struct obex_session *os, void *user_data)
+{
+	DBG("");
+	/* Reject all PUTs */
+	return -EBADR;
+}
+
+static void *irmc_open_devinfo(struct irmc_session *irmc, int *err)
+{
+	if (!irmc->buffer)
+		irmc->buffer = g_string_new("");
+
+	g_string_append_printf(irmc->buffer,
+				"MANU:%s\r\n"
+				"MOD:%s\r\n"
+				"SN:%s\r\n"
+				"IRMC-VERSION:1.1\r\n"
+				"PB-TYPE-TX:VCARD2.1\r\n"
+				"PB-TYPE-RX:NONE\r\n"
+				"CAL-TYPE-TX:NONE\r\n"
+				"CAL-TYPE-RX:NONE\r\n"
+				"MSG-TYPE-TX:NONE\r\n"
+				"MSG-TYPE-RX:NONE\r\n"
+				"NOTE-TYPE-TX:NONE\r\n"
+				"NOTE-TYPE-RX:NONE\r\n",
+				irmc->manu, irmc->model, irmc->sn);
+
+	return irmc;
+}
+
+static void *irmc_open_pb(const char *name,
+		 struct irmc_session *irmc, int *err)
+{
+	GString *mybuf;
+	int ret;
+
+	if (!g_strcmp0(name, ".vcf")) {
+		/* how can we tell if the vcard count call already finished? */
+		ret = phonebook_pull("telecom/pb.vcf", irmc->params,
+							query_result, irmc);
+		if (ret < 0) {
+			DBG("phonebook_pull failed...");
+			goto fail;
+		}
+		return irmc;
+	}
+
+	if (!g_strcmp0(name, "/info.log")) {
+		mybuf = g_string_new("");
+		g_string_printf(mybuf, "Total-Records:%d\r\n"
+				"Maximum-Records:%d\r\n"
+				"IEL:2\r\n"
+				"DID:%s\r\n",
+				irmc->params->maxlistcount,
+				irmc->params->maxlistcount, irmc->did);
+	}
+	else if (!strncmp(name, "/luid/", 6)) {
+		name += 6;
+		if (!g_strcmp0(name, "cc.log")) {
+			mybuf = g_string_new("");
+			g_string_printf(mybuf, "%d\r\n", irmc->params->maxlistcount);
+		}
+		else {
+			int l = strlen(name);
+			/* FIXME:
+			 * Reply the same to any *.log so we hopefully force a
+			 * full phonebook dump.
+			 * Is IEL:2 ok?
+			 */
+			if (l > 4 && !g_strcmp0(name + l - 4, ".log")) {
+				DBG("changelog request, force whole book");
+				mybuf = g_string_new("");
+				g_string_printf(mybuf, "SN:%s\r\n"
+							"DID:%s\r\n"
+							"Total-Records:%d\r\n"
+							"Maximum-Records:%d\r\n"
+							"*\r\n",
+						irmc->sn, irmc->did,
+						irmc->params->maxlistcount,
+						irmc->params->maxlistcount);
+			}
+			else {
+				ret = -EBADR;
+				goto fail;
+			}
+		}
+	}
+	else {
+		ret = -EBADR;
+		goto fail;
+	}
+
+	if (!irmc->buffer)
+		irmc->buffer = mybuf;
+	else {
+		irmc->buffer = g_string_append(irmc->buffer, mybuf->str);
+		string_free(mybuf);
+	}
+
+	return irmc;
+
+fail:
+	if (err)
+		*err = ret;
+
+	return NULL;
+}
+
+static void *irmc_open_cal(const char *name,
+		 struct irmc_session *irmc, int *err)
+{
+	/* no suport yet. Just return an empty buffer. cal.vcs */
+	DBG("unsupported, returning empty buffer");
+	if (!irmc->buffer)
+		irmc->buffer = g_string_new("");
+
+	return irmc;
+}
+
+static void *irmc_open_nt(const char *name,
+		 struct irmc_session *irmc, int *err)
+{
+	/* no suport yet. Just return an empty buffer. nt.vnt */
+	DBG("unsupported, returning empty buffer");
+	if (!irmc->buffer)
+		irmc->buffer = g_string_new("");
+
+	return irmc;
+}
+
+static void *irmc_open(const char *name, int oflag, mode_t mode,
+		void *context, size_t *size, int *err)
+{
+	struct irmc_session *irmc = context;
+	int ret;
+	const char *p;
+
+	DBG("name %s context %p", name, context);
+	if (oflag != O_RDONLY) {
+		ret = -EPERM;
+		goto fail;
+	}
+	if (name == NULL || strncmp(name, "telecom/", 8) != 0) {
+		ret = -EBADR;
+		goto fail;
+	}
+
+	p = name + 8;
+	if (!g_strcmp0(p, "devinfo.txt"))
+		return irmc_open_devinfo(irmc, err);
+	else if (!strncmp(p, "pb", 2))
+		return irmc_open_pb(p+2, irmc, err);
+	else if (!strncmp(p, "cal", 3))
+		return irmc_open_cal(p+3, irmc, err);
+	else if (!strncmp(p, "nt", 2))
+		return irmc_open_nt(p+2, irmc, err);
+
+fail:
+	if (err)
+		*err = ret;
+
+	return NULL;
+}
+
+static int irmc_close(void *object)
+{
+	struct irmc_session *irmc = object;
+
+	DBG("");
+	if (irmc->buffer) {
+		string_free(irmc->buffer);
+		irmc->buffer = NULL;
+	}
+
+	return 0;
+}
+
+static ssize_t irmc_read(void *object, void *buf, size_t count, uint8_t *hi)
+{
+	struct irmc_session *irmc = object;
+        int len;
+
+	DBG("buffer %p count %d", irmc->buffer, count);
+	if (!irmc->buffer)
+                return -EAGAIN;
+
+	*hi = OBEX_HDR_BODY;
+	len = string_read(irmc->buffer, buf, count);
+	DBG("returning %d bytes", len);
+	return len;
+}
+
+static struct obex_mime_type_driver irmc_driver = {
+	.target = IRMC_TARGET,
+	.target_size = IRMC_TARGET_SIZE,
+	.open = irmc_open,
+	.close = irmc_close,
+	.read = irmc_read,
+};
+
+static struct obex_service_driver irmc = {
+	.name = "IRMC Sync server",
+	.service = OBEX_IRMC,
+	.channel = IRMC_CHANNEL,
+	.record = IRMC_RECORD,
+	.target = IRMC_TARGET,
+	.target_size = IRMC_TARGET_SIZE,
+	.connect = irmc_connect,
+	.get = irmc_get,
+	.disconnect = irmc_disconnect,
+	.chkput = irmc_chkput
+};
+
+static int irmc_init(void)
+{
+	int err;
+
+	DBG("");
+	err = phonebook_init();
+	if (err < 0)
+		goto fail_pb_init;
+
+	err = obex_mime_type_driver_register(&irmc_driver);
+	if (err < 0)
+                goto fail_mime_irmc;
+
+	err = obex_service_driver_register(&irmc);
+	if (err < 0)
+		goto fail_irmc_reg;
+
+	return 0;
+
+fail_irmc_reg:
+	obex_mime_type_driver_unregister(&irmc_driver);
+fail_mime_irmc:
+	phonebook_exit();
+fail_pb_init:
+	return err;
+}
+
+static void irmc_exit(void)
+{
+	DBG("");
+	obex_service_driver_unregister(&irmc);
+	obex_mime_type_driver_unregister(&irmc_driver);
+	phonebook_exit();
+}
+
+OBEX_PLUGIN_DEFINE(irmc, irmc_init, irmc_exit)
diff --git a/src/main.c b/src/main.c
index 649acf9..ef894cf 100644
--- a/src/main.c
+++ b/src/main.c
@@ -80,6 +80,7 @@ static gboolean option_autoaccept = FALSE;
 static gboolean option_opp = FALSE;
 static gboolean option_ftp = FALSE;
 static gboolean option_pbap = FALSE;
+static gboolean option_irmc = FALSE;
 static gboolean option_pcsuite = FALSE;
 static gboolean option_symlinks = FALSE;
 static gboolean option_syncevolution = FALSE;
@@ -120,6 +121,8 @@ static GOptionEntry options[] = {
 				"Enable Phonebook Access server" },
 	{ "pcsuite", 's', 0, G_OPTION_ARG_NONE, &option_pcsuite,
 				"Enable PC Suite Services server" },
+	{ "irmc", 'i', 0, G_OPTION_ARG_NONE, &option_irmc,
+				"Enable IrMCSync server" },
 	{ "syncevolution", 'e', 0, G_OPTION_ARG_NONE, &option_syncevolution,
 				"Enable OBEX server for SyncEvolution" },
 	{ NULL },
@@ -208,9 +211,10 @@ int main(int argc, char *argv[])
 
 	if (option_opp == FALSE && option_ftp == FALSE &&
 				option_pbap == FALSE &&
+				option_irmc == FALSE &&
 				option_syncevolution == FALSE) {
 		fprintf(stderr, "No server selected (use either "
-				"--opp, --ftp, --pbap or --syncevolution)\n");
+				"--opp, --ftp, --pbap, --irmc or --syncevolution)\n");
 		exit(EXIT_FAILURE);
 	}
 
@@ -266,6 +270,10 @@ int main(int argc, char *argv[])
 				option_autoaccept, option_symlinks,
 				option_capability);
 
+	if (option_irmc == TRUE)
+		obex_server_init(OBEX_IRMC, NULL, TRUE, FALSE, FALSE,
+				option_capability);
+
 	if (option_syncevolution == TRUE)
 		obex_server_init(OBEX_SYNCEVOLUTION, NULL, TRUE, FALSE,
 							FALSE, NULL);
diff --git a/src/obex.h b/src/obex.h
index 9424b6b..35723ca 100644
--- a/src/obex.h
+++ b/src/obex.h
@@ -34,7 +34,8 @@
 #define OBEX_BIP	(1 << 3)
 #define OBEX_PBAP	(1 << 4)
 #define OBEX_PCSUITE	(1 << 5)
-#define OBEX_SYNCEVOLUTION	(1 << 6)
+#define OBEX_IRMC	(1 << 6)
+#define OBEX_SYNCEVOLUTION	(1 << 7)
 
 #define TARGET_SIZE 16
 
-- 
1.7.2


^ permalink raw reply related

* Re: QuiC AMP development
From: Peter Krystad @ 2010-08-12 19:53 UTC (permalink / raw)
  To: sober song
  Cc: Gustavo F. Padovan, Mat Martineau, Ron Shaffer, linux-bluetooth
In-Reply-To: <AANLkTiny5zM=1vEPyn6iE9cJqi0v_o_i12GOUg=n=Dc7@mail.gmail.com>


Hi Sober,

Be careful not to top-post on this mailing list.

> Hi Shaffer/Peter
>
> static inline int send_a2mp_cmd(struct amp_mgr *mgr, u8 ident, u8
> code, u16 len, void *data) {
> 	struct a2mp_cmd_hdr *hdr;
> 	int plen;
> 	u8 *buf;
>
> 	BT_DBG("ident %d code %d", ident, code);
> 	plen = sizeof(*hdr) + len;
> 	buf = kzalloc(plen, GFP_ATOMIC);
> 	if (!buf)
> 		return -ENOMEM;
> 	hdr = (struct a2mp_cmd_hdr *) buf;
> 	hdr->code  = code;
> 	hdr->ident = ident;
> 	hdr->len   = cpu_to_le16(len);
> 	buf += sizeof(*hdr);
> 	memcpy(buf, data, len);
> 	return send_a2mp(mgr->a2mp_sock, (u8 *) hdr, plen);
> }
>
> I see that here have a malloc, but i don't see free, doesn't it cause
> memleak?
>

Looks like it, thanks. As Mat and I explained to Kevin and Dan at the
mini-summit the git repo we posted is a snapshot of our development tree,
and was provided so interested parties could review our approach. As we
post patches to the mailing list your review will be much appreciated.

> Regards
> sober
>
>

>>> >
>>> >* Ron Shaffer <rshaffer@codeaurora.org> [2010-08-08 21:43:36 -0500]:
>>> >
>>> >>As requested in today's summit discussions, here's a link that can be
>>> >>used to inspect the code we've done to add AMP support on the latest
>>> >>next tree.
>>> >>
>>> >>https://www.codeaurora.org/gitweb/quic/bluetooth/?p=bluetooth-next-2.6.git;a=summary
>>> >>branch pk-upstream
>>> >
>>> >


Peter.

-- 

Peter Krystad

Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum



^ permalink raw reply

* Re: Question about phonebook_add_contact
From: Vinicius Gomes @ 2010-08-12 17:59 UTC (permalink / raw)
  To: Radoslaw Jablonski; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <4C63F363.4000400@nokia.com>

Hi Radek,

On Thu, Aug 12, 2010 at 10:13 AM, Radoslaw Jablonski
<ext-jablonski.radoslaw@nokia.com> wrote:
>
>  Hi!
> I've got question about this part of code from beginning
>  phonebook_add_contact func:
>
> if ((contact->numbers == NULL && (contact->fullname == NULL ||
>                        contact->fullname[0] == '\0')))
>        return;
>
> Is this checking really needed in that place? It is possible to have
> contacts data
> in phonebook that have all data except phone set (by example: email
> addresses,
> main address etc). In that condition when we want to pull target vcard
> entry, then
> empty string will be returned even if user send ATTRIBUTE_MASK to include
> all fields
> in the result.
>

That piece of code is a legacy from simpler times, when we supported
less types of contact information.

> I'm asking because I have problem with "0.vcf" vcard (this is responsible
> for personal
> information of device owner). This entry by default does not have any
> phone's data
> filled in address book but it is mandatory to put this in vcard-listing.
> Actually, I am forced to initialize contact->numbers list with empty number
> even if
> none of number is available to return correct result for "0.vcf."(in
> phonebook-tracker)...
>
> Can this 'contact->numbers' checking be removed from there or maybe other
> phonebooks
> rely on this behaviour?

>From what I know, the Tracker backend is the only thing that relies on
that, so I think that check could be removed with little to no
problem. In fact, as you are already dealing with the empty numbers
list case, the problem is already solved.

>
> BR,
> Radek
>
>
> --
> 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: [PATCH] IrMC sync server support
From: Marcel J.E. Mol @ 2010-08-12 15:39 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <20100812145703.GD7345@joshua.mesa.nl>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset*=utf-8''%E2%80%9Cus-ascii, Size: 2553 bytes --]

On Thu, Aug 12, 2010 at 04:57:03PM +0200, Luiz Augusto von Dentz wrote:
> Hi Marcel,
> 
> On Thu, Aug 12, 2010 at 3:12 PM, Marcel J.E. Mol <marcel@mesa.nl> wrote:
> > abstracting it made de irmc_open() more clean (e.g. no buffer fills in
> > irmc_open()) and more readable in my opinion.
> > The specs for devinfo.txt contain many more attributes but the currrent
> > set seems to be enough. In future this may need to be extended and
> > supporting scripts may be needed for getting the info. The current
> > values are actually not correct I think as the will be the same for any
> > device now... (hence the hint for using x-obex/capabilities in
> > irmc_connect().)
> 
> It doesn't look like we can use x-obex/capabilities for this
> devinfo.txt, afaik they are not the same thing, thats why I suggested
> open it as any regular file so the platforms has some way to customize
> it. Executing scripts is always tricky since you have to depend on
> async io to suspend the request while the script is running and resume
> when finished. Of course if there is no file called devinfo.txt then
> we can think about a default value, what does the spec says in this
> regard, is devinfo.txt mandatory? If not then we can just fail as we
> do for x-obex/capabilities when not set.

I don;t know how diffenent capbilities is from the things devinfo.txt
needs. On the n900 the capabilities script seems to provide what is needed
for devinfo.txt so I was lead by that...

I'm not suere how mandatory devinfo.txt is. My carkit did ask for it but
when I returned some data to it it never asked again... It probably will 
when I remove my phones configuration from the carkit.

I never checked the specs again for it as it all seems to work fine with
the code as it is now. It also works for other n900 owners so i did not
bother any more.

I think the code seems to be in a reasonable state to be usefull now so
that is why I released it here. There is probably lots of extra work to be
done to be fully specs complient. But by releasing it other people can jump 
in and improve...

Thanks,

-Marcel
-- 
     ======--------         Marcel J.E. Mol                MESA Consulting B.V.
    =======---------        ph. +31-(0)6-54724868          P.O. Box 112
    =======---------        marcel@mesa.nl                 2630 AC  Nootdorp
__==== www.mesa.nl ---____U_n_i_x______I_n_t_e_r_n_e_t____ The Netherlands ____
 They couldn't think of a number,           Linux user 1148  --  counter.li.org
    so they gave me a name!  -- Rupert Hine  --  www.ruperthine.com

^ permalink raw reply

* Re: [PATCH] IrMC sync server support
From: Luiz Augusto von Dentz @ 2010-08-12 14:57 UTC (permalink / raw)
  To: Marcel J.E. Mol; +Cc: linux-bluetooth
In-Reply-To: <20100812121212.GA16738@joshua.mesa.nl>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset*=utf-8''%E2%80%9Cus-ascii, Size: 1210 bytes --]

Hi Marcel,

On Thu, Aug 12, 2010 at 3:12 PM, Marcel J.E. Mol <marcel@mesa.nl> wrote:
> abstracting it made de irmc_open() more clean (e.g. no buffer fills in
> irmc_open()) and more readable in my opinion.
> The specs for devinfo.txt contain many more attributes but the currrent
> set seems to be enough. In future this may need to be extended and
> supporting scripts may be needed for getting the info. The current
> values are actually not correct I think as the will be the same for any
> device now... (hence the hint for using x-obex/capabilities in
> irmc_connect().)

It doesn't look like we can use x-obex/capabilities for this
devinfo.txt, afaik they are not the same thing, thats why I suggested
open it as any regular file so the platforms has some way to customize
it. Executing scripts is always tricky since you have to depend on
async io to suspend the request while the script is running and resume
when finished. Of course if there is no file called devinfo.txt then
we can think about a default value, what does the spec says in this
regard, is devinfo.txt mandatory? If not then we can just fail as we
do for x-obex/capabilities when not set.

-- 
Luiz Augusto von Dentz
Computer Engineer

^ permalink raw reply

* Re: [PATCH] IrMC sync server support
From: Marcel J.E. Mol @ 2010-08-12 14:56 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1281613299.12579.266.camel@localhost.localdomain>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset*=utf-8''%E2%80%9Cus-ascii, Size: 18961 bytes --]

On Thu, Aug 12, 2010 at 07:41:39AM -0400, Marcel Holtmann wrote:
> Hi Marcel,
> 
> > A reasonable working IrMC SYNC server (only full phonebook sync support)
> > Support for cal and note by just returning nothing.
> > ---
> >  Makefile.am    |    3 +
> >  plugins/irmc.c |  488 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/main.c     |   12 ++-
> >  src/obex.h     |    1 +
> >  4 files changed, 502 insertions(+), 2 deletions(-)
> >  create mode 100644 plugins/irmc.c
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 73e2f28..ce8c675 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -58,6 +58,9 @@ builtin_sources += plugins/pbap.c plugins/phonebook.h \
> >  builtin_modules += syncevolution
> >  builtin_sources += plugins/syncevolution.c
> >  
> > +builtin_modules += irmc
> > +builtin_sources += plugins/irmc.c plugins/phonebook.h
> > +
> >  builtin_nodist += plugins/phonebook.c
> >  
> >  libexec_PROGRAMS += src/obexd
> > diff --git a/plugins/irmc.c b/plugins/irmc.c
> > new file mode 100644
> > index 0000000..96fd807
> > --- /dev/null
> > +++ b/plugins/irmc.c
> > @@ -0,0 +1,488 @@
> > +/*
> > + *
> > + *  OBEX IrMC Sync Server
> > + *
> > + *  Copyright (C) 2010  Marcel Mol <marcel@mesa.nl>
> > + *
> > + *  This program is free software; you can redistribute it and/or modify
> > + *  it under the terms of the GNU General Public License as published by
> > + *  the Free Software Foundation; either version 2 of the License, or
> > + *  (at your option) any later version.
> > + *
> > + *  This program is distributed in the hope that it will be useful,
> > + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + *  GNU General Public License for more details.
> > + *
> > + *  You should have received a copy of the GNU General Public License
> > + *  along with this program; if not, write to the Free Software
> > + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> > + *
> > + */
> > +
> > +#ifdef HAVE_CONFIG_H
> > +#include <config.h>
> > +#endif
> > +
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include <errno.h>
> > +#include <glib.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <arpa/inet.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> > +
> > +#include <openobex/obex.h>
> > +#include <openobex/obex_const.h>
> > +
> > +#include "plugin.h"
> > +#include "log.h"
> > +#include "obex.h"
> > +#include "service.h"
> > +#include "phonebook.h"
> > +#include "mimetype.h"
> > +#include "filesystem.h"
> > +#include "dbus.h"
> > +
> > +#define IRMC_CHANNEL	17
> 
> we did have a list of defined RFCOMM channels for various protocols, but
> just don't remember where it is.
> 
> Johan, we should put that into the doc/ directory actually. Can you dig
> up that list and commit it as text file.
> 
> > +
> > +#define IRMC_RECORD "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>	\
> > +<record>								\
> > +  <attribute id=\"0x0001\">						\
> > +    <sequence>								\
> > +      <uuid value=\"0x1104\"/>						\
> > +    </sequence>								\
> > +  </attribute>								\
> > +									\
> > +  <attribute id=\"0x0004\">						\
> > +    <sequence>								\
> > +      <sequence>							\
> > +        <uuid value=\"0x0100\"/>					\
> > +      </sequence>							\
> > +      <sequence>							\
> > +        <uuid value=\"0x0003\"/>					\
> > +        <uint8 value=\"%u\" name=\"channel\"/>				\
> > +      </sequence>							\
> > +      <sequence>							\
> > +        <uuid value=\"0x0008\"/>					\
> > +      </sequence>							\
> > +    </sequence>								\
> > +  </attribute>								\
> > +									\
> > +  <attribute id=\"0x0009\">						\
> > +    <sequence>								\
> > +      <sequence>							\
> > +        <uuid value=\"0x1104\"/>					\
> > +        <uint16 value=\"0x0100\" name=\"version\"/>			\
> > +      </sequence>							\
> > +    </sequence>								\
> > +  </attribute>								\
> > +									\
> > +  <attribute id=\"0x0100\">						\
> > +    <text value=\"%s\" name=\"name\"/>					\
> > +  </attribute>								\
> > +									\
> > +  <attribute id=\"0x0301\">						\
> > +    <sequence>								\
> > +      <uint8 value=\"0x01\"/>						\
> > +    </sequence>								\
> > +  </attribute>								\
> > +</record>"
> > +
> > +
> > +struct aparam_header {
> > +	uint8_t tag;
> > +	uint8_t len;
> > +	uint8_t val[0];
> > +} __attribute__ ((packed));
> > +
> > +#define DIDLEN 18
> > +struct irmc_session {
> > +	struct obex_session *os;
> > +	struct apparam_field *params;
> > +	uint16_t entries;
> > +	GString *buffer;
> > +	char sn[DIDLEN];
> > +	char did[DIDLEN];
> > +	char manu[DIDLEN];
> > +	char model[DIDLEN];
> > +};
> 
> What is DIDLEN.

Just to have a symbolic name for the max length of the char strings in 
the irmc_session struct.
All strings may need a different max length, but for now a single value
suffices. (see also the comment in irmc_connect())

> 
> > +
> > +#define IRMC_TARGET_SIZE 9
> > +
> > +static const guint8 IRMC_TARGET[IRMC_TARGET_SIZE] = {
> > +			0x49, 0x52, 0x4d, 0x43,  0x2d, 0x53, 0x59, 0x4e, 0x43 };
> > +
> > +static void phonebook_size_result(const char *buffer, size_t bufsize,
> > +				int vcards, int missed, void *user_data)
> > +{
> > +	struct irmc_session *irmc = user_data;
> > +
> > +	DBG("vcards %d", vcards);
> > +
> > +	irmc->params->maxlistcount = vcards;
> > +}
> > +
> > +static void query_result(const char *buffer, size_t bufsize, int vcards,
> > +					int missed, void *user_data)
> > +{
> > +	struct irmc_session *irmc = user_data;
> > +	const char *s;
> > +	char *t;
> > +
> > +	DBG("bufsize %d vcards %d missed %d", bufsize, vcards, missed);
> > +
> > +	// first create an empty 'owner' vcard
> 
> As Luiz already mentioned, please use /* */ style comments.

Fine

> 
> > +	t = "BEGIN:VCARD\r\n"
> > +		"VERSION:2.1\r\n"
> > +		"N:\r\n"
> > +		"TEL:\r\n"
> > +		"X-IRMX-LUID:0\r\n"
> > +		"END:VCARDi\r\n";
 
> You really wanna use const char *t here. And I would even prefer if you
> make that one static and global.

Ok will do.
 
> > +	if (!irmc->buffer)
> > +		irmc->buffer = g_string_new_len(t, strlen(t));
> > +	else
> > +		irmc->buffer = g_string_append_len(irmc->buffer, t, strlen(t));
> 
> Why the string_append_len. Just append it it.

Don't know :). Will change it.

> 
> > +	// loop around buffer and add X-IRMC-LUID attribs
> > +	s = buffer;
> > +	while ((t = strstr(s, "UID:")) != NULL) {
> > +		// add upto UID: into buffer
> > +		irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
> > +		// add UID: line into buffer
> > +		// Not sure if UID is still needed if X-IRMC-LUID is there
> > +		s = t;
> > +		t = strstr(s, "\r\n");
> > +		t += 2;
> > +		irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
> > +		// add X-IRMC-LUID with same number as UID
> > +		irmc->buffer = g_string_append_len(irmc->buffer,
> > +							"X-IRMC-LUID:", 12);
> > +		s += 4; // point to uid number
> > +		irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
> > +		s = t;
> > +	}
> > +	// add remaining bit of buffer
> > +	irmc->buffer = g_string_append_len(irmc->buffer, s,
> > +							bufsize - (s-buffer));
> > +
> > +	obex_object_set_io_flags(irmc, G_IO_IN, 0);
> > +}
> > +
> > +static void *irmc_connect(struct obex_session *os, int *err)
> > +{
> > +	struct irmc_session *irmc;
> > +	struct apparam_field *param;
> > +
> > +	DBG("");
> > +
> > +	manager_register_session(os);
> > +
> > +	irmc = g_new0(struct irmc_session, 1);
> > +	irmc->os = os;
> > +
> > +	/*
> > +	 * ideally get capabilities info here and use that to define
> > +	 * IrMC DID and SN etc parameters.
> > +	 * For now lets used hostname and some 'random' value
> > +	 */
> > +	gethostname(irmc->did, DIDLEN);
> > +	strncpy(irmc->sn, "12345", DIDLEN);
> > +	strncpy(irmc->manu, "obex", DIDLEN);
> > +	strncpy(irmc->model, "mymodel", DIDLEN);
> > +
> > +	/*
> > +         *  we need to know the number of contact/cal/nt entries
> > +         *  somewhere so why not do it now.
> > +         */
> > +	param = g_new0(struct apparam_field, 1);
> > +	param->maxlistcount = 0; // to count the number of vcards...
> > +	param->filter = 0x200085; // UID TEL N VERSION
> > +	irmc->params = param;
> > +	phonebook_pull("telecom/pb.vcf", irmc->params, phonebook_size_result,
> > +									irmc);
> > +
> > +	if (err)
> > +		*err = 0;
> > +
> > +	return irmc;
> > +}
> > +
> > +static int irmc_get(struct obex_session *os, obex_object_t *obj,
> > +		gboolean *stream, void *user_data)
> > +{
> > +	struct irmc_session *irmc = user_data;
> > +	const char *type = obex_get_type(os);
> > +	const char *name = obex_get_name(os);
> > +	char *path;
> > +	int ret;
> > +
> > +	DBG("name %s type %s irmc %p", name, type ? type : "NA", irmc);
> > +
> > +	path = g_strdup(name);
> > +	*stream = TRUE;
> > +
> > +	ret = obex_get_stream_start(os, path);
> > +
> > +	g_free(path);
> > +
> > +	return ret;
> > +}
> > +
> > +static void irmc_disconnect(struct obex_session *os, void *user_data)
> > +{
> > +	struct irmc_session *irmc = user_data;
> > +
> > +	DBG("");
> > +
> > +	manager_unregister_session(os);
> > +
> > +        if (irmc->params) {
> > +                if (irmc->params->searchval)
> > +			g_free(irmc->params->searchval);
> > +                g_free(irmc->params);
> > +        }
> > +	if (irmc->buffer) {
> > +		string_free(irmc->buffer);
> > +		irmc->buffer = NULL;
> > +	}
> > +
> > +	g_free(irmc);
> > +}
> > +
> > +static int irmc_chkput(struct obex_session *os, void *user_data)
> > +{
> > +	DBG("");
> > +	/* Reject all PUTs */
> > +	return -EBADR;
> > +}
> > +
> > +static void *irmc_open_devinfo(struct irmc_session *irmc, int *err)
> > +{
> > +	char mybuf[1024];
> > +
> > +	sprintf(mybuf, "MANU:%s\r\n"
> > +			"MOD:%s\r\n"
> > +			"SN:%s\r\n"
> > +			"PB-TYPE-TX:VCARD2.1\r\n",
> > +			irmc->manu, irmc->model, irmc->sn);
> > +
> > +	if (!irmc->buffer)
> > +		irmc->buffer = g_string_new(mybuf);
> > +	else
> > +		irmc->buffer = g_string_append(irmc->buffer, mybuf);
> 
> We have something that is called g_string_append_printf(). Use that one
> and not allocate a buf up-front.

thanks, was not aware of this.

> 
> > +
> > +	return irmc;
> > +}
> > +
> > +static void *irmc_open_pb(const char *name,
> > +		 struct irmc_session *irmc, int *err)
> > +{
> > +	char mybuf[1024];
> > +	int ret;
> > +
> > +	if (!g_strcmp0(name, ".vcf")) {
> > +		// how can we tell if the vcard count call already finished?
> > +		ret = phonebook_pull("telecom/pb.vcf", irmc->params,
> > +							query_result, irmc);
> > +		if (ret < 0) {
> > +			DBG("phonebook_pull failed...");
> > +			goto fail;
> > +		}
> > +		return irmc;
> > +	}
> > +
> > +	if (!g_strcmp0(name, "/info.log")) {
> > +		sprintf(mybuf, "Total-Records:%d\r\n"
> > +				"Maximum-Records:%d\r\n"
> > +				"DID:%s\r\n",
> > +				irmc->params->maxlistcount,
> > +				irmc->params->maxlistcount, irmc->did);
> > +	}
> 
> Same here. A GLib _printf() function is way better usage.

Ok wil look into that. This one may need some more thinking to make in nice and clean 

> 
> > +	else if (!strncmp(name, "/luid/", 6)) {
> > +		name += 6;
> > +		if (!g_strcmp0(name, "cc.log"))
> > +			sprintf(mybuf, "%d\r\n", irmc->params->maxlistcount);
> > +		else {
> > +			int l = strlen(name);
> > +			/* FIXME
> > +			 * reply the same to any *.log so we hopefully force a
> > +			 * full phonebook dump.
> > +			 * Is IEL:2 ok?
> > +			 */
> > +			if (l > 4 && !g_strcmp0(name + l - 4, ".log")) {
> > +				DBG("changelog request, force whole book");
> > +				sprintf(mybuf, "SN:%s\r\n"
> > +						"IEL:2\r\n"
> > +						"DID:%s\r\n"
> > +						"Total-Records:%d\r\n"
> > +						"Maximum-Records:%d\r\n"
> > +						"*\r\n",
> > +					irmc->sn, irmc->did,
> > +					irmc->params->maxlistcount,
> > +					irmc->params->maxlistcount);
> > +			}
> > +			else {
> > +				ret = -EBADR;
> > +				goto fail;
> > +			}
> > +		}
> > +	}
> > +	else {
> > +		ret = -EBADR;
> > +		goto fail;
> > +	}
> > +
> > +	if (!irmc->buffer)
> > +		irmc->buffer = g_string_new(mybuf);
> > +	else
> > +		irmc->buffer = g_string_append(irmc->buffer, mybuf);
> > +
> > +	return irmc;
> > +
> > +fail:
> > +	if (err)
> > +		*err = ret;
> > +
> > +	return NULL;
> > +}
> > +
> > +static void *irmc_open_cal(const char *name,
> > +		 struct irmc_session *irmc, int *err)
> > +{
> > +	// no suport yet. Just return an empty buffer
> > +	//cal.vcs
> > +	DBG("unsupported, returning empty buffer");
> > +	if (!irmc->buffer)
> > +		irmc->buffer = g_string_new("");
> > +	else
> > +		irmc->buffer = g_string_append(irmc->buffer, "");
> > +
> > +	return irmc;
> 
> Empty buffer is empty buffer. Just return it.

Well, an empty buffer it is. irmc->buffer may not be NULL after as that
will be seen as 'no data available yet'.

> 
> > +}
> > +
> > +static void *irmc_open_nt(const char *name,
> > +		 struct irmc_session *irmc, int *err)
> > +{
> > +	// no suport yet. Just return an empty buffer
> > +	//nt.vnt
> > +	DBG("unsupported, returning empty buffer");
> > +	if (!irmc->buffer)
> > +		irmc->buffer = g_string_new("");
> > +	else
> > +		irmc->buffer = g_string_append(irmc->buffer, "");
> > +
> > +	return irmc;
> 
> Same here.
> 
> > +}
> > +
> > +static void *irmc_open(const char *name, int oflag, mode_t mode,
> > +		void *context, size_t *size, int *err)
> > +{
> > +	struct irmc_session *irmc = context;
> > +	int ret;
> > +	const char *p;
> > +
> > +	DBG("name %s context %p", name, context);
> > +	if (oflag != O_RDONLY) {
> > +		ret = -EPERM;
> > +		goto fail;
> > +	}
> > +	if (strncmp(name, "telecom/", 8) != NULL) {
> > +		ret = -EBADR;
> > +		goto fail;
> > +	}
> > +
> > +	p = name + 8;
> > +	if (!g_strcmp0(p, "devinfo.txt"))
> > +		return irmc_open_devinfo(irmc, err);
> > +	else if (!strncmp(p, "pb", 2))
> > +		return irmc_open_pb(p+2, irmc, err);
> > +	else if (!strncmp(p, "cal", 3))
> > +		return irmc_open_cal(p+3, irmc, err);
> > +	else if (!strncmp(p, "nt", 2))
> > +		return irmc_open_nt(p+2, irmc, err);
> > +
> > +fail:
> > +	if (err)
> > +		*err = ret;
> > +
> > +	return NULL;
> > +}
> > +
> > +static int irmc_close(void *object)
> > +{
> > +	struct irmc_session *irmc = object;
> > +
> > +	DBG("");
> > +	if (irmc->buffer) {
> > +		string_free(irmc->buffer);
> > +		irmc->buffer = NULL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static ssize_t irmc_read(void *object, void *buf, size_t count, uint8_t *hi)
> > +{
> > +	struct irmc_session *irmc = object;
> > +        int len;
> > +
> > +	DBG("buffer %p count %d", irmc->buffer, count);
> > +	if (!irmc->buffer)
> > +                return -EAGAIN;
> > +
> > +        *hi = OBEX_HDR_BODY;
> > +	len = string_read(irmc->buffer, buf, count);
> > +	DBG("returning %d bytes", len);
> > +	return len;
> > +}
> > +
> > +static struct obex_mime_type_driver irmc_driver = {
> > +	.target = IRMC_TARGET,
> > +	.target_size = IRMC_TARGET_SIZE,
> > +	.open = irmc_open,
> > +	.close = irmc_close,
> > +	.read = irmc_read,
> > +};
> > +
> > +static struct obex_service_driver irmc = {
> > +	.name = "IRMC Sync server",
> > +	.service = OBEX_IRMC,
> > +	.channel = IRMC_CHANNEL,
> > +	.record = IRMC_RECORD,
> > +	.target = IRMC_TARGET,
> > +	.target_size = IRMC_TARGET_SIZE,
> > +	.connect = irmc_connect,
> > +	.get = irmc_get,
> > +	.disconnect = irmc_disconnect,
> > +	.chkput = irmc_chkput
> > +};
> > +
> > +static int irmc_init(void)
> > +{
> > +	int err;
> > +
> > +	DBG("");
> > +	err = phonebook_init();
> > +	if (err < 0)
> > +		return err;
> > +
> > +	err = obex_mime_type_driver_register(&irmc_driver);
> > +	if (err < 0)
> > +		return err;
> 
> Don't you need to phonebook_exit() here?
> 
> > +
> > +	return obex_service_driver_register(&irmc);
> > +}
> 
> Also what happens if this one fails. You do need to clean up.

Oops, should have seen this when it was fixsed for pbap.

> 
> > +
> > +static void irmc_exit(void)
> > +{
> > +	DBG("");
> > +	obex_service_driver_unregister(&irmc);
> > +	obex_mime_type_driver_unregister(&irmc_driver);
> > +	phonebook_exit();
> > +}
> > +
> > +OBEX_PLUGIN_DEFINE(irmc, irmc_init, irmc_exit)
> > diff --git a/src/main.c b/src/main.c
> > index 649acf9..96523a6 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -83,6 +83,7 @@ static gboolean option_pbap = FALSE;
> >  static gboolean option_pcsuite = FALSE;
> >  static gboolean option_symlinks = FALSE;
> >  static gboolean option_syncevolution = FALSE;
> > +static gboolean option_irmc = FALSE;
> >  
> >  static gboolean parse_debug(const char *key, const char *value,
> >  				gpointer user_data, GError **error)
> > @@ -122,6 +123,8 @@ static GOptionEntry options[] = {
> >  				"Enable PC Suite Services server" },
> >  	{ "syncevolution", 'e', 0, G_OPTION_ARG_NONE, &option_syncevolution,
> >  				"Enable OBEX server for SyncEvolution" },
> > +	{ "irmc", 'i', 0, G_OPTION_ARG_NONE, &option_irmc,
> > +				"Enable IrMCSync server" },
> >  	{ NULL },
> >  };
> 
> Just put irmc before syncevolution. Like nicer ;)

Agree :)
 
> >  
> > @@ -208,9 +211,10 @@ int main(int argc, char *argv[])
> >  
> >  	if (option_opp == FALSE && option_ftp == FALSE &&
> >  				option_pbap == FALSE &&
> > -				option_syncevolution == FALSE) {
> > +				option_syncevolution == FALSE &&
> > +				option_irmc == FALSE) {
> >  		fprintf(stderr, "No server selected (use either "
> > -				"--opp, --ftp, --pbap or --syncevolution)\n");
> > +				"--opp, --ftp, --pbap --syncevolution or --irmc)\n");
> >  		exit(EXIT_FAILURE);
> >  	}
> 
> Same here. Leave syncevolution at the end.
> 
> >  
> > @@ -270,6 +274,10 @@ int main(int argc, char *argv[])
> >  		obex_server_init(OBEX_SYNCEVOLUTION, NULL, TRUE, FALSE,
> >  							FALSE, NULL);
> >  
> > +	if (option_irmc == TRUE)
> > +		obex_server_init(OBEX_IRMC, NULL, TRUE, FALSE, FALSE,
> > +				option_capability);
> > +
> 
> And here again.
> 
> >  	if (!root_folder_setup(option_root, option_root_setup)) {
> >  		error("Unable to setup root folder %s", option_root);
> >  		exit(EXIT_FAILURE);
> > diff --git a/src/obex.h b/src/obex.h
> > index 9424b6b..cfe9159 100644
> > --- a/src/obex.h
> > +++ b/src/obex.h
> > @@ -35,6 +35,7 @@
> >  #define OBEX_PBAP	(1 << 4)
> >  #define OBEX_PCSUITE	(1 << 5)
> >  #define OBEX_SYNCEVOLUTION	(1 << 6)
> > +#define OBEX_IRMC	(1 << 7)
> >  
> 
> And here as well. Just use 1 << 6 for IRMC and move SYNCEVO to 1 << 7.
> 
> >  #define TARGET_SIZE 16
> >  
> 
> Regards
> 
> Marcel
> 

thanks for your comments Marcel

-Marcel
-- 
     ======--------         Marcel J.E. Mol                MESA Consulting B.V.
    =======---------        ph. +31-(0)6-54724868          P.O. Box 112
    =======---------        marcel@mesa.nl                 2630 AC  Nootdorp
__==== www.mesa.nl ---____U_n_i_x______I_n_t_e_r_n_e_t____ The Netherlands ____
 They couldn't think of a number,           Linux user 1148  --  counter.li.org
    so they gave me a name!  -- Rupert Hine  --  www.ruperthine.com

^ permalink raw reply

* Re: [PATCH] IrMC sync server support
From: Marcel J.E. Mol @ 2010-08-12 14:56 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <AANLkTikoUKpDtT5CU7hP+QZBkbc9-p_YymXGPUbJVB+s@mail.gmail.com>

On Thu, Aug 12, 2010 at 02:16:30PM +0300, Luiz Augusto von Dentz wrote:
> Hi Marcel,
> 
> On Tue, Jul 27, 2010 at 8:04 PM, Marcel Mol <marcel@mesa.nl> wrote:
> > A reasonable working IrMC SYNC server (only full phonebook sync support)
> > Support for cal and note by just returning nothing.
> > ---
> >  Makefile.am    |    3 +
> >  plugins/irmc.c |  488 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/main.c     |   12 ++-
> >  src/obex.h     |    1 +
> >  4 files changed, 502 insertions(+), 2 deletions(-)
> >  create mode 100644 plugins/irmc.c
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 73e2f28..ce8c675 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -58,6 +58,9 @@ builtin_sources += plugins/pbap.c plugins/phonebook.h \
> >  builtin_modules += syncevolution
> >  builtin_sources += plugins/syncevolution.c
> >
> > +builtin_modules += irmc
> > +builtin_sources += plugins/irmc.c plugins/phonebook.h
> > +
> >  builtin_nodist += plugins/phonebook.c
> >
> >  libexec_PROGRAMS += src/obexd
> > diff --git a/plugins/irmc.c b/plugins/irmc.c
> > new file mode 100644
> > index 0000000..96fd807
> > --- /dev/null
> > +++ b/plugins/irmc.c
> > @@ -0,0 +1,488 @@
> > +/*
> > + *
> > + *  OBEX IrMC Sync Server
> > + *
> > + *  Copyright (C) 2010  Marcel Mol <marcel@mesa.nl>
> > + *
> > + *  This program is free software; you can redistribute it and/or modify
> > + *  it under the terms of the GNU General Public License as published by
> > + *  the Free Software Foundation; either version 2 of the License, or
> > + *  (at your option) any later version.
> > + *
> > + *  This program is distributed in the hope that it will be useful,
> > + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + *  GNU General Public License for more details.
> > + *
> > + *  You should have received a copy of the GNU General Public License
> > + *  along with this program; if not, write to the Free Software
> > + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> > + *
> > + */
> > +
> > +#ifdef HAVE_CONFIG_H
> > +#include <config.h>
> > +#endif
> > +
> > +#include <stdio.h>
> > +#include <string.h>
> > +#include <errno.h>
> > +#include <glib.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <arpa/inet.h>
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> > +
> > +#include <openobex/obex.h>
> > +#include <openobex/obex_const.h>
> > +
> > +#include "plugin.h"
> > +#include "log.h"
> > +#include "obex.h"
> > +#include "service.h"
> > +#include "phonebook.h"
> > +#include "mimetype.h"
> > +#include "filesystem.h"
> > +#include "dbus.h"
> > +
> > +#define IRMC_CHANNEL   17
> > +
> > +#define IRMC_RECORD "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>       \
> > +<record>                                                               \
> > +  <attribute id=\"0x0001\">                                            \
> > +    <sequence>                                                         \
> > +      <uuid value=\"0x1104\"/>                                         \
> > +    </sequence>                                                                \
> > +  </attribute>                                                         \
> > +                                                                       \
> > +  <attribute id=\"0x0004\">                                            \
> > +    <sequence>                                                         \
> > +      <sequence>                                                       \
> > +        <uuid value=\"0x0100\"/>                                       \
> > +      </sequence>                                                      \
> > +      <sequence>                                                       \
> > +        <uuid value=\"0x0003\"/>                                       \
> > +        <uint8 value=\"%u\" name=\"channel\"/>                         \
> > +      </sequence>                                                      \
> > +      <sequence>                                                       \
> > +        <uuid value=\"0x0008\"/>                                       \
> > +      </sequence>                                                      \
> > +    </sequence>                                                                \
> > +  </attribute>                                                         \
> > +                                                                       \
> > +  <attribute id=\"0x0009\">                                            \
> > +    <sequence>                                                         \
> > +      <sequence>                                                       \
> > +        <uuid value=\"0x1104\"/>                                       \
> > +        <uint16 value=\"0x0100\" name=\"version\"/>                    \
> > +      </sequence>                                                      \
> > +    </sequence>                                                                \
> > +  </attribute>                                                         \
> > +                                                                       \
> > +  <attribute id=\"0x0100\">                                            \
> > +    <text value=\"%s\" name=\"name\"/>                                 \
> > +  </attribute>                                                         \
> > +                                                                       \
> > +  <attribute id=\"0x0301\">                                            \
> > +    <sequence>                                                         \
> > +      <uint8 value=\"0x01\"/>                                          \
> > +    </sequence>                                                                \
> > +  </attribute>                                                         \
> > +</record>"
> > +
> > +
> > +struct aparam_header {
> > +       uint8_t tag;
> > +       uint8_t len;
> > +       uint8_t val[0];
> > +} __attribute__ ((packed));
> > +
> > +#define DIDLEN 18
> > +struct irmc_session {
> > +       struct obex_session *os;
> > +       struct apparam_field *params;
> > +       uint16_t entries;
> > +       GString *buffer;
> > +       char sn[DIDLEN];
> > +       char did[DIDLEN];
> > +       char manu[DIDLEN];
> > +       char model[DIDLEN];
> > +};
> > +
> > +#define IRMC_TARGET_SIZE 9
> > +
> > +static const guint8 IRMC_TARGET[IRMC_TARGET_SIZE] = {
> > +                       0x49, 0x52, 0x4d, 0x43,  0x2d, 0x53, 0x59, 0x4e, 0x43 };
> > +
> > +static void phonebook_size_result(const char *buffer, size_t bufsize,
> > +                               int vcards, int missed, void *user_data)
> > +{
> > +       struct irmc_session *irmc = user_data;
> > +
> > +       DBG("vcards %d", vcards);
> > +
> > +       irmc->params->maxlistcount = vcards;
> > +}
> > +
> > +static void query_result(const char *buffer, size_t bufsize, int vcards,
> > +                                       int missed, void *user_data)
> > +{
> > +       struct irmc_session *irmc = user_data;
> > +       const char *s;
> > +       char *t;
> > +
> > +       DBG("bufsize %d vcards %d missed %d", bufsize, vcards, missed);
> > +
> > +       // first create an empty 'owner' vcard
> > +       t = "BEGIN:VCARD\r\n"
> > +               "VERSION:2.1\r\n"
> > +               "N:\r\n"
> > +               "TEL:\r\n"
> > +               "X-IRMX-LUID:0\r\n"
> > +               "END:VCARDi\r\n";
> > +       if (!irmc->buffer)
> > +               irmc->buffer = g_string_new_len(t, strlen(t));
> > +       else
> > +               irmc->buffer = g_string_append_len(irmc->buffer, t, strlen(t));
> > +
> > +       // loop around buffer and add X-IRMC-LUID attribs
> > +       s = buffer;
> > +       while ((t = strstr(s, "UID:")) != NULL) {
> > +               // add upto UID: into buffer
> > +               irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
> > +               // add UID: line into buffer
> > +               // Not sure if UID is still needed if X-IRMC-LUID is there
> > +               s = t;
> > +               t = strstr(s, "\r\n");
> > +               t += 2;
> > +               irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
> > +               // add X-IRMC-LUID with same number as UID
> > +               irmc->buffer = g_string_append_len(irmc->buffer,
> > +                                                       "X-IRMC-LUID:", 12);
> > +               s += 4; // point to uid number
> > +               irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
> > +               s = t;
> > +       }
> > +       // add remaining bit of buffer
> > +       irmc->buffer = g_string_append_len(irmc->buffer, s,
> > +                                                       bufsize - (s-buffer));
> > +
> > +       obex_object_set_io_flags(irmc, G_IO_IN, 0);
> > +}
> > +
> > +static void *irmc_connect(struct obex_session *os, int *err)
> > +{
> > +       struct irmc_session *irmc;
> > +       struct apparam_field *param;
> > +
> > +       DBG("");
> > +
> > +       manager_register_session(os);
> > +
> > +       irmc = g_new0(struct irmc_session, 1);
> > +       irmc->os = os;
> > +
> > +       /*
> > +        * ideally get capabilities info here and use that to define
> > +        * IrMC DID and SN etc parameters.
> > +        * For now lets used hostname and some 'random' value
> > +        */
> > +       gethostname(irmc->did, DIDLEN);
> > +       strncpy(irmc->sn, "12345", DIDLEN);
> > +       strncpy(irmc->manu, "obex", DIDLEN);
> > +       strncpy(irmc->model, "mymodel", DIDLEN);
> > +
> > +       /*
> > +         *  we need to know the number of contact/cal/nt entries
> > +         *  somewhere so why not do it now.
> > +         */
> > +       param = g_new0(struct apparam_field, 1);
> > +       param->maxlistcount = 0; // to count the number of vcards...
> > +       param->filter = 0x200085; // UID TEL N VERSION
> > +       irmc->params = param;
> > +       phonebook_pull("telecom/pb.vcf", irmc->params, phonebook_size_result,
> > +                                                                       irmc);
> > +
> > +       if (err)
> > +               *err = 0;
> > +
> > +       return irmc;
> > +}
> > +
> > +static int irmc_get(struct obex_session *os, obex_object_t *obj,
> > +               gboolean *stream, void *user_data)
> > +{
> > +       struct irmc_session *irmc = user_data;
> > +       const char *type = obex_get_type(os);
> > +       const char *name = obex_get_name(os);
> > +       char *path;
> > +       int ret;
> > +
> > +       DBG("name %s type %s irmc %p", name, type ? type : "NA", irmc);
> > +
> > +       path = g_strdup(name);
> > +       *stream = TRUE;
> > +
> > +       ret = obex_get_stream_start(os, path);
> > +
> > +       g_free(path);
> > +
> > +       return ret;
> > +}
> > +
> > +static void irmc_disconnect(struct obex_session *os, void *user_data)
> > +{
> > +       struct irmc_session *irmc = user_data;
> > +
> > +       DBG("");
> > +
> > +       manager_unregister_session(os);
> > +
> > +        if (irmc->params) {
> > +                if (irmc->params->searchval)
> > +                       g_free(irmc->params->searchval);
> > +                g_free(irmc->params);
> > +        }
> > +       if (irmc->buffer) {
> > +               string_free(irmc->buffer);
> > +               irmc->buffer = NULL;
> > +       }
> > +
> > +       g_free(irmc);
> > +}
> > +
> > +static int irmc_chkput(struct obex_session *os, void *user_data)
> > +{
> > +       DBG("");
> > +       /* Reject all PUTs */
> > +       return -EBADR;
> > +}
> > +
> > +static void *irmc_open_devinfo(struct irmc_session *irmc, int *err)
> > +{
> > +       char mybuf[1024];
> > +
> > +       sprintf(mybuf, "MANU:%s\r\n"
> > +                       "MOD:%s\r\n"
> > +                       "SN:%s\r\n"
> > +                       "PB-TYPE-TX:VCARD2.1\r\n",
> > +                       irmc->manu, irmc->model, irmc->sn);
> > +
> > +       if (!irmc->buffer)
> > +               irmc->buffer = g_string_new(mybuf);
> > +       else
> > +               irmc->buffer = g_string_append(irmc->buffer, mybuf);
> > +
> > +       return irmc;
> > +}
> > +
> > +static void *irmc_open_pb(const char *name,
> > +                struct irmc_session *irmc, int *err)
> > +{
> > +       char mybuf[1024];
> > +       int ret;
> > +
> > +       if (!g_strcmp0(name, ".vcf")) {
> > +               // how can we tell if the vcard count call already finished?
> > +               ret = phonebook_pull("telecom/pb.vcf", irmc->params,
> > +                                                       query_result, irmc);
> > +               if (ret < 0) {
> > +                       DBG("phonebook_pull failed...");
> > +                       goto fail;
> > +               }
> > +               return irmc;
> > +       }
> > +
> > +       if (!g_strcmp0(name, "/info.log")) {
> > +               sprintf(mybuf, "Total-Records:%d\r\n"
> > +                               "Maximum-Records:%d\r\n"
> > +                               "DID:%s\r\n",
> > +                               irmc->params->maxlistcount,
> > +                               irmc->params->maxlistcount, irmc->did);
> > +       }
> > +       else if (!strncmp(name, "/luid/", 6)) {
> > +               name += 6;
> > +               if (!g_strcmp0(name, "cc.log"))
> > +                       sprintf(mybuf, "%d\r\n", irmc->params->maxlistcount);
> > +               else {
> > +                       int l = strlen(name);
> > +                       /* FIXME
> > +                        * reply the same to any *.log so we hopefully force a
> > +                        * full phonebook dump.
> > +                        * Is IEL:2 ok?
> > +                        */
> > +                       if (l > 4 && !g_strcmp0(name + l - 4, ".log")) {
> > +                               DBG("changelog request, force whole book");
> > +                               sprintf(mybuf, "SN:%s\r\n"
> > +                                               "IEL:2\r\n"
> > +                                               "DID:%s\r\n"
> > +                                               "Total-Records:%d\r\n"
> > +                                               "Maximum-Records:%d\r\n"
> > +                                               "*\r\n",
> > +                                       irmc->sn, irmc->did,
> > +                                       irmc->params->maxlistcount,
> > +                                       irmc->params->maxlistcount);
> > +                       }
> > +                       else {
> > +                               ret = -EBADR;
> > +                               goto fail;
> > +                       }
> > +               }
> > +       }
> > +       else {
> > +               ret = -EBADR;
> > +               goto fail;
> > +       }
> > +
> > +       if (!irmc->buffer)
> > +               irmc->buffer = g_string_new(mybuf);
> > +       else
> > +               irmc->buffer = g_string_append(irmc->buffer, mybuf);
> > +
> > +       return irmc;
> > +
> > +fail:
> > +       if (err)
> > +               *err = ret;
> > +
> > +       return NULL;
> > +}
> > +
> > +static void *irmc_open_cal(const char *name,
> > +                struct irmc_session *irmc, int *err)
> > +{
> > +       // no suport yet. Just return an empty buffer
> > +       //cal.vcs
> > +       DBG("unsupported, returning empty buffer");
> > +       if (!irmc->buffer)
> > +               irmc->buffer = g_string_new("");
> > +       else
> > +               irmc->buffer = g_string_append(irmc->buffer, "");
> > +
> > +       return irmc;
> > +}
> > +
> > +static void *irmc_open_nt(const char *name,
> > +                struct irmc_session *irmc, int *err)
> > +{
> > +       // no suport yet. Just return an empty buffer
> > +       //nt.vnt
> > +       DBG("unsupported, returning empty buffer");
> > +       if (!irmc->buffer)
> > +               irmc->buffer = g_string_new("");
> > +       else
> > +               irmc->buffer = g_string_append(irmc->buffer, "");
> > +
> > +       return irmc;
> > +}
> > +
> > +static void *irmc_open(const char *name, int oflag, mode_t mode,
> > +               void *context, size_t *size, int *err)
> > +{
> > +       struct irmc_session *irmc = context;
> > +       int ret;
> > +       const char *p;
> > +
> > +       DBG("name %s context %p", name, context);
> > +       if (oflag != O_RDONLY) {
> > +               ret = -EPERM;
> > +               goto fail;
> > +       }
> > +       if (strncmp(name, "telecom/", 8) != NULL) {
> > +               ret = -EBADR;
> > +               goto fail;
> > +       }
> > +
> > +       p = name + 8;
> > +       if (!g_strcmp0(p, "devinfo.txt"))
> > +               return irmc_open_devinfo(irmc, err);
> > +       else if (!strncmp(p, "pb", 2))
> > +               return irmc_open_pb(p+2, irmc, err);
> > +       else if (!strncmp(p, "cal", 3))
> > +               return irmc_open_cal(p+3, irmc, err);
> > +       else if (!strncmp(p, "nt", 2))
> > +               return irmc_open_nt(p+2, irmc, err);
> > +
> > +fail:
> > +       if (err)
> > +               *err = ret;
> > +
> > +       return NULL;
> > +}
> > +
> > +static int irmc_close(void *object)
> > +{
> > +       struct irmc_session *irmc = object;
> > +
> > +       DBG("");
> > +       if (irmc->buffer) {
> > +               string_free(irmc->buffer);
> > +               irmc->buffer = NULL;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static ssize_t irmc_read(void *object, void *buf, size_t count, uint8_t *hi)
> > +{
> > +       struct irmc_session *irmc = object;
> > +        int len;
> > +
> > +       DBG("buffer %p count %d", irmc->buffer, count);
> > +       if (!irmc->buffer)
> > +                return -EAGAIN;
> > +
> > +        *hi = OBEX_HDR_BODY;
> > +       len = string_read(irmc->buffer, buf, count);
> > +       DBG("returning %d bytes", len);
> > +       return len;
> > +}
> > +
> > +static struct obex_mime_type_driver irmc_driver = {
> > +       .target = IRMC_TARGET,
> > +       .target_size = IRMC_TARGET_SIZE,
> > +       .open = irmc_open,
> > +       .close = irmc_close,
> > +       .read = irmc_read,
> > +};
> > +
> > +static struct obex_service_driver irmc = {
> > +       .name = "IRMC Sync server",
> > +       .service = OBEX_IRMC,
> > +       .channel = IRMC_CHANNEL,
> > +       .record = IRMC_RECORD,
> > +       .target = IRMC_TARGET,
> > +       .target_size = IRMC_TARGET_SIZE,
> > +       .connect = irmc_connect,
> > +       .get = irmc_get,
> > +       .disconnect = irmc_disconnect,
> > +       .chkput = irmc_chkput
> > +};
> > +
> > +static int irmc_init(void)
> > +{
> > +       int err;
> > +
> > +       DBG("");
> > +       err = phonebook_init();
> > +       if (err < 0)
> > +               return err;
> > +
> > +       err = obex_mime_type_driver_register(&irmc_driver);
> > +       if (err < 0)
> > +               return err;
> > +
> > +       return obex_service_driver_register(&irmc);
> > +}
> > +
> > +static void irmc_exit(void)
> > +{
> > +       DBG("");
> > +       obex_service_driver_unregister(&irmc);
> > +       obex_mime_type_driver_unregister(&irmc_driver);
> > +       phonebook_exit();
> > +}
> > +
> > +OBEX_PLUGIN_DEFINE(irmc, irmc_init, irmc_exit)
> > diff --git a/src/main.c b/src/main.c
> > index 649acf9..96523a6 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -83,6 +83,7 @@ static gboolean option_pbap = FALSE;
> >  static gboolean option_pcsuite = FALSE;
> >  static gboolean option_symlinks = FALSE;
> >  static gboolean option_syncevolution = FALSE;
> > +static gboolean option_irmc = FALSE;
> >
> >  static gboolean parse_debug(const char *key, const char *value,
> >                                gpointer user_data, GError **error)
> > @@ -122,6 +123,8 @@ static GOptionEntry options[] = {
> >                                "Enable PC Suite Services server" },
> >        { "syncevolution", 'e', 0, G_OPTION_ARG_NONE, &option_syncevolution,
> >                                "Enable OBEX server for SyncEvolution" },
> > +       { "irmc", 'i', 0, G_OPTION_ARG_NONE, &option_irmc,
> > +                               "Enable IrMCSync server" },
> >        { NULL },
> >  };
> >
> > @@ -208,9 +211,10 @@ int main(int argc, char *argv[])
> >
> >        if (option_opp == FALSE && option_ftp == FALSE &&
> >                                option_pbap == FALSE &&
> > -                               option_syncevolution == FALSE) {
> > +                               option_syncevolution == FALSE &&
> > +                               option_irmc == FALSE) {
> >                fprintf(stderr, "No server selected (use either "
> > -                               "--opp, --ftp, --pbap or --syncevolution)\n");
> > +                               "--opp, --ftp, --pbap --syncevolution or --irmc)\n");
> >                exit(EXIT_FAILURE);
> >        }
> >
> > @@ -270,6 +274,10 @@ int main(int argc, char *argv[])
> >                obex_server_init(OBEX_SYNCEVOLUTION, NULL, TRUE, FALSE,
> >                                                        FALSE, NULL);
> >
> > +       if (option_irmc == TRUE)
> > +               obex_server_init(OBEX_IRMC, NULL, TRUE, FALSE, FALSE,
> > +                               option_capability);
> > +
> >        if (!root_folder_setup(option_root, option_root_setup)) {
> >                error("Unable to setup root folder %s", option_root);
> >                exit(EXIT_FAILURE);
> > diff --git a/src/obex.h b/src/obex.h
> > index 9424b6b..cfe9159 100644
> > --- a/src/obex.h
> > +++ b/src/obex.h
> > @@ -35,6 +35,7 @@
> >  #define OBEX_PBAP      (1 << 4)
> >  #define OBEX_PCSUITE   (1 << 5)
> >  #define OBEX_SYNCEVOLUTION     (1 << 6)
> > +#define OBEX_IRMC      (1 << 7)
> >
> >  #define TARGET_SIZE 16
> >
> > --
> > 1.7.2
> 
> Looks pretty good, but you need to fix those c++ comments (//) please
> use /*,

Fine. will do.

> also the you don't necessary need to abstract the file
> devinfo.txt so platform can just write their own stuff there, if that
> is not sufficient than we can thing of supporting scripts like we do
> with x-obex/capability but I don't think we really need it since the
> device info should be static, right?

abstracting it made de irmc_open() more clean (e.g. no buffer fills in
irmc_open()) and more readable in my opinion.
The specs for devinfo.txt contain many more attributes but the currrent
set seems to be enough. In future this may need to be extended and 
supporting scripts may be needed for getting the info. The current
values are actually not correct I think as the will be the same for any
device now... (hence the hint for using x-obex/capabilities in
irmc_connect().)

^ permalink raw reply

* Re: [PATCHv1 1/2] Bluetooth: Check sk is not owned before freeing l2cap_conn
From: Andrei Emeltchenko @ 2010-08-12 13:33 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1281612425.12579.253.camel@localhost.localdomain>

Hi Marcel,

On Thu, Aug 12, 2010 at 2:27 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Andrei,
>
>> >> Check that socket sk is not locked in user process before removing
>> >> l2cap connection handler.
>> >>
>> >> krfcommd kernel thread may be preempted with l2cap tasklet which remo=
ve
>> >> l2cap_conn structure. If krfcommd is in process of sending of RFCOMM =
reply
>> >> (like "RFCOMM UA" reply to "RFCOMM DISC") then kernel crash happens.
>> >>
>> >> ...
>> >> [ =A0694.175933] Unable to handle kernel NULL pointer dereference at =
virtual address 00000000
>> >> [ =A0694.184936] pgd =3D c0004000
>> >> [ =A0694.187683] [00000000] *pgd=3D00000000
>> >> [ =A0694.191711] Internal error: Oops: 5 [#1] PREEMPT
>> >> [ =A0694.196350] last sysfs file: /sys/devices/platform/hci_h4p/firmw=
are/hci_h4p/loading
>> >> [ =A0694.260375] CPU: 0 =A0 =A0Not tainted =A0(2.6.32.10 #1)
>> >> [ =A0694.265106] PC is at l2cap_sock_sendmsg+0x43c/0x73c [l2cap]
>> >> [ =A0694.270721] LR is at 0xd7017303
>> >> ...
>> >> [ =A0694.525085] Backtrace:
>> >> [ =A0694.527587] [<bf266be0>] (l2cap_sock_sendmsg+0x0/0x73c [l2cap]) =
from [<c02f2cc8>] (sock_sendmsg+0xb8/0xd8)
>> >> [ =A0694.537292] [<c02f2c10>] (sock_sendmsg+0x0/0xd8) from [<c02f3044=
>] (kernel_sendmsg+0x48/0x80)
>> >> ...
>> >>
>> >> Modified version after comments of Gustavo F. Padovan <gustavo@padova=
n.org>
>> >>
>> >> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>> >
>> > the patch seems to be fine, but I have some extra questions/concerns.
>> >
>> > Who is now taking care of deleting the channel in this case? I think y=
ou
>> > need to show that the code flow is still valid.
>>
>> I have the other version of the I have sent already to ML where I use
>> standard l2cap
>> timer which will delete channel like the code below:
>>
>> + =A0 =A0 =A0 /* don't delete l2cap channel if sk is owned by user */
>> + =A0 =A0 =A0 if (sock_owned_by_user(sk)) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 sk->sk_state =3D BT_DISCONN;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 l2cap_sock_clear_timer(sk);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 l2cap_sock_set_timer(sk, HZ);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bh_unlock_sock(sk);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0;
>> + =A0 =A0 =A0 }
>>
>> > Also the question is how RFCOMM can send this UA or DISC with not
>> > locking the socket. The comment on l2cap_chan_del clearly states that
>> > the socket must be locked and inside L2CAP we do that. Is RFCOMM maybe
>> > at fault here?
>>
>> when RFCOMM send packets it lock_sock which marks sk as owned
>> sk->sk_lock.owned =3D 1;
>> and then can be preempted.
>
> I need a new patch with a proper and most likely lengthy commit message
> explaining every single detail here. Since right now you lost me.

I am sending other version of the patch with l2cap timer which clears
L2CAP channels.
This way we are sure that channel will be deleted despite the
technique does not look
perfect...
BTW: The crash is easy to reproduce on ARM with our test tools :-)

Regards,
Andrei

^ permalink raw reply

* [PATCHv2 2/2] Bluetooth: timer check sk is not owned before freeing
From: Emeltchenko Andrei @ 2010-08-12 13:25 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1281619537-30096-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

In timer context we might delete l2cap channel used by krfcommd.
The check makes sure that sk is not owned.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
 net/bluetooth/l2cap.c |   32 ++++++++++++++++++++------------
 1 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 0221d05..2f8ac5d 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -73,6 +73,18 @@ static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn,
 				u8 code, u8 ident, u16 dlen, void *data);
 
 /* ---- L2CAP timers ---- */
+static void l2cap_sock_set_timer(struct sock *sk, long timeout)
+{
+	BT_DBG("sk %p state %d timeout %ld", sk, sk->sk_state, timeout);
+	sk_reset_timer(sk, &sk->sk_timer, jiffies + timeout);
+}
+
+static void l2cap_sock_clear_timer(struct sock *sk)
+{
+	BT_DBG("sock %p state %d", sk, sk->sk_state);
+	sk_stop_timer(sk, &sk->sk_timer);
+}
+
 static void l2cap_sock_timeout(unsigned long arg)
 {
 	struct sock *sk = (struct sock *) arg;
@@ -82,6 +94,14 @@ static void l2cap_sock_timeout(unsigned long arg)
 
 	bh_lock_sock(sk);
 
+	if (sock_owned_by_user(sk)) {
+		/* sk is owned by user. Try again later */
+		l2cap_sock_set_timer(sk, HZ * 2);
+		bh_unlock_sock(sk);
+		sock_put(sk);
+		return;
+	}
+
 	if (sk->sk_state == BT_CONNECTED || sk->sk_state == BT_CONFIG)
 		reason = ECONNREFUSED;
 	else if (sk->sk_state == BT_CONNECT &&
@@ -98,18 +118,6 @@ static void l2cap_sock_timeout(unsigned long arg)
 	sock_put(sk);
 }
 
-static void l2cap_sock_set_timer(struct sock *sk, long timeout)
-{
-	BT_DBG("sk %p state %d timeout %ld", sk, sk->sk_state, timeout);
-	sk_reset_timer(sk, &sk->sk_timer, jiffies + timeout);
-}
-
-static void l2cap_sock_clear_timer(struct sock *sk)
-{
-	BT_DBG("sock %p state %d", sk, sk->sk_state);
-	sk_stop_timer(sk, &sk->sk_timer);
-}
-
 /* ---- L2CAP channels ---- */
 static struct sock *__l2cap_get_chan_by_dcid(struct l2cap_chan_list *l, u16 cid)
 {
-- 
1.7.0.4


^ permalink raw reply related

* [PATCHv2 1/2] Bluetooth: Check sk is not owned before freeing l2cap_conn
From: Emeltchenko Andrei @ 2010-08-12 13:25 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1281619537-30096-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

Check that socket sk is not locked in user process before removing
l2cap connection handler.

krfcommd kernel thread may be preempted with l2cap tasklet which remove
l2cap_conn structure. If krfcommd is in process of sending of RFCOMM reply
(like "RFCOMM UA" reply to "RFCOMM DISC") then kernel crash happens.

...
[  694.175933] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[  694.184936] pgd = c0004000
[  694.187683] [00000000] *pgd=00000000
[  694.191711] Internal error: Oops: 5 [#1] PREEMPT
[  694.196350] last sysfs file: /sys/devices/platform/hci_h4p/firmware/hci_h4p/loading
[  694.260375] CPU: 0    Not tainted  (2.6.32.10 #1)
[  694.265106] PC is at l2cap_sock_sendmsg+0x43c/0x73c [l2cap]
[  694.270721] LR is at 0xd7017303
...
[  694.525085] Backtrace:
[  694.527587] [<bf266be0>] (l2cap_sock_sendmsg+0x0/0x73c [l2cap]) from [<c02f2cc8>] (sock_sendmsg+0xb8/0xd8)
[  694.537292] [<c02f2c10>] (sock_sendmsg+0x0/0xd8) from [<c02f3044>] (kernel_sendmsg+0x48/0x80)

Modified version after comments of Gustavo F. Padovan <gustavo@padovan.org>

Fixes: NB#164721 - Device reboots while executing OBEX FTP tests

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
 net/bluetooth/l2cap.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 1874ece..0221d05 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2708,6 +2708,13 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd
 		break;
 
 	default:
+		/* don't delete l2cap channel if sk is owned by user */
+		if (sock_owned_by_user(sk)) {
+			sk->sk_state = BT_DISCONN;
+			l2cap_sock_clear_timer(sk);
+			l2cap_sock_set_timer(sk, HZ);
+			break;
+		}
 		l2cap_chan_del(sk, ECONNREFUSED);
 		break;
 	}
@@ -2914,6 +2921,15 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd
 		del_timer(&l2cap_pi(sk)->monitor_timer);
 	}
 
+	/* don't delete l2cap channel if sk is owned by user */
+	if (sock_owned_by_user(sk)) {
+		sk->sk_state = BT_DISCONN;
+		l2cap_sock_clear_timer(sk);
+		l2cap_sock_set_timer(sk, HZ);
+		bh_unlock_sock(sk);
+		return 0;
+	}
+
 	l2cap_chan_del(sk, ECONNRESET);
 	bh_unlock_sock(sk);
 
@@ -2944,6 +2960,15 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
 		del_timer(&l2cap_pi(sk)->monitor_timer);
 	}
 
+	/* don't delete l2cap channel if sk is owned by user */
+	if (sock_owned_by_user(sk)) {
+		sk->sk_state = BT_DISCONN;
+		l2cap_sock_clear_timer(sk);
+		l2cap_sock_set_timer(sk, HZ);
+		bh_unlock_sock(sk);
+		return 0;
+	}
+
 	l2cap_chan_del(sk, 0);
 	bh_unlock_sock(sk);
 
-- 
1.7.0.4


^ permalink raw reply related

* [PATCHv2 0/2] Fix kernel crash in rfcomm/l2cap
From: Emeltchenko Andrei @ 2010-08-12 13:25 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

Version 2 of patches fixing kernel crash in RFCOMM / L2CAP.

Do not delete l2cap channel and socket sk when sk is owned by user.
To delete l2cap channel standard timer is used.

lock_sock and release_sock do not hold a normal spinlock directly but 
instead hold the owner field. This means bh_lock_sock can still execute
even if the socket is "locked". More info can be found here:
http://www.linuxfoundation.org/collaborate/workgroups/networking/socketlocks

Andrei Emeltchenko (2):
  Bluetooth: Check sk is not owned before freeing l2cap_conn
  Bluetooth: timer check sk is not owned before freeing

 net/bluetooth/l2cap.c |   57 ++++++++++++++++++++++++++++++++++++++----------
 1 files changed, 45 insertions(+), 12 deletions(-)


^ permalink raw reply

* Question about phonebook_add_contact
From: Radoslaw Jablonski @ 2010-08-12 13:13 UTC (permalink / raw)
  To: linux-bluetooth@vger.kernel.org


  Hi!
I've got question about this part of code from beginning  
phonebook_add_contact func:

if ((contact->numbers == NULL && (contact->fullname == NULL ||
                         contact->fullname[0] == '\0')))
         return;

Is this checking really needed in that place? It is possible to have 
contacts data
in phonebook that have all data except phone set (by example: email 
addresses,
main address etc). In that condition when we want to pull target vcard 
entry, then
empty string will be returned even if user send ATTRIBUTE_MASK to 
include all fields
in the result.

I'm asking because I have problem with "0.vcf" vcard (this is 
responsible for personal
information of device owner). This entry by default does not have any 
phone's data
filled in address book but it is mandatory to put this in vcard-listing.
Actually, I am forced to initialize contact->numbers list with empty 
number even if
none of number is available to return correct result for "0.vcf."(in 
phonebook-tracker)...

Can this 'contact->numbers' checking be removed from there or maybe 
other phonebooks
rely on this behaviour?

BR,
Radek



^ permalink raw reply

* [PATCH] Changes in HDP API.
From: Jose Antonio Santos Cadenas @ 2010-08-12 13:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas

Now the connection is made directly to a device and bluetoothd daemon
guess the PSM connection and the MDEPID based on the data get from the
SDP record.

Also a property that indicates the MainChannel is added to the
HealthDevice object.
---
 doc/health-api.txt |  119 +++++++++++++++++++++++-----------------------------
 1 files changed, 53 insertions(+), 66 deletions(-)

diff --git a/doc/health-api.txt b/doc/health-api.txt
index f469df3..0a7ae73 100644
--- a/doc/health-api.txt
+++ b/doc/health-api.txt
@@ -14,12 +14,9 @@ Object path	/org/bluez/
 
 Methods:
 
-	object	CreateApplication(dict config, object agent)
+	object	CreateApplication(dict config)
 
-		Returns the path of the new registered application. The agent
-		parameter is the path of the object with the callbacks to
-		notify events (see org.bluez.HealthAgent at the end
-		of this document).
+		Returns the path of the new registered application.
 
 		Dict is defined as bellow:
 		{
@@ -39,32 +36,33 @@ Methods:
 
 		Closes the HDP application identified by the object path. Also
 		application will be closed if the process that started it leaves
-		the bus.
+		the bus. Only the creator of the application will be able to
+		destroy it.
 
 		Possible errors: org.bluez.Error.InvalidArguments
 				org.bluez.Error.NotFound
+				org.bluez.Error.NotAllowed
 
 --------------------------------------------------------------------------------
 
 Service		org.bluez
-Interface	org.bluez.HealthApplication
-Object path	[variable prefix]/health_app_ZZZZ
+Interface	org.bluez.HealthDevice
+Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
 
 Methods:
 
-	void Echo(object service)
+	Boolean Echo()
 
 		Sends an echo petition to the remote service. Returns True if
 		response matches with the buffer sent. If some error is detected
-		False value is returned and the associated MCL is closed.
+		False value is returned.
 
 		Possible errors: org.bluez.Error.InvalidArguments
 				org.bluez.Error.OutOfRange
 
-	object CreateChannel(object service, string type)
+	object CreateChannel(object application, string configuration)
 
-		Creates a new data channel with the indicated config to the
-		remote Service.
+		Creates a new data channel.
 		The configuration should indicate the channel quality of
 		service using one of this values "Reliable", "Streaming", "Any".
 
@@ -76,16 +74,39 @@ Methods:
 
 	void DestroyChannel(object channel)
 
-		Destroys the data channel object.
+		Destroys the data channel object. Only the creator of the
+		channel or the creator of the HealtApplication that received the
+		data channel will be able to destroy it
 
 		Possible errors: org.bluez.Error.InvalidArguments
-				orb.bluez.Error.NotFound
+				org.bluez.Error.NotFound
+				org.bluez.Error.NotAllowed
 
---------------------------------------------------------------------------------
+Signals:
 
-Service		org.bluez
-Interface	org.bluez.HealthService
-Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
+	void ChannelConnected(object channel)
+
+		This signal is launched when a new data channel is created or
+		when a known data channel is reconnected.
+
+	void ChannelDeleted(object channel)
+
+		This signal is launched when a data channel is deleted.
+
+		After this signal the data channel path will not be valid and
+		its path can be reused for future data channels.
+
+	void PropertyChanged(string name, variant value)
+
+		This signal indicates a changed value of the given property.
+
+Properties:
+
+	object MainChannel [readonly]
+
+		The first reliable channel opened. It is needed by upper
+		applications in order to send specific protocol data units. The
+		first reliable can change after a reconnection.
 
 --------------------------------------------------------------------------------
 
@@ -94,6 +115,9 @@ Interface	org.bluez.HealthChannel
 Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/
 							hdp_YYYY/channel_ZZ
 
+Only the process that created the data channel or the creator of the
+HealthApplication that received it will be able to call this methods.
+
 Methods:
 
 	dict GetProperties()
@@ -101,68 +125,31 @@ Methods:
 		Returns all properties for the interface. See the properties
 		section for available properties.
 
+		Posible errors: org.bluez.Error.NotAllowed
+
 	fd Acquire()
 
 		Returns the file descriptor for this data channel. If the data
 		channel is not connected it will also reconnect.
 
 		Possible errors: org.bluez.Error.NotConnected
+				org.bluez.Error.NotAllowed
 
 	void Release()
 
 		Releases the fd. Application should also need to close() it.
 
+		Possible errors: org.bluez.Error.NotAcquired
+				org.bluez.Error.NotAllowed
+
 Properties:
 
-	string Type (read only)
+	string Type [readonly]
 
 		The quality of service of the data channel. ("Reliable" or
 		"Streaming")
 
-	object Service (read only)
-
-		Identifies the Remote Service that is connected with. Maps with
-		a HealthService object.
-
-HealthAgent hierarchy
-=====================
-
-(this object is implemented by the HDP user in order to receive notifications)
-
-Service		unique name
-Interface	org.bluez.HealthAgent
-Object path	freely definable
-
-Methods:
-
-	void Release()
-
-		This method gets called when the service daemon unregisters the
-		agent. An agent can use it to do cleanup tasks. There is no need
-		to unregister the agent, because when this method gets called it
-		has already been unregistered.
-
-	void ServiceDiscovered(object service)
-
-		This method is called when a device containing an HDP
-		application is paired or when the method Update of the
-		HealthManager is called and new HealthServices are discovered.
-		The method will be called once for each HealthService.
-
-	void ServiceRemoved(object service)
-
-		This is called if during an Update some HealthServices
-		have disappeared. The method is called once for each removed
-		HealthService.
-
-	void ChannelConnected(object channel)
-
-		This method is called when a new data channel is created or when
-		a known data channel is reconnected.
-
-	void ChannelDeleted(object channel)
-
-		This method is called when a data channel is deleted.
+	object Device [readonly]
 
-		After this call the data channel path will not be valid and can
-		be reused for future creation of data channels.
+		Identifies the Remote Device that is connected with. Maps with
+		a HealthDevice object.
-- 
1.7.0.4


^ permalink raw reply related

* Re: [PATCHv2] Bluetooth: new sockopt to enter active state when sending data
From: Marcel Holtmann @ 2010-08-12 13:07 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <AANLkTinp-wXzgUU3dSNteSGdBmgtcB8S+sWZZ-9b0BnD@mail.gmail.com>

Hi Andrei,

> >> Patch adds new socket option to enter active state when sending
> >> data. Modified version of Fabien Chevalier patch (Sep/2008).
> >> Discussions:
> >> http://www.spinics.net/lists/bluez-devel/msg00567.html
> >> http://www.spinics.net/lists/linux-bluetooth/msg03765.html
> >>
> >> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> >> ---
> >>  include/net/bluetooth/hci_core.h |    1 +
> >>  include/net/bluetooth/l2cap.h    |    2 ++
> >>  net/bluetooth/hci_conn.c         |    7 ++++++-
> >>  net/bluetooth/l2cap.c            |   15 +++++++++++++++
> >>  4 files changed, 24 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> >> index e42f6ed..477a492 100644
> >> --- a/include/net/bluetooth/hci_core.h
> >> +++ b/include/net/bluetooth/hci_core.h
> >> @@ -178,6 +178,7 @@ struct hci_conn {
> >>       __u8             auth_type;
> >>       __u8             sec_level;
> >>       __u8             power_save;
> >> +     __u8             force_active;
> >>       __u16            disc_timeout;
> >>       unsigned long    pend;
> >>
> >> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> >> index 7c695bf..540cca7 100644
> >> --- a/include/net/bluetooth/l2cap.h
> >> +++ b/include/net/bluetooth/l2cap.h
> >> @@ -74,6 +74,8 @@ struct l2cap_conninfo {
> >>  #define L2CAP_LM_RELIABLE    0x0010
> >>  #define L2CAP_LM_SECURE              0x0020
> >>
> >> +#define L2CAP_FORCE_ACTIVE_MODE      0x04
> >> +
> >>  /* L2CAP command codes */
> >>  #define L2CAP_COMMAND_REJ    0x01
> >>  #define L2CAP_CONN_REQ               0x02
> >> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> >> index 800b6b9..11fc44a 100644
> >> --- a/net/bluetooth/hci_conn.c
> >> +++ b/net/bluetooth/hci_conn.c
> >> @@ -214,6 +214,8 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
> >>       conn->auth_type = HCI_AT_GENERAL_BONDING;
> >>
> >>       conn->power_save = 1;
> >> +     /* Do not enter active state by default */
> >> +     conn->force_active = 0;
> >>       conn->disc_timeout = HCI_DISCONN_TIMEOUT;
> >>
> >>       switch (type) {
> >> @@ -505,7 +507,10 @@ void hci_conn_enter_active_mode(struct hci_conn *conn)
> >>       if (test_bit(HCI_RAW, &hdev->flags))
> >>               return;
> >>
> >> -     if (conn->mode != HCI_CM_SNIFF || !conn->power_save)
> >> +     if (conn->mode != HCI_CM_SNIFF)
> >> +             goto timer;
> >> +
> >> +     if (!conn->power_save && !conn->force_active)
> >>               goto timer;
> >>
> >>       if (!test_and_set_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend)) {
> >> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> >> index cf3c407..5328e82 100644
> >> --- a/net/bluetooth/l2cap.c
> >> +++ b/net/bluetooth/l2cap.c
> >> @@ -1909,6 +1909,15 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us
> >>               l2cap_pi(sk)->force_reliable = (opt & L2CAP_LM_RELIABLE);
> >>               break;
> >>
> >> +     case L2CAP_FORCE_ACTIVE_MODE:
> >> +             if (get_user(opt, (u32 __user *) optval)) {
> >> +                     err = -EFAULT;
> >> +                     break;
> >> +             }
> >> +
> >> +             l2cap_pi(sk)->conn->hcon->force_active = opt;
> >> +             break;
> >> +
> >>       default:
> >>               err = -ENOPROTOOPT;
> >>               break;
> >> @@ -2058,6 +2067,12 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname, char __us
> >>
> >>               break;
> >>
> >> +     case L2CAP_FORCE_ACTIVE_MODE:
> >> +             if (put_user(l2cap_pi(sk)->conn->hcon->force_active,
> >> +                                     (u32 __user *) optval))
> >> +                     err = -EFAULT;
> >> +             break;
> >> +
> >>       default:
> >>               err = -ENOPROTOOPT;
> >>               break;
> >
> > I think that I said this before. Why do you think that adding a new
> > socket option to something called _old is a good idea? I made it pretty
> > clear in the code that it is an OLD interface.
> >
> > I want this on the SOL_BLUETOOTH level with a proper new socket option.
> > And we might even consider doing it like BT_SECURITY where create the
> > struct to potentially add other flags later.
> 
> This can be done.
> 
> > So why not doing BT_POWER or something like that.
> >
> > And in addition you need to store these values on a per L2CAP or RFCOMM
> > socket. You can not just go ahead and mess inside hci_conn via an L2CAP
> > socket option.
> 
> I felt that Sniff Mode is a property of ACL link and not L2CAP or RFCOMM socket.
> 
> There is already code in the same functions which use the same
> technique to access
> following hci_conn fields:
> 
> l2cap_pi(sk)->conn->hcon->handle
> l2cap_pi(sk)->conn->hcon->dev_class

that is for pure read-only access. If multiple L2CAP sockets are open
you have to be smart about the policy you put onto your ACL link.

Regards

Marcel



^ permalink raw reply

* Re: [PATCHv2] Bluetooth: new sockopt to enter active state when sending data
From: Andrei Emeltchenko @ 2010-08-12 12:25 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1281612715.12579.257.camel@localhost.localdomain>

Hi Marcel,

On Thu, Aug 12, 2010 at 2:31 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Andrei,
>
>> Patch adds new socket option to enter active state when sending
>> data. Modified version of Fabien Chevalier patch (Sep/2008).
>> Discussions:
>> http://www.spinics.net/lists/bluez-devel/msg00567.html
>> http://www.spinics.net/lists/linux-bluetooth/msg03765.html
>>
>> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>> ---
>> =A0include/net/bluetooth/hci_core.h | =A0 =A01 +
>> =A0include/net/bluetooth/l2cap.h =A0 =A0| =A0 =A02 ++
>> =A0net/bluetooth/hci_conn.c =A0 =A0 =A0 =A0 | =A0 =A07 ++++++-
>> =A0net/bluetooth/l2cap.c =A0 =A0 =A0 =A0 =A0 =A0| =A0 15 +++++++++++++++
>> =A04 files changed, 24 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hc=
i_core.h
>> index e42f6ed..477a492 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -178,6 +178,7 @@ struct hci_conn {
>> =A0 =A0 =A0 __u8 =A0 =A0 =A0 =A0 =A0 =A0 auth_type;
>> =A0 =A0 =A0 __u8 =A0 =A0 =A0 =A0 =A0 =A0 sec_level;
>> =A0 =A0 =A0 __u8 =A0 =A0 =A0 =A0 =A0 =A0 power_save;
>> + =A0 =A0 __u8 =A0 =A0 =A0 =A0 =A0 =A0 force_active;
>> =A0 =A0 =A0 __u16 =A0 =A0 =A0 =A0 =A0 =A0disc_timeout;
>> =A0 =A0 =A0 unsigned long =A0 =A0pend;
>>
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap=
.h
>> index 7c695bf..540cca7 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -74,6 +74,8 @@ struct l2cap_conninfo {
>> =A0#define L2CAP_LM_RELIABLE =A0 =A00x0010
>> =A0#define L2CAP_LM_SECURE =A0 =A0 =A0 =A0 =A0 =A0 =A00x0020
>>
>> +#define L2CAP_FORCE_ACTIVE_MODE =A0 =A0 =A00x04
>> +
>> =A0/* L2CAP command codes */
>> =A0#define L2CAP_COMMAND_REJ =A0 =A00x01
>> =A0#define L2CAP_CONN_REQ =A0 =A0 =A0 =A0 =A0 =A0 =A0 0x02
>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> index 800b6b9..11fc44a 100644
>> --- a/net/bluetooth/hci_conn.c
>> +++ b/net/bluetooth/hci_conn.c
>> @@ -214,6 +214,8 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, =
int type, bdaddr_t *dst)
>> =A0 =A0 =A0 conn->auth_type =3D HCI_AT_GENERAL_BONDING;
>>
>> =A0 =A0 =A0 conn->power_save =3D 1;
>> + =A0 =A0 /* Do not enter active state by default */
>> + =A0 =A0 conn->force_active =3D 0;
>> =A0 =A0 =A0 conn->disc_timeout =3D HCI_DISCONN_TIMEOUT;
>>
>> =A0 =A0 =A0 switch (type) {
>> @@ -505,7 +507,10 @@ void hci_conn_enter_active_mode(struct hci_conn *co=
nn)
>> =A0 =A0 =A0 if (test_bit(HCI_RAW, &hdev->flags))
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
>>
>> - =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF || !conn->power_save)
>> + =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF)
>> + =A0 =A0 =A0 =A0 =A0 =A0 goto timer;
>> +
>> + =A0 =A0 if (!conn->power_save && !conn->force_active)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto timer;
>>
>> =A0 =A0 =A0 if (!test_and_set_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend=
)) {
>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>> index cf3c407..5328e82 100644
>> --- a/net/bluetooth/l2cap.c
>> +++ b/net/bluetooth/l2cap.c
>> @@ -1909,6 +1909,15 @@ static int l2cap_sock_setsockopt_old(struct socke=
t *sock, int optname, char __us
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 l2cap_pi(sk)->force_reliable =3D (opt & L2CA=
P_LM_RELIABLE);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>>
>> + =A0 =A0 case L2CAP_FORCE_ACTIVE_MODE:
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (get_user(opt, (u32 __user *) optval)) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D -EFAULT;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>> + =A0 =A0 =A0 =A0 =A0 =A0 }
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 l2cap_pi(sk)->conn->hcon->force_active =3D opt=
;
>> + =A0 =A0 =A0 =A0 =A0 =A0 break;
>> +
>> =A0 =A0 =A0 default:
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D -ENOPROTOOPT;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>> @@ -2058,6 +2067,12 @@ static int l2cap_sock_getsockopt_old(struct socke=
t *sock, int optname, char __us
>>
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>>
>> + =A0 =A0 case L2CAP_FORCE_ACTIVE_MODE:
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (put_user(l2cap_pi(sk)->conn->hcon->force_a=
ctive,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 (u32 __user *) optval))
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D -EFAULT;
>> + =A0 =A0 =A0 =A0 =A0 =A0 break;
>> +
>> =A0 =A0 =A0 default:
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D -ENOPROTOOPT;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>
> I think that I said this before. Why do you think that adding a new
> socket option to something called _old is a good idea? I made it pretty
> clear in the code that it is an OLD interface.
>
> I want this on the SOL_BLUETOOTH level with a proper new socket option.
> And we might even consider doing it like BT_SECURITY where create the
> struct to potentially add other flags later.

This can be done.

> So why not doing BT_POWER or something like that.
>
> And in addition you need to store these values on a per L2CAP or RFCOMM
> socket. You can not just go ahead and mess inside hci_conn via an L2CAP
> socket option.

I felt that Sniff Mode is a property of ACL link and not L2CAP or RFCOMM so=
cket.

There is already code in the same functions which use the same
technique to access
following hci_conn fields:

l2cap_pi(sk)->conn->hcon->handle
l2cap_pi(sk)->conn->hcon->dev_class

Regards,
Andrei

^ permalink raw reply

* Re: resending patch to retry reset in case it failed
From: Oliver Neukum @ 2010-08-12 12:16 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth, linux-usb
In-Reply-To: <1281612323.12579.252.camel@localhost.localdomain>

Am Donnerstag, 12. August 2010, 13:25:23 schrieb Marcel Holtmann:
> Hi Oliver,
> 
> > may I ask what happened to this patch?
> > Do you take it, want modifications or reject it?
> > 
> > 	Regards
> > 		Oliver
> > 
> > From af61f0663f095599003290b13ad6fadee1d4fb25 Mon Sep 17 00:00:00 2001
> > From: Oliver Neukum <oliver@neukum.org>
> > Date: Tue, 6 Jul 2010 08:08:42 +0200
> > Subject: [PATCH] bluetooth: retry reset for devices that fail
> > 
> > Some devices fail to reset properly at the first attempt to reset
> > them under unknown circumstances. Failures can be identified by
> > an invalid btaddr. Retry in those cases.
> 
> I was pondering about it and haven't really made up my mind with this
> stuff. To be honest, I don't like the patch a little bit. I can see your
> point why you want it, but the patch is a bit too ugly for my taste.

I am very open to alternatives. But without that it is needed to make the
hardware work.

> Tell me which device is causing this and I might be able to reproduce
> this and find small less ugly fix.


I happens with this hardware from Broadcom. There may be others I don't
know about. It is an internal dongle. I don't know whether it can be found in
external hardware.

	Regards
		Oliver

Bus 002 Device 002: ID 0a5c:21b4 Broadcom Corp. 
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass          224 Wireless
  bDeviceSubClass         1 Radio Frequency
  bDeviceProtocol         1 Bluetooth
  bMaxPacketSize0        64
  idVendor           0x0a5c Broadcom Corp.
  idProduct          0x21b4 
  bcdDevice            4.81
  iManufacturer           1 
  iProduct                2 
  iSerial                 3 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength          216
    bNumInterfaces          4
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0xe0
      Self Powered
      Remote Wakeup
    MaxPower                0mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           3
      bInterfaceClass       224 Wireless
      bInterfaceSubClass      1 Radio Frequency
      bInterfaceProtocol      1 Bluetooth
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0010  1x 16 bytes
        bInterval               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x02  EP 2 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass       224 Wireless
      bInterfaceSubClass      1 Radio Frequency
      bInterfaceProtocol      1 Bluetooth
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            1
          Transfer Type            Isochronous
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0000  1x 0 bytes
        bInterval               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x03  EP 3 OUT
        bmAttributes            1
          Transfer Type            Isochronous
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0000  1x 0 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       1
      bNumEndpoints           2
      bInterfaceClass       224 Wireless
      bInterfaceSubClass      1 Radio Frequency
      bInterfaceProtocol      1 Bluetooth
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            1
          Transfer Type            Isochronous
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0009  1x 9 bytes
        bInterval               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x03  EP 3 OUT
        bmAttributes            1
          Transfer Type            Isochronous
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0009  1x 9 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       2
      bNumEndpoints           2
      bInterfaceClass       224 Wireless
      bInterfaceSubClass      1 Radio Frequency
      bInterfaceProtocol      1 Bluetooth
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            1
          Transfer Type            Isochronous
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0011  1x 17 bytes
        bInterval               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x03  EP 3 OUT
        bmAttributes            1
          Transfer Type            Isochronous
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0011  1x 17 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       3
      bNumEndpoints           2
      bInterfaceClass       224 Wireless
      bInterfaceSubClass      1 Radio Frequency
      bInterfaceProtocol      1 Bluetooth
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            1
          Transfer Type            Isochronous
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0020  1x 32 bytes
        bInterval               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x03  EP 3 OUT
        bmAttributes            1
          Transfer Type            Isochronous
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0020  1x 32 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       4
      bNumEndpoints           2
      bInterfaceClass       224 Wireless
      bInterfaceSubClass      1 Radio Frequency
      bInterfaceProtocol      1 Bluetooth
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            1
          Transfer Type            Isochronous
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x03  EP 3 OUT
        bmAttributes            1
          Transfer Type            Isochronous
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       5
      bNumEndpoints           2
      bInterfaceClass       224 Wireless
      bInterfaceSubClass      1 Radio Frequency
      bInterfaceProtocol      1 Bluetooth
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            1
          Transfer Type            Isochronous
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x03  EP 3 OUT
        bmAttributes            1
          Transfer Type            Isochronous
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        2
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass    255 Vendor Specific Subclass
      bInterfaceProtocol    255 Vendor Specific Protocol
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x84  EP 4 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0020  1x 32 bytes
        bInterval               1
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x04  EP 4 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0020  1x 32 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        3
      bAlternateSetting       0
      bNumEndpoints           0
      bInterfaceClass       254 Application Specific Interface
      bInterfaceSubClass      1 Device Firmware Update
      bInterfaceProtocol      1 
      iInterface              0 
      ** UNRECOGNIZED:  07 21 07 88 13 40 00

^ 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