2011-02-11 14:05:33

by Radoslaw Jablonski

[permalink] [raw]
Subject: [PATCH 1/7 v2] Add return value to query tracker

Previously errors were returned via parameter err - it was needed
because some time ago query_tracker was returning newly allocated
structure. Now returning error through return value has more
sense.
---
plugins/phonebook-tracker.c | 40 +++++++++++++++++++++-------------------
1 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index e116175..623e294 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -1078,9 +1078,8 @@ failed:
g_free(pending);
}

-static void query_tracker(const char *query, int num_fields,
- reply_list_foreach_t callback, void *user_data,
- int *err)
+static int query_tracker(const char *query, int num_fields,
+ reply_list_foreach_t callback, void *user_data)
{
struct pending_reply *pending;
GCancellable *cancellable;
@@ -1099,7 +1098,7 @@ static void query_tracker(const char *query, int num_fields,
g_error_free(error);
}

- goto failed;
+ return -EINTR;
}

cancellable = g_cancellable_new();
@@ -1115,7 +1114,7 @@ static void query_tracker(const char *query, int num_fields,

g_object_unref(cancellable);

- goto failed;
+ return -EINTR;
}

pending = g_new0(struct pending_reply, 1);
@@ -1130,14 +1129,7 @@ static void query_tracker(const char *query, int num_fields,
async_query_cursor_next_cb,
pending);

- if (err)
- *err = 0;
-
- return;
-
-failed:
- if (err)
- *err = -EPERM;
+ return 0;
}

static char *iso8601_utc_to_localtime(const char *datetime)
@@ -1877,7 +1869,7 @@ done:
pull_cb = pull_contacts;
}

- query_tracker(query, col_amount, pull_cb, data, &err);
+ err = query_tracker(query, col_amount, pull_cb, data);
if (err < 0)
data->cb(NULL, 0, err, 0, data->user_data);
}
@@ -1888,7 +1880,7 @@ void *phonebook_pull(const char *name, const struct apparam_field *params,
struct phonebook_data *data;
const char *query;
reply_list_foreach_t pull_cb;
- int col_amount;
+ int col_amount, ret;

DBG("name %s", name);

@@ -1916,7 +1908,10 @@ void *phonebook_pull(const char *name, const struct apparam_field *params,
data->params = params;
data->user_data = user_data;
data->cb = cb;
- query_tracker(query, col_amount, pull_cb, data, err);
+ ret = query_tracker(query, col_amount, pull_cb, data);
+
+ if(err)
+ *err = ret;

return data;
}
@@ -1927,6 +1922,7 @@ void *phonebook_get_entry(const char *folder, const char *id,
{
struct phonebook_data *data;
char *query;
+ int ret;

DBG("folder %s id %s", folder, id);

@@ -1944,11 +1940,13 @@ void *phonebook_get_entry(const char *folder, const char *id,
query = g_strdup_printf(CONTACTS_OTHER_QUERY_FROM_URI,
id, id, id);

- query_tracker(query, PULL_QUERY_COL_AMOUNT,
- pull_contacts, data, err);
+ ret = query_tracker(query, PULL_QUERY_COL_AMOUNT, pull_contacts, data);

g_free(query);

+ if(err)
+ *err = ret;
+
return data;
}

@@ -1957,6 +1955,7 @@ void *phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
{
struct phonebook_data *data;
const char *query;
+ int ret;

DBG("name %s", name);

@@ -1971,7 +1970,10 @@ void *phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
data->entry_cb = entry_cb;
data->ready_cb = ready_cb;
data->user_data = user_data;
- query_tracker(query, 7, add_to_cache, data, err);
+ ret = query_tracker(query, 7, add_to_cache, data);
+
+ if(err)
+ *err = ret;

return data;
}
--
1.7.0.4



2011-02-11 21:14:51

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 6/7 v2] Introduction of phonebook_pull_read

Hi Radek,

On Fri, Feb 11, 2011, Radoslaw Jablonski wrote:
> + ret = phonebook_pull_read(irmc->request);
> +
> + if (err) {
> + *err = ret;
> + }

No empty line before the if-statement and no {} for one-line sections.

> + ret = phonebook_pull_read(irmc->request);
> +
> + if (ret < 0) {

No empty line here, like above.

> + ret = phonebook_pull_read(request);
> +
> + if (ret < 0)

Same here.

> + len = string_read(obj->buffer, buf, count);
> +
> + if (len == 0 && !obj->lastpart) {

And here.

Johan

2011-02-11 21:11:55

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 1/7 v2] Add return value to query tracker

Hi Radek,

On Fri, Feb 11, 2011, Radoslaw Jablonski wrote:
> + if(err)

Coding style: space between if and (

> + if(err)

Same here.

> + if(err)

And here.

Johan

2011-02-11 14:05:34

by Radoslaw Jablonski

[permalink] [raw]
Subject: [PATCH 2/7 v2] Add return value to reply_list_foreach_t in phonebook-tracker

This changes are needed for notyfying that fetching data
from tracker should be stopped.
This will be usable in scenarios like:
*some error occurred during processing data
*maximum number of results already processed (due to pbap
maxlistcount parameter)
*suspending processing data if pbap buffer is too large
---
plugins/phonebook-tracker.c | 58 ++++++++++++++++++++++++++----------------
1 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 623e294..a8c8a87 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -890,7 +890,7 @@
"} GROUP BY ?call ORDER BY DESC(nmo:receivedDate(?call)) " \
"LIMIT 40"

-typedef void (*reply_list_foreach_t) (const char **reply, int num_fields,
+typedef int (*reply_list_foreach_t) (const char **reply, int num_fields,
void *user_data);

typedef void (*add_field_t) (struct phonebook_contact *contact,
@@ -1042,6 +1042,7 @@ static void async_query_cursor_next_cb(GObject *source, GAsyncResult *result,
GError *error = NULL;
gboolean success;
const char **node;
+ int err;

success = tracker_sparql_cursor_next_finish(
TRACKER_SPARQL_CURSOR(source),
@@ -1061,17 +1062,22 @@ static void async_query_cursor_next_cb(GObject *source, GAsyncResult *result,
}

node = string_array_from_cursor(cursor, pending->num_fields);
- pending->callback(node, pending->num_fields, pending->user_data);
+ err = pending->callback(node, pending->num_fields, pending->user_data);
g_free(node);

-
- /* getting next row from query results */
- cancellable = g_cancellable_new();
- update_cancellable(pending->user_data, cancellable);
- tracker_sparql_cursor_next_async(cursor, cancellable,
+ /* Fetch next result only if processing current chunk ended with
+ * success. Sometimes during processing data, we are able to determine
+ * if there is no need to get more data from tracker - by example
+ * stored amount of data parts is big enough for sending and we might
+ * want to suspend processing or just some error occurred. */
+ if (!err) {
+ cancellable = g_cancellable_new();
+ update_cancellable(pending->user_data, cancellable);
+ tracker_sparql_cursor_next_async(cursor, cancellable,
async_query_cursor_next_cb,
pending);
- return;
+ return;
+ }

failed:
g_object_unref(cursor);
@@ -1322,23 +1328,24 @@ static GString *gen_vcards(GSList *contacts,
return vcards;
}

-static void pull_contacts_size(const char **reply, int num_fields,
+static int pull_contacts_size(const char **reply, int num_fields,
void *user_data)
{
struct phonebook_data *data = user_data;

if (num_fields < 0) {
data->cb(NULL, 0, num_fields, 0, data->user_data);
- return;
+ return -EINTR;
}

if (reply != NULL) {
data->index = atoi(reply[0]);
- return;
+ return 0;
}

data->cb(NULL, 0, data->index, data->newmissedcalls, data->user_data);

+ return 0;
/*
* phonebook_data is freed in phonebook_req_finalize. Useful in
* cases when call is terminated.
@@ -1542,7 +1549,7 @@ static void contact_add_organization(struct phonebook_contact *contact,
add_affiliation(&contact->role, reply[COL_ORG_ROLE]);
}

-static void pull_contacts(const char **reply, int num_fields, void *user_data)
+static int pull_contacts(const char **reply, int num_fields, void *user_data)
{
struct phonebook_data *data = user_data;
const struct apparam_field *params = data->params;
@@ -1585,7 +1592,7 @@ static void pull_contacts(const char **reply, int num_fields, void *user_data)

if (i == num_fields - 4 && !g_str_equal(reply[CONTACTS_ID_COL],
TRACKER_DEFAULT_CONTACT_ME))
- return;
+ return 0;

if (g_strcmp0(temp_id, reply[CONTACTS_ID_COL])) {
data->index++;
@@ -1598,7 +1605,7 @@ static void pull_contacts(const char **reply, int num_fields, void *user_data)
if ((data->index <= params->liststartoffset ||
data->index > last_index) &&
params->maxlistcount > 0)
- return;
+ return 0;

add_entry:
contact = g_new0(struct phonebook_contact, 1);
@@ -1622,7 +1629,7 @@ add_numbers:
data->contacts = g_slist_append(data->contacts, contact_data);
}

- return;
+ return 0;

done:
vcards = gen_vcards(data->contacts, params);
@@ -1637,13 +1644,14 @@ fail:
g_free(temp_id);
temp_id = NULL;

+ return -EINTR;
/*
* phonebook_data is freed in phonebook_req_finalize. Useful in
* cases when call is terminated.
*/
}

-static void add_to_cache(const char **reply, int num_fields, void *user_data)
+static int add_to_cache(const char **reply, int num_fields, void *user_data)
{
struct phonebook_data *data = user_data;
char *formatted;
@@ -1660,7 +1668,7 @@ static void add_to_cache(const char **reply, int num_fields, void *user_data)

if (i == num_fields &&
!g_str_equal(reply[0], TRACKER_DEFAULT_CONTACT_ME))
- return;
+ return 0;

if (i == 6)
formatted = g_strdup(reply[6]);
@@ -1679,12 +1687,13 @@ static void add_to_cache(const char **reply, int num_fields, void *user_data)

g_free(formatted);

- return;
+ return 0;

done:
if (num_fields <= 0)
data->ready_cb(data->user_data);

+ return -EINTR;
/*
* phonebook_data is freed in phonebook_req_finalize. Useful in
* cases when call is terminated.
@@ -1826,7 +1835,7 @@ static void gstring_free_helper(gpointer data, gpointer user_data)
g_string_free(data, TRUE);
}

-static void pull_newmissedcalls(const char **reply, int num_fields,
+static int pull_newmissedcalls(const char **reply, int num_fields,
void *user_data)
{
struct phonebook_data *data = user_data;
@@ -1846,7 +1855,7 @@ static void pull_newmissedcalls(const char **reply, int num_fields,
number);
}
}
- return;
+ return 0;

done:
DBG("newmissedcalls %d", data->newmissedcalls);
@@ -1856,7 +1865,7 @@ done:

if (num_fields < 0) {
data->cb(NULL, 0, num_fields, 0, data->user_data);
- return;
+ return -EINTR;
}

if (data->params->maxlistcount == 0) {
@@ -1870,8 +1879,13 @@ done:
}

err = query_tracker(query, col_amount, pull_cb, data);
- if (err < 0)
+ if (err < 0) {
data->cb(NULL, 0, err, 0, data->user_data);
+
+ return -EINTR;
+ }
+
+ return 0;
}

void *phonebook_pull(const char *name, const struct apparam_field *params,
--
1.7.0.4


2011-02-11 14:05:37

by Radoslaw Jablonski

[permalink] [raw]
Subject: [PATCH 5/7 v2] Add support for notyfying pbap about more parts from backend

Added new parameter to phonebook_cb - lastpart variable.
If backend want to notify that more parts of current
request will be delivered later, it should use lastpart=FALSE.
Because of that, PBAP core will 'know' that should not finalize
request immediately after receiving data and wait for more
parts to come.
If result is returned in one part and no more responses part
will be sent later, then backend should use lastparam=TRUE.
Previously results of request from backend was always returned
in one part to PBAP core.
---
plugins/irmc.c | 5 +++--
plugins/pbap.c | 10 +++++++---
plugins/phonebook-dummy.c | 4 ++--
plugins/phonebook-ebook.c | 7 ++++---
plugins/phonebook-tracker.c | 13 +++++++------
plugins/phonebook.h | 2 +-
6 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/plugins/irmc.c b/plugins/irmc.c
index 0488cae..e1e83f9 100644
--- a/plugins/irmc.c
+++ b/plugins/irmc.c
@@ -133,7 +133,8 @@ static const char *owner_vcard =
"END:VCARD\r\n";

static void phonebook_size_result(const char *buffer, size_t bufsize,
- int vcards, int missed, void *user_data)
+ int vcards, int missed,
+ gboolean lastpart, void *user_data)
{
struct irmc_session *irmc = user_data;

@@ -148,7 +149,7 @@ static void phonebook_size_result(const char *buffer, size_t bufsize,
}

static void query_result(const char *buffer, size_t bufsize, int vcards,
- int missed, void *user_data)
+ int missed, gboolean lastpart, void *user_data)
{
struct irmc_session *irmc = user_data;
const char *s, *t;
diff --git a/plugins/pbap.c b/plugins/pbap.c
index 6579d09..5775eea 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -148,6 +148,7 @@ struct pbap_object {
GString *buffer;
GByteArray *aparams;
gboolean firstpacket;
+ gboolean lastpart;
struct pbap_session *session;
void *request;
};
@@ -254,7 +255,8 @@ static GByteArray *append_aparam_header(GByteArray *buf, uint8_t tag,
}

static void phonebook_size_result(const char *buffer, size_t bufsize,
- int vcards, int missed, void *user_data)
+ int vcards, int missed,
+ gboolean lastpart, void *user_data)
{
struct pbap_session *pbap = user_data;
uint16_t phonebooksize;
@@ -286,17 +288,19 @@ static void phonebook_size_result(const char *buffer, size_t bufsize,
}

static void query_result(const char *buffer, size_t bufsize, int vcards,
- int missed, void *user_data)
+ int missed, gboolean lastpart, void *user_data)
{
struct pbap_session *pbap = user_data;

DBG("");

- if (pbap->obj->request) {
+ if (pbap->obj->request && lastpart) {
phonebook_req_finalize(pbap->obj->request);
pbap->obj->request = NULL;
}

+ pbap->obj->lastpart = lastpart;
+
if (vcards <= 0) {
obex_object_set_io_flags(pbap->obj, G_IO_ERR, -ENOENT);
return;
diff --git a/plugins/phonebook-dummy.c b/plugins/phonebook-dummy.c
index 60b7640..76dd550 100644
--- a/plugins/phonebook-dummy.c
+++ b/plugins/phonebook-dummy.c
@@ -248,7 +248,7 @@ static gboolean read_dir(void *user_data)
closedir(dp);
done:
/* FIXME: Missing vCards fields filtering */
- dummy->cb(buffer->str, buffer->len, count, 0, dummy->user_data);
+ dummy->cb(buffer->str, buffer->len, count, 0, TRUE, dummy->user_data);

g_string_free(buffer, TRUE);

@@ -346,7 +346,7 @@ static gboolean read_entry(void *user_data)

/* FIXME: Missing vCards fields filtering */

- dummy->cb(buffer, count, 1, 0, dummy->user_data);
+ dummy->cb(buffer, count, 1, 0, TRUE, dummy->user_data);

return FALSE;
}
diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
index 70b9c02..6cc4f31 100644
--- a/plugins/phonebook-ebook.c
+++ b/plugins/phonebook-ebook.c
@@ -186,7 +186,8 @@ static void ebookpull_cb(EBook *book, EBookStatus estatus, GList *contacts,

done:
data->completed = TRUE;
- data->contacts_cb(string->str, string->len, count, 0, data->user_data);
+ data->contacts_cb(string->str, string->len, count, 0, TRUE,
+ data->user_data);

fail:
g_string_free(string, TRUE);
@@ -212,7 +213,7 @@ static void ebook_entry_cb(EBook *book, EBookStatus estatus,

if (estatus != E_BOOK_ERROR_OK) {
error("E-Book query failed: status %d", estatus);
- data->contacts_cb(NULL, 0, 1, 0, data->user_data);
+ data->contacts_cb(NULL, 0, 1, 0, TRUE, data->user_data);
goto fail;
}

@@ -223,7 +224,7 @@ static void ebook_entry_cb(EBook *book, EBookStatus estatus,

len = vcard ? strlen(vcard) : 0;

- data->contacts_cb(vcard, len, 1, 0, data->user_data);
+ data->contacts_cb(vcard, len, 1, 0, TRUE, data->user_data);

g_free(vcard);

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index afd738c..0762787 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -1334,7 +1334,7 @@ static int pull_contacts_size(const char **reply, int num_fields,
struct phonebook_data *data = user_data;

if (num_fields < 0) {
- data->cb(NULL, 0, num_fields, 0, data->user_data);
+ data->cb(NULL, 0, num_fields, 0, TRUE, data->user_data);
return -EINTR;
}

@@ -1343,7 +1343,8 @@ static int pull_contacts_size(const char **reply, int num_fields,
return 0;
}

- data->cb(NULL, 0, data->index, data->newmissedcalls, data->user_data);
+ data->cb(NULL, 0, data->index, data->newmissedcalls, TRUE,
+ data->user_data);

return 0;
/*
@@ -1561,7 +1562,7 @@ static int pull_contacts(const char **reply, int num_fields, void *user_data)
static char *temp_id = NULL;

if (num_fields < 0) {
- data->cb(NULL, 0, num_fields, 0, data->user_data);
+ data->cb(NULL, 0, num_fields, 0, TRUE, data->user_data);
goto fail;
}

@@ -1641,7 +1642,7 @@ done:
vcards = gen_vcards(data->contacts, params);

data->cb(vcards->str, vcards->len, g_slist_length(data->contacts),
- data->newmissedcalls, data->user_data);
+ data->newmissedcalls, TRUE, data->user_data);

g_string_free(vcards, TRUE);
fail:
@@ -1868,7 +1869,7 @@ done:
data->contacts = NULL;

if (num_fields < 0) {
- data->cb(NULL, 0, num_fields, 0, data->user_data);
+ data->cb(NULL, 0, num_fields, 0, TRUE, data->user_data);
return -EINTR;
}

@@ -1884,7 +1885,7 @@ done:

err = query_tracker(query, col_amount, pull_cb, data);
if (err < 0) {
- data->cb(NULL, 0, err, 0, data->user_data);
+ data->cb(NULL, 0, err, 0, TRUE, data->user_data);

return -EINTR;
}
diff --git a/plugins/phonebook.h b/plugins/phonebook.h
index bfbae0f..f6df164 100644
--- a/plugins/phonebook.h
+++ b/plugins/phonebook.h
@@ -50,7 +50,7 @@ struct apparam_field {
* Contacts will be returned in the vcard format.
*/
typedef void (*phonebook_cb) (const char *buffer, size_t bufsize,
- int vcards, int missed, void *user_data);
+ int vcards, int missed, gboolean lastpart, void *user_data);

/*
* Interface between the PBAP core and backends to
--
1.7.0.4


2011-02-11 14:05:39

by Radoslaw Jablonski

[permalink] [raw]
Subject: [PATCH 7/7 v2] Support for multipart response sending from phonebook-tracker

Now data are being sent with smaller parts - vcard amount for
one part is defined in VCARDS_PART_COUNT. When one part of
data is sent, then data download from tracker is stopped.
It will be resumed when phonebook_pull_read will be called again.
This is needed to start sending data from PBAP quicker - now
transfer can be started when first part of data is processed.
Previously transfer was started when all results for response
were downloaded, and (for large responses) some PBAP clients
were disconnecting due to timeout.
---
plugins/phonebook-tracker.c | 81 +++++++++++++++++++++++++++++++++---------
1 files changed, 63 insertions(+), 18 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 48748f3..ab8d428 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -81,6 +81,8 @@
#define MAIN_DELIM "\30" /* Main delimiter between phones, addresses, emails*/
#define SUB_DELIM "\31" /* Delimiter used in telephone number strings*/
#define MAX_FIELDS 100 /* Max amount of fields to be concatenated at once*/
+#define VCARDS_PART_COUNT 50 /* amount of vcards sent at once to PBAP core */
+#define QUERY_OFFSET_FORMAT "%s OFFSET %d"

#define CONTACTS_QUERY_ALL \
"SELECT " \
@@ -919,6 +921,8 @@ struct phonebook_data {
int newmissedcalls;
GCancellable *query_canc;
char *req_name;
+ int vcard_part_count;
+ int tracker_index;
};

struct phonebook_index {
@@ -1551,15 +1555,45 @@ static void contact_add_organization(struct phonebook_contact *contact,
add_affiliation(&contact->role, reply[COL_ORG_ROLE]);
}

+static void free_data_contacts(struct phonebook_data *data)
+{
+ GSList *l;
+
+ /* freeing contacts */
+ for (l = data->contacts; l; l = l->next) {
+ struct contact_data *c_data = l->data;
+
+ g_free(c_data->id);
+ phonebook_contact_free(c_data->contact);
+ g_free(c_data);
+ }
+
+ g_slist_free(data->contacts);
+ data->contacts = NULL;
+}
+
+static void send_pull_part(struct phonebook_data *data,
+ const struct apparam_field *params, gboolean lastpart)
+{
+ GString *vcards;
+
+ DBG("");
+ vcards = gen_vcards(data->contacts, params);
+ data->cb(vcards->str, vcards->len, g_slist_length(data->contacts),
+ data->newmissedcalls, lastpart, data->user_data);
+
+ free_data_contacts(data);
+ g_string_free(vcards, TRUE);
+}
+
static int pull_contacts(const char **reply, int num_fields, void *user_data)
{
struct phonebook_data *data = user_data;
const struct apparam_field *params = data->params;
struct phonebook_contact *contact;
struct contact_data *contact_data;
- GString *vcards;
int last_index, i;
- gboolean cdata_present = FALSE;
+ gboolean cdata_present = FALSE, part_sent = FALSE;
static char *temp_id = NULL;

if (num_fields < 0) {
@@ -1568,6 +1602,7 @@ static int pull_contacts(const char **reply, int num_fields, void *user_data)
}

DBG("reply %p", reply);
+ data->tracker_index++;

if (reply == NULL)
goto done;
@@ -1600,6 +1635,25 @@ static int pull_contacts(const char **reply, int num_fields, void *user_data)
data->index++;
g_free(temp_id);
temp_id = g_strdup(reply[CONTACTS_ID_COL]);
+
+ /* Incrementing counter for vcards in current part of data,
+ * but only if liststartoffset has been already reached */
+ if (data->index > params->liststartoffset)
+ data->vcard_part_count++;
+ }
+
+ if (data->vcard_part_count > VCARDS_PART_COUNT) {
+ DBG("Part of vcard data ready for sending...");
+ data->vcard_part_count = 0;
+ /* Sending part of data to PBAP core - more data can be still
+ * fetched, so marking lastpart as FALSE */
+ send_pull_part(data, params, FALSE);
+
+ /* Later, after adding contact data, need to return -EINTR to
+ * stop fetching more data for this request. Data will be
+ * downloaded again from this point, when phonebook_pull_read
+ * will be called again with current request as a parameter*/
+ part_sent = TRUE;
}

last_index = params->liststartoffset + params->maxlistcount;
@@ -1637,15 +1691,16 @@ add_numbers:
data->contacts = g_slist_append(data->contacts, contact_data);
}

+ if (part_sent)
+ return -EINTR;
+
return 0;

done:
- vcards = gen_vcards(data->contacts, params);
+ /* Processing is end, this is definitely last part of transmission
+ * (marking lastpart as TRUE) */
+ send_pull_part(data, params, TRUE);

- data->cb(vcards->str, vcards->len, g_slist_length(data->contacts),
- data->newmissedcalls, TRUE, data->user_data);
-
- g_string_free(vcards, TRUE);
fail:
g_free(temp_id);
temp_id = NULL;
@@ -1797,7 +1852,6 @@ done:
void phonebook_req_finalize(void *request)
{
struct phonebook_data *data = request;
- GSList *l;

DBG("");

@@ -1810,16 +1864,7 @@ void phonebook_req_finalize(void *request)
g_object_unref(data->query_canc);
}

- /* freeing contacts */
- for (l = data->contacts; l; l = l->next) {
- struct contact_data *c_data = l->data;
-
- g_free(c_data->id);
- phonebook_contact_free(c_data->contact);
- g_free(c_data);
- }
-
- g_slist_free(data->contacts);
+ free_data_contacts(data);
g_free(data->req_name);
g_free(data);
}
--
1.7.0.4


2011-02-11 14:05:36

by Radoslaw Jablonski

[permalink] [raw]
Subject: [PATCH 4/7 v2] Support for stop fetching data when maxlistcount is achieved

When desired number of results has been already processed,
then sending already collected data and stopping fetching
more results from tracker (previously in this scenario
unnecessarily processing data continued untill end of results
in db)
---
plugins/phonebook-tracker.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index be94867..afd738c 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -1602,11 +1602,17 @@ static int pull_contacts(const char **reply, int num_fields, void *user_data)

last_index = params->liststartoffset + params->maxlistcount;

- if ((data->index <= params->liststartoffset ||
- data->index > last_index) &&
- params->maxlistcount > 0)
+ if (data->index <= params->liststartoffset)
return 0;

+ /* max number of results achieved - need send vcards data that was
+ * already collected and stop further data processing (these operations
+ * will be invoked in "done" section) */
+ if (data->index > last_index && params->maxlistcount > 0) {
+ DBG("Maxlistcount achieved");
+ goto done;
+ }
+
add_entry:
contact = g_new0(struct phonebook_contact, 1);
contact_init(contact, reply);
--
1.7.0.4


2011-02-11 14:05:35

by Radoslaw Jablonski

[permalink] [raw]
Subject: [PATCH 3/7 v2] Remove unnecessary checking for num_fields in pull_contact

This check is no longer needed - it was used to ensure that no
error occured in querying tracker. This is redundant because
checking for that case is located in the beginning of pull_contacts
function.
---
plugins/phonebook-tracker.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index a8c8a87..be94867 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -1634,9 +1634,7 @@ add_numbers:
done:
vcards = gen_vcards(data->contacts, params);

- if (num_fields == 0)
- data->cb(vcards->str, vcards->len,
- g_slist_length(data->contacts),
+ data->cb(vcards->str, vcards->len, g_slist_length(data->contacts),
data->newmissedcalls, data->user_data);

g_string_free(vcards, TRUE);
--
1.7.0.4


2011-02-11 14:05:38

by Radoslaw Jablonski

[permalink] [raw]
Subject: [PATCH 6/7 v2] Introduction of phonebook_pull_read

Previosly reading from backend was initialized in phonebook_pull.
Now phonebook_pull should be used only for preparing request data
and phonebook_pull_read for 'real' reading vcards data from back-end.
The back-end can return data in one response or it can return data in
many parts. After obtaining one part, PBAP core need to call
phonebook_pull_read with the same request again to get more results.
Using that, PBAP core has control of its the buffer size - it can
ask for new parts of data when buffer is empty or when its size
will be lower than some level.
---
plugins/irmc.c | 14 +++++++++++
plugins/pbap.c | 25 +++++++++++++++++++-
plugins/phonebook-dummy.c | 27 ++++++++++++++++-----
plugins/phonebook-ebook.c | 23 +++++++++++++-----
plugins/phonebook-tracker.c | 52 ++++++++++++++++++++++++++----------------
plugins/phonebook.h | 20 +++++++++++++---
6 files changed, 122 insertions(+), 39 deletions(-)

diff --git a/plugins/irmc.c b/plugins/irmc.c
index e1e83f9..68aa6e2 100644
--- a/plugins/irmc.c
+++ b/plugins/irmc.c
@@ -197,6 +197,7 @@ static void *irmc_connect(struct obex_session *os, int *err)
{
struct irmc_session *irmc;
struct apparam_field *param;
+ int ret;

DBG("");

@@ -224,6 +225,11 @@ static void *irmc_connect(struct obex_session *os, int *err)
irmc->params = param;
irmc->request = phonebook_pull("telecom/pb.vcf", irmc->params,
phonebook_size_result, irmc, err);
+ ret = phonebook_pull_read(irmc->request);
+
+ if (err) {
+ *err = ret;
+ }

return irmc;
}
@@ -313,6 +319,14 @@ static void *irmc_open_pb(const char *name, struct irmc_session *irmc,
DBG("phonebook_pull failed...");
goto fail;
}
+
+ ret = phonebook_pull_read(irmc->request);
+
+ if (ret < 0) {
+ DBG("phonebook_pull_read failed...");
+ goto fail;
+ }
+
return irmc;
}

diff --git a/plugins/pbap.c b/plugins/pbap.c
index 5775eea..33e40b4 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -806,6 +806,12 @@ static void *vobject_pull_open(const char *name, int oflag, mode_t mode,
if (ret < 0)
goto fail;

+ /* reading first part of results from backend */
+ ret = phonebook_pull_read(request);
+
+ if (ret < 0)
+ goto fail;
+
if (err)
*err = 0;

@@ -962,6 +968,7 @@ static ssize_t vobject_pull_read(void *object, void *buf, size_t count,
{
struct pbap_object *obj = object;
struct pbap_session *pbap = obj->session;
+ int len, ret;

DBG("buffer %p maxlistcount %d", obj->buffer,
pbap->params->maxlistcount);
@@ -987,7 +994,23 @@ static ssize_t vobject_pull_read(void *object, void *buf, size_t count,
*hi = OBEX_HDR_BODY;
if (flags)
*flags = 0;
- return string_read(obj->buffer, buf, count);
+
+ len = string_read(obj->buffer, buf, count);
+
+ if (len == 0 && !obj->lastpart) {
+ /* in case when buffer is empty and we know that more
+ * data is still available in backend, requesting new
+ * data part via phonebook_pull_read and returning
+ * -EAGAIN to suspend request for now */
+ ret = phonebook_pull_read(obj->request);
+
+ if (ret)
+ return -EPERM;
+
+ return -EAGAIN;
+ }
+
+ return len;
}
}

diff --git a/plugins/phonebook-dummy.c b/plugins/phonebook-dummy.c
index 76dd550..532c921 100644
--- a/plugins/phonebook-dummy.c
+++ b/plugins/phonebook-dummy.c
@@ -52,6 +52,7 @@ struct dummy_data {
const struct apparam_field *apparams;
char *folder;
int fd;
+ guint id;
};

struct cache_query {
@@ -449,9 +450,12 @@ done:

void phonebook_req_finalize(void *request)
{
- guint id = GPOINTER_TO_INT(request);
+ struct dummy_data *dummy = request;

- g_source_remove(id);
+ /* dummy_data will be cleaned when request will be finished via
+ * g_source_remove */
+ if (dummy && dummy->id)
+ g_source_remove(dummy->id);
}

void *phonebook_pull(const char *name, const struct apparam_field *params,
@@ -459,7 +463,6 @@ void *phonebook_pull(const char *name, const struct apparam_field *params,
{
struct dummy_data *dummy;
char *filename, *folder;
- guint ret;

/*
* Main phonebook objects will be created dinamically based on the
@@ -492,13 +495,23 @@ void *phonebook_pull(const char *name, const struct apparam_field *params,
dummy->folder = folder;
dummy->fd = -1;

- ret = g_idle_add_full(G_PRIORITY_DEFAULT_IDLE, read_dir, dummy,
- dummy_free);
-
if (err)
*err = 0;

- return GINT_TO_POINTER(ret);
+ return dummy;
+}
+
+int phonebook_pull_read(void *request)
+{
+ struct dummy_data *dummy = request;
+
+ if (!dummy)
+ return -ENOENT;
+
+ dummy->id = g_idle_add_full(G_PRIORITY_DEFAULT_IDLE, read_dir, dummy,
+ dummy_free);
+
+ return 0;
}

void *phonebook_get_entry(const char *folder, const char *id,
diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
index 6cc4f31..82019da 100644
--- a/plugins/phonebook-ebook.c
+++ b/plugins/phonebook-ebook.c
@@ -453,25 +453,34 @@ void *phonebook_pull(const char *name, const struct apparam_field *params,
phonebook_cb cb, void *user_data, int *err)
{
struct query_context *data;
- EBookQuery *query;
-
- query = e_book_query_any_field_contains("");

data = g_new0(struct query_context, 1);
data->contacts_cb = cb;
data->params = params;
data->user_data = user_data;

- e_book_async_get_contacts(ebook, query, ebookpull_cb, data);
-
- e_book_query_unref(query);
-
if (err)
*err = 0;

return data;
}

+int phonebook_pull_read(void *request)
+{
+ struct query_context *data = request;
+ EBookQuery *query;
+
+ if (!data)
+ return -ENOENT;
+
+ query = e_book_query_any_field_contains("");
+ e_book_async_get_contacts(ebook, query, ebookpull_cb, data);
+
+ e_book_query_unref(query);
+
+ return 0;
+}
+
void *phonebook_get_entry(const char *folder, const char *id,
const struct apparam_field *params,
phonebook_cb cb, void *user_data, int *err)
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 0762787..48748f3 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -918,6 +918,7 @@ struct phonebook_data {
phonebook_entry_cb entry_cb;
int newmissedcalls;
GCancellable *query_canc;
+ char *req_name;
};

struct phonebook_index {
@@ -1819,6 +1820,7 @@ void phonebook_req_finalize(void *request)
}

g_slist_free(data->contacts);
+ g_free(data->req_name);
g_free(data);
}

@@ -1897,42 +1899,52 @@ void *phonebook_pull(const char *name, const struct apparam_field *params,
phonebook_cb cb, void *user_data, int *err)
{
struct phonebook_data *data;
- const char *query;
- reply_list_foreach_t pull_cb;
- int col_amount, ret;

DBG("name %s", name);

- if (g_strcmp0(name, "telecom/mch.vcf") == 0) {
+ data = g_new0(struct phonebook_data, 1);
+ data->params = params;
+ data->user_data = user_data;
+ data->cb = cb;
+ data->req_name = g_strdup(name);
+
+ if (err)
+ *err = 0;
+
+ return data;
+}
+
+int phonebook_pull_read(void *request)
+{
+ struct phonebook_data *data = request;
+ reply_list_foreach_t pull_cb;
+ const char *query;
+ int col_amount;
+ int ret;
+
+ if(!data)
+ return -ENOENT;
+
+ if (g_strcmp0(data->req_name, "telecom/mch.vcf") == 0) {
query = NEW_MISSED_CALLS_LIST;
col_amount = PULL_QUERY_COL_AMOUNT;
pull_cb = pull_newmissedcalls;
- } else if (params->maxlistcount == 0) {
- query = name2count_query(name);
+ } else if (data->params->maxlistcount == 0) {
+ query = name2count_query(data->req_name);
col_amount = COUNT_QUERY_COL_AMOUNT;
pull_cb = pull_contacts_size;
} else {
- query = name2query(name);
+ query = name2query(data->req_name);
col_amount = PULL_QUERY_COL_AMOUNT;
pull_cb = pull_contacts;
}

- if (query == NULL) {
- if (err)
- *err = -ENOENT;
- return NULL;
- }
+ if (query == NULL)
+ return -ENOENT;

- data = g_new0(struct phonebook_data, 1);
- data->params = params;
- data->user_data = user_data;
- data->cb = cb;
ret = query_tracker(query, col_amount, pull_cb, data);

- if(err)
- *err = ret;
-
- return data;
+ return ret;
}

void *phonebook_get_entry(const char *folder, const char *id,
diff --git a/plugins/phonebook.h b/plugins/phonebook.h
index f6df164..00abc08 100644
--- a/plugins/phonebook.h
+++ b/plugins/phonebook.h
@@ -82,17 +82,29 @@ char *phonebook_set_folder(const char *current_folder,
const char *new_folder, uint8_t flags, int *err);

/*
- * PullPhoneBook never use cached entries. PCE use this function to get all
- * entries of a given folder. The back-end MUST return only the content based
- * on the application parameters requested by the client.
+ * phonebook_pull should be used only to prepare pull request - prepared
+ * request data is returned by this function. Start of fetching data from
+ * back-end will be done only after calling phonebook_pull_read with this
+ * returned request given as a parameter.
*
- * Return value is a pointer to asynchronous request to phonebook back-end.
* phonebook_req_finalize MUST always be used to free associated resources.
*/
void *phonebook_pull(const char *name, const struct apparam_field *params,
phonebook_cb cb, void *user_data, int *err);

/*
+ * phonebook_pull_read should be used to start getting results from back-end.
+ * The back-end can return data as one response or can return it many parts.
+ * After obtaining one part, PBAP core need to call phonebook_pull_read with
+ * the same request again to get more results from back-end.
+ * The back-end MUST return only the content based on the application
+ * parameters requested by the client.
+ *
+ * Returns error code or 0 in case of success
+ */
+int phonebook_pull_read(void *request);
+
+/*
* Function used to retrieve a contact from the backend. Only contacts
* found in the cache are requested to the back-ends. The back-end MUST
* return only the content based on the application parameters requested
--
1.7.0.4