2011-08-09 23:05:39

by Bartosz Szatkowski

[permalink] [raw]
Subject: [PATCH obexd 1/2] Fix memory issues in EDS PBAP

---
plugins/phonebook-ebook.c | 19 +++++++++++++------
1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
index e53da12..903cd01 100644
--- a/plugins/phonebook-ebook.c
+++ b/plugins/phonebook-ebook.c
@@ -343,10 +343,11 @@ done:
}

static GSList *traverse_sources(GSList *ebooks, GSList *sources,
- char *default_src) {
+ char **default_src) {
GError *gerr;

while (sources != NULL) {
+ char *uri;
EBook *ebook = e_book_new(E_SOURCE(sources->data), &gerr);
if (ebook == NULL) {
error("Can't create user's address book: %s",
@@ -357,12 +358,14 @@ static GSList *traverse_sources(GSList *ebooks, GSList *sources,
continue;
}

- if (g_strcmp0(default_src, e_source_get_uri(
- E_SOURCE(sources->data))) == 0) {
+ uri = e_source_get_uri(E_SOURCE(sources->data));
+ if (g_strcmp0(*default_src, uri) == 0) {
sources = sources->next;
+ g_free(uri);

continue;
}
+ g_free(uri);

if (e_book_open(ebook, FALSE, &gerr) == FALSE) {
error("Can't open e-book address book: %s",
@@ -374,8 +377,9 @@ static GSList *traverse_sources(GSList *ebooks, GSList *sources,
continue;
}

- if (default_src == NULL)
- default_src = e_source_get_uri(E_SOURCE(sources->data));
+ if (*default_src == NULL)
+ *default_src = e_source_get_uri(
+ E_SOURCE(sources->data));

DBG("%s address book opened",
e_source_peek_name(sources->data));
@@ -416,11 +420,14 @@ static GSList *open_ebooks(void)

GSList *sources = e_source_group_peek_sources(group);

- ebooks = traverse_sources(ebooks, sources, default_src);
+ ebooks = traverse_sources(ebooks, sources, &default_src);

list = list->next;
}

+ g_free(default_src);
+ g_object_unref(src_list);
+
return ebooks;
}

--
1.7.6



2011-08-10 09:31:32

by Bartosz Szatkowski

[permalink] [raw]
Subject: Re: [PATCH obexd 1/2] Fix memory issues in EDS PBAP

On Wed, Aug 10, 2011 at 11:30 AM, Johan Hedberg <[email protected]> wrote:
> Hi Bartosz,
>
> On Wed, Aug 10, 2011, Bartosz Szatkowski wrote:
>> ---
>>  plugins/phonebook-ebook.c |   19 +++++++++++++------
>>  1 files changed, 13 insertions(+), 6 deletions(-)
>
> Both patches have been applied. Thanks.
>
> I additionally pushed a cleanup patch to phonebook-ebook.c since there
> were quite a lot of coding style issues in it as well as GError misuse
> (you always need to initialize them to NULL).
>
> Johan
>

Thanks.

--
Pozdrowienia,
Bartosz Szatkowski

2011-08-10 09:30:29

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH obexd 1/2] Fix memory issues in EDS PBAP

Hi Bartosz,

On Wed, Aug 10, 2011, Bartosz Szatkowski wrote:
> ---
> plugins/phonebook-ebook.c | 19 +++++++++++++------
> 1 files changed, 13 insertions(+), 6 deletions(-)

Both patches have been applied. Thanks.

I additionally pushed a cleanup patch to phonebook-ebook.c since there
were quite a lot of coding style issues in it as well as GError misuse
(you always need to initialize them to NULL).

Johan

2011-08-09 23:05:40

by Bartosz Szatkowski

[permalink] [raw]
Subject: [PATCH obexd 2/2] Fix callback logic in EDS PBAP

It seems that e_book_cancel (in libebook) is broken -- it should return
TRUE when request is canceled and FALSE otherwise, but after
"successfully" canceling async request, supplied callback is still
called without any error.
---
plugins/phonebook-ebook.c | 56 ++++++++++++++++++++++++++++++--------------
1 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
index 903cd01..d0e08a5 100644
--- a/plugins/phonebook-ebook.c
+++ b/plugins/phonebook-ebook.c
@@ -58,6 +58,7 @@ struct query_context {
unsigned queued_calls;
void *user_data;
GSList *ebooks;
+ gboolean canceled;
};

static char *attribute_mask[] = {
@@ -167,6 +168,11 @@ static void ebookpull_cb(EBook *book, const GError *gerr, GList *contacts,
GList *l;
unsigned int count, maxcount;

+ data->queued_calls--;
+
+ if (data->canceled)
+ goto canceled;
+
if (gerr != NULL) {
error("E-Book query failed: %s", gerr->message);
goto done;
@@ -205,10 +211,9 @@ static void ebookpull_cb(EBook *book, const GError *gerr, GList *contacts,

data->count += count;

-done:
g_list_free_full(contacts, g_object_unref);

- data->queued_calls--;
+done:
if (data->queued_calls == 0) {
GString *buf = data->buf;
data->buf = NULL;
@@ -217,7 +222,14 @@ done:
0, TRUE, data->user_data);

g_string_free(buf, TRUE);
+
}
+
+ return;
+
+canceled:
+ if (data->queued_calls == 0)
+ free_query_context(data);
}

static void ebook_entry_cb(EBook *book, const GError *gerr,
@@ -228,6 +240,11 @@ static void ebook_entry_cb(EBook *book, const GError *gerr,
char *vcard;
size_t len;

+ data->queued_calls--;
+
+ if (data->canceled)
+ goto done;
+
if (gerr != NULL) {
error("E-Book query failed: %s", gerr->message);
goto done;
@@ -248,14 +265,15 @@ static void ebook_entry_cb(EBook *book, const GError *gerr,
g_free(vcard);
g_object_unref(contact);

+ return;
+
done:
- data->queued_calls--;
if (data->queued_calls == 0) {
if (data->count == 0)
data->contacts_cb(NULL, 0, 1, 0, TRUE,
data->user_data);
-
- free_query_context(data);
+ else if (data->canceled)
+ free_query_context(data);
}
}

@@ -295,6 +313,11 @@ static void cache_cb(EBook *book, const GError *gerr, GList *contacts,
struct query_context *data = user_data;
GList *l;

+ data->queued_calls--;
+
+ if (data->canceled)
+ goto canceled;
+
if (gerr != NULL) {
error("E-Book operation failed: %s", gerr->message);
goto done;
@@ -334,12 +357,17 @@ static void cache_cb(EBook *book, const GError *gerr, GList *contacts,
g_free(tel);
}

-done:
g_list_free_full(contacts, g_object_unref);

- data->queued_calls--;
+done:
if (data->queued_calls == 0)
data->ready_cb(data->user_data);
+
+ return;
+
+canceled:
+ if (data->queued_calls == 0)
+ free_query_context(data);
}

static GSList *traverse_sources(GSList *ebooks, GSList *sources,
@@ -522,19 +550,11 @@ done:
void phonebook_req_finalize(void *request)
{
struct query_context *data = request;
- GSList *ebook = data->ebooks;

- DBG("");
-
- while (ebook != NULL) {
- if (e_book_cancel(ebook->data, NULL) == TRUE)
- data->queued_calls--;
-
- ebook = ebook->next;
- }
-
- if (data != NULL && data->queued_calls == 0)
+ if (data->queued_calls == 0)
free_query_context(data);
+ else
+ data->canceled = TRUE;
}

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