* [PATCH] bluetooth: cmtp: fix information leak to userland
From: Vasiliy Kulikov @ 2010-10-30 14:26 UTC (permalink / raw)
To: kernel-janitors
Cc: Marcel Holtmann, Gustavo F. Padovan, David S. Miller,
Eric Dumazet, linux-bluetooth, netdev, linux-kernel
Structure cmtp_conninfo is copied to userland with some padding fields
unitialized. It leads to leaking of contents of kernel stack memory.
Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
---
Compile tested.
net/bluetooth/cmtp/core.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/cmtp/core.c b/net/bluetooth/cmtp/core.c
index ec0a134..8e5f292 100644
--- a/net/bluetooth/cmtp/core.c
+++ b/net/bluetooth/cmtp/core.c
@@ -78,6 +78,7 @@ static void __cmtp_unlink_session(struct cmtp_session *session)
static void __cmtp_copy_session(struct cmtp_session *session, struct cmtp_conninfo *ci)
{
+ memset(ci, 0, sizeof(*ci));
bacpy(&ci->bdaddr, &session->bdaddr);
ci->flags = session->flags;
--
1.7.0.4
^ permalink raw reply related
* [PATCH] bluetooth: hidp: fix information leak to userland
From: Vasiliy Kulikov @ 2010-10-30 14:26 UTC (permalink / raw)
To: kernel-janitors
Cc: Marcel Holtmann, Gustavo F. Padovan, David S. Miller, Jiri Kosina,
Michael Poole, Bastien Nocera, linux-bluetooth, netdev,
linux-kernel
Structure hidp_conninfo is copied to userland with version, product,
vendor and name fields unitialized if both session->input and session->hid
are NULL. It leads to leaking of contents of kernel stack memory.
Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
---
Compile tested.
net/bluetooth/hidp/core.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index c0ee8b3..29544c2 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -107,6 +107,7 @@ static void __hidp_unlink_session(struct hidp_session *session)
static void __hidp_copy_session(struct hidp_session *session, struct hidp_conninfo *ci)
{
+ memset(ci, 0, sizeof(*ci));
bacpy(&ci->bdaddr, &session->bdaddr);
ci->flags = session->flags;
@@ -115,7 +116,6 @@ static void __hidp_copy_session(struct hidp_session *session, struct hidp_connin
ci->vendor = 0x0000;
ci->product = 0x0000;
ci->version = 0x0000;
- memset(ci->name, 0, 128);
if (session->input) {
ci->vendor = session->input->id.vendor;
--
1.7.0.4
^ permalink raw reply related
* AVRCP 1.3/1.4 current implementation
From: David Stockwell @ 2010-10-30 17:57 UTC (permalink / raw)
To: João Paulo Rechi Vita, linux-bluetooth
Cc: Sander van Grieken, Shivendra Agrawal, Waldemar Rymarkiewicz,
Luiz Augusto von Dentz, Johan Hedberg
In-Reply-To: <AANLkTi=ThA2ojhj8PBHK_VybmFcHO91K0g=y7nhfTBAO@mail.gmail.com>
Hello to all,
In my earlier work, I noticed that in audio/manager.c, function
handle_uuids, when the UUID was found for an AVRCP target or controller, it
would only launch avrcp_connect IF a Sink was enabled and present.
I suspect this was done to make sure that if a headset or similar was
connecting, it would make sure the audio side of the connection was up and
running before enabling the control side of the interface.
However, my device has no audio side, it is a pure AVRCP CT, so there is no
audio-side connection. In previous testing, I just unconditionally started
avrcp_connect and it all worked fine for my purposes. I am wondering if
there is any other impact.
I could submit the patch in which avrcp_connect is unconditionally called.
Or, I could just wait until all of the uuids are processed (after the
g_slist_foreach in audio_probe), then launch avrcp_connect if either (a) a
sink is connected and enabled as at present, or (b) if no sink is called for
or provided by the CT/TG (as expressed in SDP), just launch avrcp_connect
without the sink. Are there any other impacts that this might cause?
To Joao, I would like to test my device against the GSOC work you did; can
you send me the URL? If you are farther along, no sense re-inventing the
wheel. Thanks/obrigado.
David Stockwell
^ permalink raw reply
* Re: AVRCP 1.3/1.4 current implementation
From: Sander van Grieken @ 2010-10-30 18:54 UTC (permalink / raw)
To: David Stockwell
Cc: João Paulo Rechi Vita, linux-bluetooth, Shivendra Agrawal,
Waldemar Rymarkiewicz, Luiz Augusto von Dentz, Johan Hedberg
In-Reply-To: <AF6253527A02449484D3BA2FF254AE2B@freqoneremote>
On Saturday 30 October 2010 19:57:31 David Stockwell wrote:
> Hello to all,
>
> In my earlier work, I noticed that in audio/manager.c, function
> handle_uuids, when the UUID was found for an AVRCP target or controller, it
> would only launch avrcp_connect IF a Sink was enabled and present.
>
> I suspect this was done to make sure that if a headset or similar was
> connecting, it would make sure the audio side of the connection was up and
> running before enabling the control side of the interface.
>
> However, my device has no audio side, it is a pure AVRCP CT, so there is no
> audio-side connection. In previous testing, I just unconditionally started
> avrcp_connect and it all worked fine for my purposes. I am wondering if
> there is any other impact.
Yes the current implementation only (implicitly) connects the RCP if a connection is made
to a DTP (audio) profile, or if a RCP connection request is received from the peer.
I have already implemented a DBUS method to connect only the RCP, but is only tested with
devices that also have the DTP profile, so this might not work if there's no DTP profile
detected on the peer. The code uses a struct audio_device * to pass along to the various
RCP functions, that might or might not be available when the peer has no DTP profile..
I think we can add the factoring out of the audio_device dependency to the TODO list.
Probably something along the lines of defining a struct control_device and have a pointer
to an optional audio_device struct in there.
I've made this stuff available in my git at github:
git://github.com/accumulator/bluez.git
grtz,
Sander
^ permalink raw reply
* Re: >net-wireless/bluez-4.63 unable to connect audio streams due commit
From: Pacho Ramos @ 2010-10-31 11:25 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Luiz Augusto von Dentz, Johan Hedberg, linux-bluetooth
In-Reply-To: <1287426290.6820.1.camel@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]
El lun, 18-10-2010 a las 20:24 +0200, Pacho Ramos escribió:
> El lun, 04-10-2010 a las 14:35 +0200, Uwe Kleine-König escribió:
> > Hello Pacho,
> >
> > On Mon, Oct 04, 2010 at 12:25:46PM +0200, Pacho Ramos wrote:
> > > > I would say this was because of double authentication request, but it
> > > > seems it is not the case, actually ssp doesn't seems to be used at all
> > > > here so this must be something else, maybe you should try this:
> > > >
> > > > http://thread.gmane.org/gmane.linux.bluez.kernel/7256
> > > >
> > >
> > > Thanks but, how should I try to apply that patch? Looks like
> > > net/bluetooth/rfcomm/core.c is not present on bluez-4.72 sources
> > I guess this is a patch to apply to your kernel, not bluez.
> >
> > Best regards
> > Uwe
> >
>
> Downstream affected reported told me it's still failing even with the
> patch:
>
> http://bugs.gentoo.org/show_bug.cgi?id=327705#c19
>
> Attached is the new hcidump output
>
> Thanks a lot for your help :-)
>
There is no possible solution to this? :-(
Thanks
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 5/9] mfd: Add UART support for the ST-Ericsson CG2900.
From: Alan Cox @ 2010-10-31 12:04 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Par-Gunnar Hjalmdahl, linus.walleij, linux-bluetooth,
linux-kernel, Lukasz.Rymanowski
In-Reply-To: <201010300201.01940.arnd@arndb.de>
> It's about the ldisc number. Both bluetooth and cg2900 register themselves
> to ldisc 15 (N_HCI). A user doing TIOCSETD can only get one of the two.
Ah I see - I had assumed any actual final merge would be assigning it a
new LDISC code as is required.
^ permalink raw reply
* Re: HFP Pulseaudio Source destroyed "too quickly" at the end of a call
From: Luiz Augusto von Dentz @ 2010-10-31 19:26 UTC (permalink / raw)
To: Peter Dons Tychsen; +Cc: Thomas Wälti, linux-bluetooth
In-Reply-To: <1288369169.4351.8.camel@donpedro>
Hi,
On Fri, Oct 29, 2010 at 7:19 PM, Peter Dons Tychsen <donpedro@tdcadsl.dk> wrote:
> On Wed, 2010-10-27 at 22:03 +0200, Thomas Wälti wrote:
>
>> All works well except when ending the recording of Bluetooth
>> Conversations: Once a party hangs up the call, the PulseAudio source
>> and sink disappear before I can stop GStreamer recording (I'm
>> listening to D-Bus events). Unfortunately, this causes my GStreamer
>> pipeline to crash.
Well then there is a problem in your pipeline, bluetooth sinks and
sources may disappear at any point and in fact PA do move audio,
depending on the system policy, when that occurs. Also note that
bluetooth modules do handle DisconnectRequest signal so it might react
a bit faster when user request audio to be disconnected, but that
should not make any difference for an application using PA.
> There are many reasons (especially for wireless device) that the source
> or sink could disappear at any time. Audio disappearing before or after
> the accompanying signaling furthermore a classic scenario which apps
> should be built to handle. So fixing the race (which i am not sure is
> really a race) does not seem like the correct solution. Trying to fix
> such "races" can lead to inefficiency by using unneeded timers and
> spin-locks, and will not provide robustness, as the audio streams can
> disappear for other reasons anyway.
100% agreed, arbitrary timers/timeouts are definable not robust and
iirc PA tries to be lock free.
^ permalink raw reply
* Re: [PATCH 1/2 v2] Bluetooth: Fix system crash caused by del_timer()
From: haijun liu @ 2010-11-01 1:22 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: Haijun Liu, linux-bluetooth
In-Reply-To: <20101028075346.GA15997@vigoh>
Hi Gustavo,
>> >> >> During test session with another vendor's bt stack, found that in
>> >> >> l2cap_chan_del() using del_timer() caused l2cap_monitor_timeout()
>> >> >> be called after the sock was freed, so it raised a system crash.
>> >> >> So I just replaced del_timer() with del_timer_sync() to solve it.
>> >> >
>> >> > NAK on this. If you read the del_timer_sync() documentation you can
>> >> > see that you can't call del_timer_sync() on interrupt context. The
>> >> > possible solution here is to check in the beginning of
>> >> > l2cap_monitor_timeout() if your sock is still valid.
>> >> >
>> >>
>> >> You are right, I only considered close() interface, so missed the interrupt
>> >> context.
>> >>
>> >> It's very difficult to check sock valid or not in timeout procedure, since it's
>> >> an interrupt context, and only can get context from parameter pre-stored,
>> >> except global variables.
>> >
>> > I think you can check for sk == null there.
>> >
>>
>> It's a pre-stored parameter, it will not change by itself.
>
> I looked a bit into this and a good solution seems to be to hold a
> reference to the sock when we call a mod_timer() and then put the
> reference when we call del_timer() and the timer is inactive or when
> l2cap_monitor_timeout(). Look net/sctp/ for examples.
>
Same situation still is there, in this case, l2cap_monitor_timeout()
how to know the reference has been released by del_timer()?
If we refer to net/sctp, use timer_pending() to detect it, unless we can
ensure del_timer() always be called in interrupt context and same
level compare to timer interrupt, otherwise it's not an atomic operation
for this case.
--
Haijun Liu
^ permalink raw reply
* Re: HFP Pulseaudio Source destroyed "too quickly" at the end of a call
From: Thomas Wälti @ 2010-11-01 8:11 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Peter Dons Tychsen, linux-bluetooth
In-Reply-To: <AANLkTi=BeeuAKjt8bKro2KH0SXMJDUuwMznRS5NcDj4K@mail.gmail.com>
2010/10/31 Luiz Augusto von Dentz <luiz.dentz@gmail.com>:
>> On Wed, 2010-10-27 at 22:03 +0200, Thomas Wälti wrote:
>>
>>> All works well except when ending the recording of Bluetooth
>>> Conversations: Once a party hangs up the call, the PulseAudio source
>>> and sink disappear before I can stop GStreamer recording (I'm
>>> listening to D-Bus events). Unfortunately, this causes my GStreamer
>>> pipeline to crash.
>
> Well then there is a problem in your pipeline, bluetooth sinks and
> sources may disappear at any point and in fact PA do move audio,
> depending on the system policy, when that occurs. Also note that
> bluetooth modules do handle DisconnectRequest signal so it might react
> a bit faster when user request audio to be disconnected, but that
> should not make any difference for an application using PA.
Thanks everybody for the helpful comments and valuable details,
helping me to understand how the "underworld" of my app works in the
Bluez/PA corner.
I'll now shift my focus towards GStreamer :-)
Best regards
-Tom
^ permalink raw reply
* Re: [PATCH 2/6] Bluetooth: fix receiving L2CAP packets over LE
From: Ville Tervo @ 2010-11-01 8:51 UTC (permalink / raw)
To: ext Gustavo F. Padovan
Cc: Vinicius Gomes, ext Anderson Briglia,
linux-bluetooth@vger.kernel.org
In-Reply-To: <20101029205033.GB14961@vigoh>
Hi,
On Fri, Oct 29, 2010 at 10:50:33PM +0200, ext Gustavo F. Padovan wrote:
> Hi,
>
> * Vinicius Gomes <vinicius.gomes@openbossa.org> [2010-10-29 09:41:55 -0400]:
>
> > Hi Ville,
> >
> > On Fri, Oct 29, 2010 at 6:44 AM, Ville Tervo <ville.tervo@nokia.com> wrote:
> > > Hi Anderson,
> > >
> > > On Sat, Oct 23, 2010 at 01:56:56AM +0200, ext Anderson Briglia wrote:
> > >> From: Vinicius Costa Gomes <vinicius.gomes@openbossa.org>
> > >>
> > >> As L2CAP packets coming over LE don't have any more encapsulation,
> > >> other than L2CAP, we are able to process them as soon as they arrive.
> > >
> > > Why is this change needed? Was something broken without this patch?
> > >
> >
> > This change is needed because without it the receiving side would
> > always think that it was receiving continuation frames.
> >
> > As the flags parameter is zero, it would fall into the "} else {"
> > condition, and because no frame was received before, conn->rx_len
> > would be zero and the frame would be discarded. Without this patch I
> > was seeing those "Unexpected continuation frame ..." messages on the
> > receiving side.
>
> From what I understood the flags are only for ACL connections and LE
> links doesn't have fragmentation, right? So we don't need to check flags
> here, actually it seems we don't have flags for LE like ACL.
Yes it has. See spec. Vol 2 Part E 5.4.2.
It seems that controller should not be sending 0 PB flag in any situation
(except loopback). Vinicius that controller are you using?
I haven't seen this ever in my testing.
--
Ville
^ permalink raw reply
* [PATCH] Add support for sending small data through obex
From: Radoslaw Jablonski @ 2010-11-01 9:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Radoslaw Jablonski
Added handling packets smaller than mtu in obex_write_stream. Now trying
to read from source until mtu will be filled properly and not sending
immediately data if it is smaller than mtu.
---
src/obex.c | 48 +++++++++++++++++++++++++++++++++++++-----------
1 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/src/obex.c b/src/obex.c
index 6d4430d..d3a6ccd 100644
--- a/src/obex.c
+++ b/src/obex.c
@@ -623,6 +623,7 @@ static int obex_write_stream(struct obex_session *os,
obex_headerdata_t hd;
uint8_t *ptr;
ssize_t len;
+ ssize_t r_len;
unsigned int flags;
uint8_t hi;
@@ -642,18 +643,37 @@ static int obex_write_stream(struct obex_session *os,
goto add_header;
}
- len = os->driver->read(os->object, os->buf, os->tx_mtu, &hi);
- if (len < 0) {
- error("read(): %s (%zd)", strerror(-len), -len);
- if (len == -EAGAIN)
- return len;
- else if (len == -ENOSTR)
- return 0;
+ /* Copying data from source until we reach end of the stream. Sending
+ * data only if MTU will be filled in 100% or we reach end of data.
+ * Remaining data in buffer will be sent with next amount of data
+ * from source.*/
+ do {
+ r_len = os->driver->read(os->object, os->buf + os->pending,
+ os->tx_mtu - os->pending, &hi);
+
+ if (r_len < 0) {
+ error("read(): %s (%zd)", strerror(-r_len), -r_len);
+
+ switch(r_len) {
+ case -EAGAIN:
+ return r_len;
+ case -EINTR:
+ continue;
+ case -ENOSTR:
+ return 0;
+ default:
+ g_free(os->buf);
+ os->buf = NULL;
+ return r_len;
+ }
+ }
- g_free(os->buf);
- os->buf = NULL;
- return len;
- }
+ /* Saving amount of data accumulated in obex buffer */
+ os->pending += r_len;
+ } while (os->pending < os->tx_mtu && r_len != 0);
+
+ len = os->pending;
+ os->pending = 0;
ptr = os->buf;
@@ -702,6 +722,12 @@ static gboolean handle_async_io(void *object, int flags, int err,
ret = obex_read_stream(os, os->obex, os->obj);
proceed:
+
+ /* Returning TRUE to not delete current watcher - it need to be active
+ * to handle next io flag changes (more data will be available later)*/
+ if (ret == -EAGAIN)
+ return TRUE;
+
if (ret < 0) {
os_set_response(os->obj, err);
OBEX_CancelRequest(os->obex, TRUE);
--
1.7.0.4
^ permalink raw reply related
* [PATCH 1/2] Add support for sending PBAP response in many parts
From: Radoslaw Jablonski @ 2010-11-01 10:54 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Radoslaw Jablonski
Previously pull result from PBAP was returned always in one big
part - a lot of memory was used to store all data. Now it is
possible to send data from pbap in many smaller chunks.
---
plugins/pbap.c | 29 ++++++++++++++++++++++++++++-
1 files changed, 28 insertions(+), 1 deletions(-)
diff --git a/plugins/pbap.c b/plugins/pbap.c
index 3ea7d6b..b4c1f00 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -143,6 +143,7 @@ struct pbap_session {
uint32_t find_handle;
GString *buffer;
struct cache cache;
+ gboolean partial_resp;
};
static const uint8_t PBAP_TARGET[TARGET_SIZE] = {
@@ -262,6 +263,10 @@ static void query_result(const char *buffer, size_t bufsize, int vcards,
pbap->buffer = g_string_append_len(pbap->buffer, buffer,
bufsize);
+ /* If partial_resp will be set to TRUE, then we won't end transmission
+ * after sending one part of results to the client via obex*/
+ pbap->partial_resp = missed ? TRUE : FALSE;
+
obex_object_set_io_flags(pbap, G_IO_IN, 0);
}
@@ -829,6 +834,28 @@ fail:
return NULL;
}
+static ssize_t string_read_part(void *object, void *buf, size_t count,
+ gboolean partial)
+{
+ GString *string = object;
+ ssize_t len;
+
+ if (string->len == 0) {
+ /* if more data will be available later, returning -EAGAIN
+ * instead of 0 (it will suspend request) */
+ if (partial)
+ return -EAGAIN;
+ else
+ return 0;
+ }
+
+ len = MIN(string->len, count);
+ memcpy(buf, string->str, len);
+ string = g_string_erase(string, 0, len);
+
+ return len;
+}
+
static ssize_t vobject_pull_read(void *object, void *buf, size_t count,
uint8_t *hi)
{
@@ -847,7 +874,7 @@ static ssize_t vobject_pull_read(void *object, void *buf, size_t count,
/* Stream data */
*hi = OBEX_HDR_BODY;
- return string_read(pbap->buffer, buf, count);
+ return string_read_part(pbap->buffer, buf, count, pbap->partial_resp);
}
static ssize_t vobject_list_read(void *object, void *buf, size_t count,
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/2] Add support for generating pull response in many parts
From: Radoslaw Jablonski @ 2010-11-01 10:54 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Radoslaw Jablonski
In-Reply-To: <1288608899-23032-1-git-send-email-ext-jablonski.radoslaw@nokia.com>
Now data from tracker is fetched in many smaller parts (instead of one
big query before). This is needed to save memory and to not overload
dbus and tracker when generating query result.
---
plugins/phonebook-tracker.c | 71 ++++++++++++++++++++++++++++++++++++-------
1 files changed, 60 insertions(+), 11 deletions(-)
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 58f52ab..46ef5fb 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -57,6 +57,8 @@
#define COL_ANSWERED 37
#define ADDR_FIELD_AMOUNT 7
#define CONTACT_ID_PREFIX "contact:"
+#define QUERY_LIMIT_FORMAT "%s LIMIT %d OFFSET %d"
+#define QUERY_LIMIT 50
#define CONTACTS_QUERY_ALL \
"SELECT ?v nco:fullname(?c) " \
@@ -650,6 +652,9 @@ struct phonebook_data {
gboolean vcardentry;
const struct apparam_field *params;
GSList *contacts;
+ char *name;
+ int offset;
+ int num_row;
};
struct cache_data {
@@ -1098,6 +1103,12 @@ static void add_affiliation(char **field, const char *value)
*field = g_strdup(value);
}
+static char *gen_partial_query(const char *name, int limit, int offset)
+{
+ return g_strdup_printf(QUERY_LIMIT_FORMAT, name2query(name),
+ limit, offset);
+}
+
static void pull_contacts(char **reply, int num_fields, void *user_data)
{
struct phonebook_data *data = user_data;
@@ -1107,13 +1118,17 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
GString *vcards;
int last_index, i;
gboolean cdata_present = FALSE;
- char *home_addr, *work_addr;
+ gboolean last_resp = FALSE;
+ char *home_addr, *work_addr, *query;
if (num_fields < 0) {
data->cb(NULL, 0, num_fields, 0, data->user_data);
goto fail;
}
+ data->num_row++;
+ last_index = params->liststartoffset + params->maxlistcount;
+
DBG("reply %p", reply);
if (reply == NULL)
@@ -1145,8 +1160,6 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
data->index++;
- last_index = params->liststartoffset + params->maxlistcount;
-
if ((data->index <= params->liststartoffset ||
data->index > last_index) &&
params->maxlistcount > 0)
@@ -1222,14 +1235,46 @@ add_numbers:
done:
vcards = gen_vcards(data->contacts, params);
- if (num_fields == 0)
+ /* If tracker returned only empty row - all results already returned */
+ if (num_fields == 0 && data->num_row == 1)
+ last_resp = TRUE;
+
+ /* Check if tracker could return desired number of results - if coldn't,
+ * all results are fetched already and this is last response */
+ if (data->num_row < QUERY_LIMIT)
+ last_resp = TRUE;
+
+ /* Check needed for 'maxlistcount' and 'liststartoffset' parameters */
+ if (data->index > last_index)
+ last_resp = TRUE;
+
+ /* Data won't be send if starting offset has not been achieved (unless
+ * now handling last response from tracker) */
+ if (data->index > params->liststartoffset || last_resp)
+ /* 4th parameter of callback is used to mark if stream should be
+ * closed or more data will be sent*/
data->cb(vcards->str, vcards->len,
- g_slist_length(data->contacts), 0,
- data->user_data);
+ g_slist_length(data->contacts), !last_resp,
+ data->user_data);
+ g_slist_free(data->contacts);data->contacts = NULL;
g_string_free(vcards, TRUE);
+ data->num_row = 0;
+
+ /* Sending query to tracker to get next part of results (only for pull
+ * phonebook queries) */
+ if (!data->vcardentry && !last_resp) {
+ data->offset += QUERY_LIMIT;
+ query = gen_partial_query(data->name, QUERY_LIMIT,
+ data->offset);
+ query_tracker(query, PULL_QUERY_COL_AMOUNT,
+ pull_contacts, data);
+ g_free(query);
+ return;
+ }
fail:
g_slist_free(data->contacts);
+ g_free(data->name);
g_free(data);
}
@@ -1367,18 +1412,18 @@ int phonebook_pull(const char *name, const struct apparam_field *params,
phonebook_cb cb, void *user_data)
{
struct phonebook_data *data;
- const char *query;
+ char *query;
reply_list_foreach_t pull_cb;
- int col_amount;
+ int col_amount, ret;
DBG("name %s", name);
if (params->maxlistcount == 0) {
- query = name2count_query(name);
+ query = g_strdup(name2count_query(name));
col_amount = COUNT_QUERY_COL_AMOUNT;
pull_cb = pull_contacts_size;
} else {
- query = name2query(name);
+ query = gen_partial_query(name, QUERY_LIMIT, 0);
col_amount = PULL_QUERY_COL_AMOUNT;
pull_cb = pull_contacts;
}
@@ -1390,8 +1435,12 @@ int phonebook_pull(const char *name, const struct apparam_field *params,
data->params = params;
data->user_data = user_data;
data->cb = cb;
+ data->name = g_strdup(name);
+
+ ret = query_tracker(query, col_amount, pull_cb, data);
+ g_free(query);
- return query_tracker(query, col_amount, pull_cb, data);
+ return ret;
}
int phonebook_get_entry(const char *folder, const char *id,
--
1.7.0.4
^ permalink raw reply related
* [PATCH] Fix avoid starting AVDTP disconnect timer twice
From: Daniel Örstadius @ 2010-11-01 13:36 UTC (permalink / raw)
To: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 36 bytes --]
Patch proposal for review.
/Daniel
[-- Attachment #2: 0001-Fix-avoid-starting-AVDTP-disconnect-timer-twice.patch --]
[-- Type: text/x-patch, Size: 1371 bytes --]
From 0aad44e90585e6a7c9a59cee2cdeff00316a527c Mon Sep 17 00:00:00 2001
From: Daniel Orstadius <daniel.orstadius@nokia.com>
Date: Mon, 1 Nov 2010 12:44:25 +0200
Subject: [PATCH] Fix avoid starting AVDTP disconnect timer twice
Remove starting the timer when setting the AVDTP state to idle. If
needed, the timer should probably already have been started in
avdtp_unref when the reference count goes to one.
Since reference counting is handled in avdtp_ref and avdtp_unref, it
seems reasonable that not to inspect the count outside of those
functions.
The issue was found when using Device.Disconnect to disconnect a
headset. It was revealed by commit
c72ce0f12a8387a70a6f0109f13bd6f414f32be8.
Before the commit, the timer was removed and then started again.
After applying it, the idle callback (disconnect_timeout) is called
twice, causing a crash.
---
audio/avdtp.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/audio/avdtp.c b/audio/avdtp.c
index 5e84a45..611fd7b 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -1034,8 +1034,6 @@ static void avdtp_sep_set_state(struct avdtp *session,
/* Remove pending commands for this stream from the queue */
cleanup_queue(session, stream);
stream_free(stream);
- if (session->ref == 1 && !session->streams)
- set_disconnect_timer(session);
break;
default:
break;
--
1.6.0.4
^ permalink raw reply related
* Re: [PATCHv3 2/2] Bluetooth: timer check sk is not owned before freeing
From: Andrei Emeltchenko @ 2010-11-01 14:20 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <20101029211718.GC14961@vigoh>
Hi Gustavo
On Sat, Oct 30, 2010 at 12:17 AM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
> Hi Andrei,
>
> * Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2010-10-29 16:43:01 +0300]:
>
>> 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. If sk is owned we
>> restart timer for HZ/5.
>>
>> 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 b1344d8..c67b3c6 100644
>> --- a/net/bluetooth/l2cap.c
>> +++ b/net/bluetooth/l2cap.c
>> @@ -83,6 +83,18 @@ static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn,
>> static int l2cap_ertm_data_rcv(struct sock *sk, struct sk_buff *skb);
>>
>> /* ---- 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;
>> @@ -92,6 +104,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 / 5);
>> + bh_unlock_sock(sk);
>> + sock_put(sk);
>
> You can't do a sock_put() here, you have to keep the referencee to the
> socket while the timer is enabled.
sk_reset_timer is holding sock when timer restarts. The same way done
in TCP code in function:
static void tcp_delack_timer(unsigned long data)
Regards,
Andrei
^ permalink raw reply
* Re: [PATCH] Fix avoid starting AVDTP disconnect timer twice
From: Johan Hedberg @ 2010-11-01 14:45 UTC (permalink / raw)
To: Daniel Örstadius; +Cc: linux-bluetooth
In-Reply-To: <AANLkTikPhBzAhQ=CeDkQokMK39k=c-VxU3re5GtMdiFZ@mail.gmail.com>
Hi Daniel,
On Mon, Nov 01, 2010, Daniel Örstadius wrote:
> Remove starting the timer when setting the AVDTP state to idle. If
> needed, the timer should probably already have been started in
> avdtp_unref when the reference count goes to one.
>
> Since reference counting is handled in avdtp_ref and avdtp_unref, it
> seems reasonable that not to inspect the count outside of those
> functions.
>
> The issue was found when using Device.Disconnect to disconnect a
> headset. It was revealed by commit
> c72ce0f12a8387a70a6f0109f13bd6f414f32be8.
>
> Before the commit, the timer was removed and then started again.
> After applying it, the idle callback (disconnect_timeout) is called
> twice, causing a crash.
Thanks for investigating and fixing this. avdtp_unref indeed does
already take care of the timer so the call in set_state seems redundant
(in addition to being in a questionable place to begin with). The patch
has been pushed upstream.
Johan
^ permalink raw reply
* [PATCH] [RFC] Add Application property to HealthChannel
From: Elvis Pfützenreuter @ 2010-11-01 18:16 UTC (permalink / raw)
To: linux-bluetooth; +Cc: epx
This patch adds the Application property to HealthChannel, which
allows to unambiguously relate a channel to an application.
This property is useful when there are several processes interested
in accepting HealthChannels but device address is not sufficient
criteria to engage upon, or ignore, the ChannelConnected signal.
Having the application path allows to determine role and data type,
in the context of the process that has created that application.
Also, channel path skeleton is fixed in documentation, in order to
reflect atual implementation.
---
doc/health-api.txt | 8 ++++++--
health/hdp.c | 3 +++
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/doc/health-api.txt b/doc/health-api.txt
index 89d5ff9..a0a1685 100644
--- a/doc/health-api.txt
+++ b/doc/health-api.txt
@@ -119,8 +119,7 @@ Properties:
Service org.bluez
Interface org.bluez.HealthChannel
-Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/
- hdp_YYYY/channel_ZZ
+Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/chanZZZ
Only the process that created the data channel or the creator of the
HealthApplication that received it will be able to call this methods.
@@ -160,3 +159,8 @@ Properties:
Identifies the Remote Device that is connected with. Maps with
a HealthDevice object.
+
+ object Application [readonly]
+
+ Identifies the HealthApplication to which this channel is
+ related to (which indirectly defines its role and data type).
diff --git a/health/hdp.c b/health/hdp.c
index be30204..1eba8e1 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -423,6 +423,9 @@ static DBusMessage *channel_get_properties(DBusConnection *conn,
path = device_get_path(chan->dev->dev);
dict_append_entry(&dict, "Device", DBUS_TYPE_OBJECT_PATH, &path);
+ path = chan->app->path;
+ dict_append_entry(&dict, "Application", DBUS_TYPE_OBJECT_PATH, &path);
+
if (chan->config == HDP_RELIABLE_DC)
type = g_strdup("Reliable");
else
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH v4 0/2] Get and Set Feature Reports on HIDRAW (USB and Bluetooth)
From: Jiri Kosina @ 2010-11-01 19:23 UTC (permalink / raw)
To: Alan Ott, Marcel Holtmann, David S. Miller
Cc: Stefan Achatz, Antonio Ospite, Alexey Dobriyan, Tejun Heo,
Alan Stern, Greg Kroah-Hartman, Stephane Chatty, Michael Poole,
Bastien Nocera, Eric Dumazet, linux-input, linux-kernel,
linux-usb, linux-bluetooth, netdev
In-Reply-To: <alpine.LNX.2.00.1009221408330.26813@pobox.suse.cz>
On Wed, 22 Sep 2010, Jiri Kosina wrote:
> > > > This is version 4. Built against 2.6.35+ revision 320b2b8de12698 .
> > > >
> > > > Alan Ott (2):
> > > > HID: Add Support for Setting and Getting Feature Reports from hidraw
> > > > Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and
> > > > HIDIOCSFEATURE
> > > >
> > > > drivers/hid/hidraw.c | 105 ++++++++++++++++++++++++++++++++++++--
> > > > drivers/hid/usbhid/hid-core.c | 37 +++++++++++++-
> > > > include/linux/hid.h | 3 +
> > > > include/linux/hidraw.h | 3 +
> > > > net/bluetooth/hidp/core.c | 114 +++++++++++++++++++++++++++++++++++++++--
> > > > net/bluetooth/hidp/hidp.h | 8 +++
> > > > 6 files changed, 260 insertions(+), 10 deletions(-)
> > >
> > > Marcel, as per our previous discussion -- what is your word on this? I'd
> > > be glad taking it once you Ack the bluetooth bits (which, as far as I
> > > understood from your last mail, don't have strong objections against any
> > > more).
> >
> > ... Marcel?
> >
> > I'd really like not to miss 2.6.37 merge window with this.
>
> Seemingly I have not enought powers to get statement from Marcel here
> these days/weeks.
>
> Davem, would you perhaps be able to step in here?
Marcel, any word on this patchset by chance?
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
^ permalink raw reply
* Re: [PATCH] Add support for sending small data through obex
From: Luiz Augusto von Dentz @ 2010-11-01 23:01 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <1288603499-16456-1-git-send-email-ext-jablonski.radoslaw@nokia.com>
Hi,
On Mon, Nov 1, 2010 at 11:24 AM, Radoslaw Jablonski
<ext-jablonski.radoslaw@nokia.com> wrote:
> Added handling packets smaller than mtu in obex_write_stream. Now trying
> to read from source until mtu will be filled properly and not sending
> immediately data if it is smaller than mtu.
> ---
> src/obex.c | 48 +++++++++++++++++++++++++++++++++++++-----------
> 1 files changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/src/obex.c b/src/obex.c
> index 6d4430d..d3a6ccd 100644
> --- a/src/obex.c
> +++ b/src/obex.c
> @@ -623,6 +623,7 @@ static int obex_write_stream(struct obex_session *os,
> obex_headerdata_t hd;
> uint8_t *ptr;
> ssize_t len;
> + ssize_t r_len;
You can have len and r_len in the same line above.
> unsigned int flags;
> uint8_t hi;
>
> @@ -642,18 +643,37 @@ static int obex_write_stream(struct obex_session *os,
> goto add_header;
> }
>
> - len = os->driver->read(os->object, os->buf, os->tx_mtu, &hi);
> - if (len < 0) {
> - error("read(): %s (%zd)", strerror(-len), -len);
> - if (len == -EAGAIN)
> - return len;
> - else if (len == -ENOSTR)
> - return 0;
> + /* Copying data from source until we reach end of the stream. Sending
> + * data only if MTU will be filled in 100% or we reach end of data.
> + * Remaining data in buffer will be sent with next amount of data
> + * from source.*/
> + do {
> + r_len = os->driver->read(os->object, os->buf + os->pending,
> + os->tx_mtu - os->pending, &hi);
It looks like you should add one more tab in the line above it that
doesn't go over 80 columns.
> + if (r_len < 0) {
> + error("read(): %s (%zd)", strerror(-r_len), -r_len);
> +
> + switch(r_len) {
> + case -EAGAIN:
> + return r_len;
> + case -EINTR:
> + continue;
> + case -ENOSTR:
> + return 0;
> + default:
> + g_free(os->buf);
> + os->buf = NULL;
> + return r_len;
> + }
> + }
I think it is probably more efficiently to handle rlen == 0 here, like
if (rlen == 0) break; else if (rlen < 0)
> - g_free(os->buf);
> - os->buf = NULL;
> - return len;
> - }
> + /* Saving amount of data accumulated in obex buffer */
> + os->pending += r_len;
> + } while (os->pending < os->tx_mtu && r_len != 0);
That simplify here too, just do while (os->pending < os->txt_mtu) if
you handle rlen == 0 like I said before.
> +
> + len = os->pending;
> + os->pending = 0;
>
> ptr = os->buf;
>
> @@ -702,6 +722,12 @@ static gboolean handle_async_io(void *object, int flags, int err,
> ret = obex_read_stream(os, os->obex, os->obj);
>
> proceed:
> +
> + /* Returning TRUE to not delete current watcher - it need to be active
> + * to handle next io flag changes (more data will be available later)*/
> + if (ret == -EAGAIN)
> + return TRUE;
> +
> if (ret < 0) {
> os_set_response(os->obj, err);
> OBEX_CancelRequest(os->obex, TRUE);
> --
> 1.7.0.4
>
> --
> 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
>
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH 1/2] Add support for sending PBAP response in many parts
From: Luiz Augusto von Dentz @ 2010-11-01 23:10 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <1288608899-23032-1-git-send-email-ext-jablonski.radoslaw@nokia.com>
Hi,
On Mon, Nov 1, 2010 at 12:54 PM, Radoslaw Jablonski
<ext-jablonski.radoslaw@nokia.com> wrote:
> Previously pull result from PBAP was returned always in one big
> part - a lot of memory was used to store all data. Now it is
> possible to send data from pbap in many smaller chunks.
> ---
> plugins/pbap.c | 29 ++++++++++++++++++++++++++++-
> 1 files changed, 28 insertions(+), 1 deletions(-)
>
> diff --git a/plugins/pbap.c b/plugins/pbap.c
> index 3ea7d6b..b4c1f00 100644
> --- a/plugins/pbap.c
> +++ b/plugins/pbap.c
> @@ -143,6 +143,7 @@ struct pbap_session {
> uint32_t find_handle;
> GString *buffer;
> struct cache cache;
> + gboolean partial_resp;
> };
>
> static const uint8_t PBAP_TARGET[TARGET_SIZE] = {
> @@ -262,6 +263,10 @@ static void query_result(const char *buffer, size_t bufsize, int vcards,
> pbap->buffer = g_string_append_len(pbap->buffer, buffer,
> bufsize);
>
> + /* If partial_resp will be set to TRUE, then we won't end transmission
> + * after sending one part of results to the client via obex*/
> + pbap->partial_resp = missed ? TRUE : FALSE;
> +
> obex_object_set_io_flags(pbap, G_IO_IN, 0);
> }
>
> @@ -829,6 +834,28 @@ fail:
> return NULL;
> }
>
> +static ssize_t string_read_part(void *object, void *buf, size_t count,
> + gboolean partial)
> +{
> + GString *string = object;
> + ssize_t len;
> +
> + if (string->len == 0) {
> + /* if more data will be available later, returning -EAGAIN
> + * instead of 0 (it will suspend request) */
> + if (partial)
> + return -EAGAIN;
> + else
> + return 0;
> + }
> +
> + len = MIN(string->len, count);
> + memcpy(buf, string->str, len);
> + string = g_string_erase(string, 0, len);
> +
> + return len;
> +}
> +
> static ssize_t vobject_pull_read(void *object, void *buf, size_t count,
> uint8_t *hi)
> {
> @@ -847,7 +874,7 @@ static ssize_t vobject_pull_read(void *object, void *buf, size_t count,
> /* Stream data */
> *hi = OBEX_HDR_BODY;
>
> - return string_read(pbap->buffer, buf, count);
> + return string_read_part(pbap->buffer, buf, count, pbap->partial_resp);
> }
Im not sure why you decided to create another string_read copy to
handle this, why not using string_read return and check if it is a
partial response on directly on vobject_pull_read? Sounds simple to me
and don't duplicate any code.
> static ssize_t vobject_list_read(void *object, void *buf, size_t count,
> --
> 1.7.0.4
>
> --
> 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
>
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH 2/2] Add support for generating pull response in many parts
From: Luiz Augusto von Dentz @ 2010-11-01 23:56 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <1288608899-23032-2-git-send-email-ext-jablonski.radoslaw@nokia.com>
Hi,
On Mon, Nov 1, 2010 at 12:54 PM, Radoslaw Jablonski
<ext-jablonski.radoslaw@nokia.com> wrote:
> Now data from tracker is fetched in many smaller parts (instead of one
> big query before). This is needed to save memory and to not overload
> dbus and tracker when generating query result.
> ---
> plugins/phonebook-tracker.c | 71 ++++++++++++++++++++++++++++++++++++-------
> 1 files changed, 60 insertions(+), 11 deletions(-)
>
> diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
> index 58f52ab..46ef5fb 100644
> --- a/plugins/phonebook-tracker.c
> +++ b/plugins/phonebook-tracker.c
> @@ -57,6 +57,8 @@
> #define COL_ANSWERED 37
> #define ADDR_FIELD_AMOUNT 7
> #define CONTACT_ID_PREFIX "contact:"
> +#define QUERY_LIMIT_FORMAT "%s LIMIT %d OFFSET %d"
> +#define QUERY_LIMIT 50
>
> #define CONTACTS_QUERY_ALL \
> "SELECT ?v nco:fullname(?c) " \
> @@ -650,6 +652,9 @@ struct phonebook_data {
> gboolean vcardentry;
> const struct apparam_field *params;
> GSList *contacts;
> + char *name;
> + int offset;
> + int num_row;
> };
>
> struct cache_data {
> @@ -1098,6 +1103,12 @@ static void add_affiliation(char **field, const char *value)
> *field = g_strdup(value);
> }
>
> +static char *gen_partial_query(const char *name, int limit, int offset)
> +{
> + return g_strdup_printf(QUERY_LIMIT_FORMAT, name2query(name),
> + limit, offset);
> +}
> +
> static void pull_contacts(char **reply, int num_fields, void *user_data)
> {
> struct phonebook_data *data = user_data;
> @@ -1107,13 +1118,17 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
> GString *vcards;
> int last_index, i;
> gboolean cdata_present = FALSE;
> - char *home_addr, *work_addr;
> + gboolean last_resp = FALSE;
> + char *home_addr, *work_addr, *query;
>
> if (num_fields < 0) {
> data->cb(NULL, 0, num_fields, 0, data->user_data);
> goto fail;
> }
>
> + data->num_row++;
> + last_index = params->liststartoffset + params->maxlistcount;
> +
> DBG("reply %p", reply);
>
> if (reply == NULL)
> @@ -1145,8 +1160,6 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
>
> data->index++;
>
> - last_index = params->liststartoffset + params->maxlistcount;
> -
> if ((data->index <= params->liststartoffset ||
> data->index > last_index) &&
> params->maxlistcount > 0)
> @@ -1222,14 +1235,46 @@ add_numbers:
> done:
> vcards = gen_vcards(data->contacts, params);
>
> - if (num_fields == 0)
> + /* If tracker returned only empty row - all results already returned */
> + if (num_fields == 0 && data->num_row == 1)
> + last_resp = TRUE;
> +
> + /* Check if tracker could return desired number of results - if coldn't,
> + * all results are fetched already and this is last response */
> + if (data->num_row < QUERY_LIMIT)
> + last_resp = TRUE;
> +
> + /* Check needed for 'maxlistcount' and 'liststartoffset' parameters */
> + if (data->index > last_index)
> + last_resp = TRUE;
> +
> + /* Data won't be send if starting offset has not been achieved (unless
> + * now handling last response from tracker) */
> + if (data->index > params->liststartoffset || last_resp)
> + /* 4th parameter of callback is used to mark if stream should be
> + * closed or more data will be sent*/
> data->cb(vcards->str, vcards->len,
> - g_slist_length(data->contacts), 0,
> - data->user_data);
> + g_slist_length(data->contacts), !last_resp,
> + data->user_data);
>
> + g_slist_free(data->contacts);data->contacts = NULL;
> g_string_free(vcards, TRUE);
> + data->num_row = 0;
> +
> + /* Sending query to tracker to get next part of results (only for pull
> + * phonebook queries) */
> + if (!data->vcardentry && !last_resp) {
> + data->offset += QUERY_LIMIT;
> + query = gen_partial_query(data->name, QUERY_LIMIT,
> + data->offset);
> + query_tracker(query, PULL_QUERY_COL_AMOUNT,
> + pull_contacts, data);
> + g_free(query);
> + return;
> + }
> fail:
> g_slist_free(data->contacts);
> + g_free(data->name);
> g_free(data);
> }
>
> @@ -1367,18 +1412,18 @@ int phonebook_pull(const char *name, const struct apparam_field *params,
> phonebook_cb cb, void *user_data)
> {
> struct phonebook_data *data;
> - const char *query;
> + char *query;
> reply_list_foreach_t pull_cb;
> - int col_amount;
> + int col_amount, ret;
>
> DBG("name %s", name);
>
> if (params->maxlistcount == 0) {
> - query = name2count_query(name);
> + query = g_strdup(name2count_query(name));
> col_amount = COUNT_QUERY_COL_AMOUNT;
> pull_cb = pull_contacts_size;
> } else {
> - query = name2query(name);
> + query = gen_partial_query(name, QUERY_LIMIT, 0);
> col_amount = PULL_QUERY_COL_AMOUNT;
> pull_cb = pull_contacts;
> }
> @@ -1390,8 +1435,12 @@ int phonebook_pull(const char *name, const struct apparam_field *params,
> data->params = params;
> data->user_data = user_data;
> data->cb = cb;
> + data->name = g_strdup(name);
> +
> + ret = query_tracker(query, col_amount, pull_cb, data);
> + g_free(query);
>
> - return query_tracker(query, col_amount, pull_cb, data);
> + return ret;
> }
Did you actually check if tracker/sparql doesn't support having a byte
limit instead of contact/row? I know this sounds crazy, but Ive seem
some other implementations of pbap that does similar things as to
query a number of contacts and they can cause big pauses when
generating the responses depending on the size of the MTU being used
and in fact doesn't completely eliminate the extra buffering on the
plugin side. Also I think we might need to use the read callback to
continue the queries and not do it regardless of the speed the client
can read, otherwise we may run in the same situation as we have now
but instead of asking all the data at once we do it in parts but we
still don't care if the client is fetching that data at the same pace
we buffer.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH] [RFC] Add Application property to HealthChannel
From: Gustavo F. Padovan @ 2010-11-02 3:35 UTC (permalink / raw)
To: Elvis Pfützenreuter; +Cc: linux-bluetooth
In-Reply-To: <1288635382-2032-1-git-send-email-epx@signove.com>
* Elvis Pfützenreuter <epx@signove.com> [2010-11-01 16:16:22 -0200]:
> This patch adds the Application property to HealthChannel, which
> allows to unambiguously relate a channel to an application.
>
> This property is useful when there are several processes interested
> in accepting HealthChannels but device address is not sufficient
> criteria to engage upon, or ignore, the ChannelConnected signal.
> Having the application path allows to determine role and data type,
> in the context of the process that has created that application.
>
> Also, channel path skeleton is fixed in documentation, in order to
> reflect atual implementation.
So a separeted patch here.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
^ permalink raw reply
* [PATCH v2] Add support for sending small data through obex
From: Radoslaw Jablonski @ 2010-11-02 9:25 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Radoslaw Jablonski
Added handling packets smaller than mtu in obex_write_stream. Now trying
to read from source until mtu will be filled properly and not sending
immediately data if it is smaller than mtu.
---
src/obex.c | 51 +++++++++++++++++++++++++++++++++++++++------------
1 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/src/obex.c b/src/obex.c
index 6d4430d..4cbc1f8 100644
--- a/src/obex.c
+++ b/src/obex.c
@@ -622,7 +622,7 @@ static int obex_write_stream(struct obex_session *os,
{
obex_headerdata_t hd;
uint8_t *ptr;
- ssize_t len;
+ ssize_t len, r_len;
unsigned int flags;
uint8_t hi;
@@ -642,18 +642,39 @@ static int obex_write_stream(struct obex_session *os,
goto add_header;
}
- len = os->driver->read(os->object, os->buf, os->tx_mtu, &hi);
- if (len < 0) {
- error("read(): %s (%zd)", strerror(-len), -len);
- if (len == -EAGAIN)
- return len;
- else if (len == -ENOSTR)
- return 0;
+ /* Copying data from source until we reach end of the stream. Sending
+ * data only if MTU will be filled in 100% or we reach end of data.
+ * Remaining data in buffer will be sent with next amount of data
+ * from source.*/
+ do {
+ r_len = os->driver->read(os->object, os->buf + os->pending,
+ os->tx_mtu - os->pending, &hi);
- g_free(os->buf);
- os->buf = NULL;
- return len;
- }
+ if (r_len == 0)
+ break;
+ else if (r_len < 0) {
+ error("read(): %s (%zd)", strerror(-r_len), -r_len);
+
+ switch(r_len) {
+ case -EAGAIN:
+ return r_len;
+ case -EINTR:
+ continue;
+ case -ENOSTR:
+ return 0;
+ default:
+ g_free(os->buf);
+ os->buf = NULL;
+ return r_len;
+ }
+ }
+
+ /* Saving amount of data accumulated in obex buffer */
+ os->pending += r_len;
+ } while (os->pending < os->tx_mtu);
+
+ len = os->pending;
+ os->pending = 0;
ptr = os->buf;
@@ -702,6 +723,12 @@ static gboolean handle_async_io(void *object, int flags, int err,
ret = obex_read_stream(os, os->obex, os->obj);
proceed:
+
+ /* Returning TRUE to not delete current watcher - it need to be active
+ * to handle next io flag changes (more data will be available later)*/
+ if (ret == -EAGAIN)
+ return TRUE;
+
if (ret < 0) {
os_set_response(os->obj, err);
OBEX_CancelRequest(os->obex, TRUE);
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH v2] Add support for sending small data through obex
From: Luiz Augusto von Dentz @ 2010-11-02 12:05 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <1288689911-18507-1-git-send-email-ext-jablonski.radoslaw@nokia.com>
Hi,
On Tue, Nov 2, 2010 at 11:25 AM, Radoslaw Jablonski
<ext-jablonski.radoslaw@nokia.com> wrote:
> Added handling packets smaller than mtu in obex_write_stream. Now trying
> to read from source until mtu will be filled properly and not sending
> immediately data if it is smaller than mtu.
> ---
> src/obex.c | 51 +++++++++++++++++++++++++++++++++++++++------------
> 1 files changed, 39 insertions(+), 12 deletions(-)
>
> diff --git a/src/obex.c b/src/obex.c
> index 6d4430d..4cbc1f8 100644
> --- a/src/obex.c
> +++ b/src/obex.c
> @@ -622,7 +622,7 @@ static int obex_write_stream(struct obex_session *os,
> {
> obex_headerdata_t hd;
> uint8_t *ptr;
> - ssize_t len;
> + ssize_t len, r_len;
> unsigned int flags;
> uint8_t hi;
>
> @@ -642,18 +642,39 @@ static int obex_write_stream(struct obex_session *os,
> goto add_header;
> }
>
> - len = os->driver->read(os->object, os->buf, os->tx_mtu, &hi);
> - if (len < 0) {
> - error("read(): %s (%zd)", strerror(-len), -len);
> - if (len == -EAGAIN)
> - return len;
> - else if (len == -ENOSTR)
> - return 0;
> + /* Copying data from source until we reach end of the stream. Sending
> + * data only if MTU will be filled in 100% or we reach end of data.
> + * Remaining data in buffer will be sent with next amount of data
> + * from source.*/
> + do {
> + r_len = os->driver->read(os->object, os->buf + os->pending,
> + os->tx_mtu - os->pending, &hi);
>
> - g_free(os->buf);
> - os->buf = NULL;
> - return len;
> - }
> + if (r_len == 0)
> + break;
> + else if (r_len < 0) {
> + error("read(): %s (%zd)", strerror(-r_len), -r_len);
> +
> + switch(r_len) {
> + case -EAGAIN:
> + return r_len;
> + case -EINTR:
> + continue;
> + case -ENOSTR:
> + return 0;
> + default:
> + g_free(os->buf);
> + os->buf = NULL;
> + return r_len;
> + }
> + }
> +
> + /* Saving amount of data accumulated in obex buffer */
> + os->pending += r_len;
> + } while (os->pending < os->tx_mtu);
> +
> + len = os->pending;
> + os->pending = 0;
>
> ptr = os->buf;
>
> @@ -702,6 +723,12 @@ static gboolean handle_async_io(void *object, int flags, int err,
> ret = obex_read_stream(os, os->obex, os->obj);
>
> proceed:
> +
> + /* Returning TRUE to not delete current watcher - it need to be active
> + * to handle next io flag changes (more data will be available later)*/
> + if (ret == -EAGAIN)
> + return TRUE;
> +
> if (ret < 0) {
> os_set_response(os->obj, err);
> OBEX_CancelRequest(os->obex, TRUE);
> --
> 1.7.0.4
>
> --
> 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
>
Ack
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH 2/2] Add support for generating pull response in many parts
From: Luiz Augusto von Dentz @ 2010-11-02 12:48 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=xHO1Ste4+6AEULbs91qeL6NCuYAK8exh63gqy@mail.gmail.com>
Hi,
On Tue, Nov 2, 2010 at 1:56 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Did you actually check if tracker/sparql doesn't support having a byte
> limit instead of contact/row? I know this sounds crazy, but Ive seem
> some other implementations of pbap that does similar things as to
> query a number of contacts and they can cause big pauses when
> generating the responses depending on the size of the MTU being used
> and in fact doesn't completely eliminate the extra buffering on the
> plugin side. Also I think we might need to use the read callback to
> continue the queries and not do it regardless of the speed the client
> can read, otherwise we may run in the same situation as we have now
> but instead of asking all the data at once we do it in parts but we
> still don't care if the client is fetching that data at the same pace
> we buffer.
Radek and I discussed this offline and came to a conclusion that using
a temporary files to buffer the data is probably a better idea, first
because it will be difficult to synchronize the speed of client and
backend which can either cause too much buffering (OOM) or slow
transfer speed, the second reason is that if we start supporting
avatar/image then each contact will probably consume a lot more memory
than it does right now and third caching can probably be done much
more efficiently using temporary files.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox