[PATCH obexd 1/5] Fix pulling vCard for multiple ebooks

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH obexd 1/5] Fix pulling vCard for multiple ebooks
@ 2011-07-13 22:10 Bartosz Szatkowski
  2011-07-13 22:10 ` [PATCH obexd 2/5] Fix ebook opening error handling Bartosz Szatkowski
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Bartosz Szatkowski @ 2011-07-13 22:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bartosz Szatkowski

Problem occurred when there were multiple ebooks and the last one was
empty. Reported vCard number was 0 and no data was transported.
---
 plugins/phonebook-ebook.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
index b51d34d..b2f16e0 100644
--- a/plugins/phonebook-ebook.c
+++ b/plugins/phonebook-ebook.c
@@ -51,7 +51,7 @@ struct query_context {
 	phonebook_entry_cb entry_cb;
 	phonebook_cache_ready_cb ready_cb;
 	EBookQuery *query;
-	int count;
+	unsigned int count;
 	GString *buf;
 	char *id;
 	unsigned queued_calls;
@@ -158,7 +158,7 @@ static void ebookpull_cb(EBook *book, const GError *gerr, GList *contacts,
 {
 	struct query_context *data = user_data;
 	GList *l;
-	unsigned int count = data->count, maxcount;
+	unsigned int count = 0, maxcount;
 
 	if (gerr != NULL) {
 		error("E-Book query failed: %s", gerr->message);
@@ -180,9 +180,8 @@ static void ebookpull_cb(EBook *book, const GError *gerr, GList *contacts,
 
 	l = g_list_nth(contacts, data->params->liststartoffset);
 
-	/* FIXME: Missing 0.vcf */
-
-	for (; l && count < maxcount; l = g_list_next(l), count++) {
+	for (; l && count + data->count < maxcount; l = g_list_next(l),
+								count++) {
 		EContact *contact = E_CONTACT(l->data);
 		EVCard *evcard = E_VCARD(contact);
 		char *vcard;
@@ -195,6 +194,8 @@ static void ebookpull_cb(EBook *book, const GError *gerr, GList *contacts,
 		g_free(vcard);
 	}
 
+	data->count += count;
+
 done:
 	g_list_free_full(contacts, g_object_unref);
 
@@ -202,8 +203,8 @@ done:
 
 	data->queued_calls--;
 	if (data->queued_calls == 0)
-		data->contacts_cb(data->buf->str, data->buf->len, count, 0,
-							TRUE, data->user_data);
+		data->contacts_cb(data->buf->str, data->buf->len, data->count,
+						0, TRUE, data->user_data);
 }
 
 static void ebook_entry_cb(EBook *book, const GError *gerr,
-- 
1.7.5.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH obexd 2/5] Fix ebook opening error handling
  2011-07-13 22:10 [PATCH obexd 1/5] Fix pulling vCard for multiple ebooks Bartosz Szatkowski
@ 2011-07-13 22:10 ` Bartosz Szatkowski
  2011-07-13 22:10 ` [PATCH obexd 3/5] Add opening ebooks before each operation Bartosz Szatkowski
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Bartosz Szatkowski @ 2011-07-13 22:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bartosz Szatkowski

Individual address book may be not accessible (offline, not
authenticated etc.) it should not prevent plugin to load properly.
---
 plugins/phonebook-ebook.c |   21 ++++++++-------------
 1 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
index b2f16e0..0b2878a 100644
--- a/plugins/phonebook-ebook.c
+++ b/plugins/phonebook-ebook.c
@@ -331,16 +331,16 @@ done:
 
 static int traverse_sources(GSList *sources, char *default_src) {
 	GError *gerr;
-	int status;
 
 	while (sources != NULL) {
 		EBook *ebook = e_book_new(E_SOURCE(sources->data), &gerr);
 		if (ebook == NULL) {
 			error("Can't create user's address book: %s",
 								gerr->message);
+			sources = sources->next;
 
-			status = -EIO;
-			goto fail;
+			g_error_free(gerr);
+			continue;
 		}
 
 		if (g_strcmp0(default_src, e_source_get_uri(
@@ -353,9 +353,11 @@ static int traverse_sources(GSList *sources, char *default_src) {
 		if (e_book_open(ebook, FALSE, &gerr) == FALSE) {
 			error("Can't open e-book address book: %s",
 							gerr->message);
+			sources = sources->next;
 
-			status = -EIO;
-			goto fail;
+			g_object_unref(ebook);
+			g_error_free(gerr);
+			continue;
 		}
 
 		if (default_src == NULL)
@@ -370,11 +372,6 @@ static int traverse_sources(GSList *sources, char *default_src) {
 	}
 
 	return 0;
-
-fail:
-	g_error_free(gerr);
-
-	return status;
 }
 
 int phonebook_init(void)
@@ -404,9 +401,7 @@ int phonebook_init(void)
 
 		GSList *sources = e_source_group_peek_sources(group);
 
-		status = traverse_sources(sources, default_src);
-		if (status != 0)
-			goto fail;
+		traverse_sources(sources, default_src);
 
 		list = list->next;
 	}
-- 
1.7.5.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH obexd 3/5] Add opening ebooks before each operation
  2011-07-13 22:10 [PATCH obexd 1/5] Fix pulling vCard for multiple ebooks Bartosz Szatkowski
  2011-07-13 22:10 ` [PATCH obexd 2/5] Fix ebook opening error handling Bartosz Szatkowski
@ 2011-07-13 22:10 ` Bartosz Szatkowski
  2011-07-13 22:10 ` [PATCH obexd 4/5] Fix no phone number in vCard listing Bartosz Szatkowski
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Bartosz Szatkowski @ 2011-07-13 22:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bartosz Szatkowski

Until now ebooks were opened only on phonebook_init, but particular ebook
may be offline or not authenticated or user may be offline at the time.
Better idea is try to open ebooks before each operation, especially
as time overhead is minimal.
---
 plugins/phonebook-ebook.c |   66 +++++++++++++++++++++------------------------
 1 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
index 0b2878a..2ee5473 100644
--- a/plugins/phonebook-ebook.c
+++ b/plugins/phonebook-ebook.c
@@ -56,10 +56,9 @@ struct query_context {
 	char *id;
 	unsigned queued_calls;
 	void *user_data;
+	GSList *ebooks;
 };
 
-static GSList *ebooks = NULL;
-
 static char *attribute_mask[] = {
 /* 0 */		"VERSION",
 		"FN",
@@ -94,6 +93,11 @@ static char *attribute_mask[] = {
 
 };
 
+static void close_ebooks(GSList *ebooks)
+{
+	g_slist_free_full(ebooks, g_object_unref);
+}
+
 static void free_query_context(struct query_context *data)
 {
 	g_free(data->id);
@@ -104,6 +108,8 @@ static void free_query_context(struct query_context *data)
 	if (data->query != NULL)
 		e_book_query_unref(data->query);
 
+	close_ebooks(data->ebooks);
+
 	g_free(data);
 }
 
@@ -329,7 +335,8 @@ done:
 		data->ready_cb(data->user_data);
 }
 
-static int traverse_sources(GSList *sources, char *default_src) {
+static GSList *traverse_sources(GSList *ebooks, GSList *sources,
+							char *default_src) {
 	GError *gerr;
 
 	while (sources != NULL) {
@@ -371,28 +378,29 @@ static int traverse_sources(GSList *sources, char *default_src) {
 		sources = sources->next;
 	}
 
-	return 0;
+	return ebooks;
 }
 
 int phonebook_init(void)
 {
+	g_type_init();
+
+	return 0;
+}
+
+static GSList *open_ebooks(void)
+{
 	GError *gerr;
 	ESourceList *src_list;
 	GSList *list;
 	gchar *default_src = NULL;
-	int status = 0;
-
-	if (ebooks)
-		return 0;
-
-	g_type_init();
+	GSList *ebooks = NULL;
 
 	if (e_book_get_addressbooks(&src_list, &gerr) == FALSE) {
 		error("Can't list user's address books: %s", gerr->message);
 		g_error_free(gerr);
 
-		status = -EIO;
-		goto fail;
+		return NULL;
 	}
 
 	list = e_source_list_peek_groups(src_list);
@@ -401,29 +409,16 @@ int phonebook_init(void)
 
 		GSList *sources = e_source_group_peek_sources(group);
 
-		traverse_sources(sources, default_src);
+		ebooks = traverse_sources(ebooks, sources, default_src);
 
 		list = list->next;
 	}
 
-	return status;
-
-fail:
-	g_slist_free_full(ebooks, g_object_unref);
-	g_object_unref(src_list);
-
-	return status;
+	return ebooks;
 }
 
 void phonebook_exit(void)
 {
-	DBG("");
-
-	if (ebooks == NULL)
-		return;
-
-	g_slist_free_full(ebooks, g_object_unref);
-	ebooks = NULL;
 }
 
 char *phonebook_set_folder(const char *current_folder,
@@ -513,7 +508,7 @@ done:
 void phonebook_req_finalize(void *request)
 {
 	struct query_context *data = request;
-	GSList *ebook = ebooks;
+	GSList *ebook = data->ebooks;
 
 	DBG("");
 
@@ -545,9 +540,11 @@ void *phonebook_pull(const char *name, const struct apparam_field *params,
 	data->params = params;
 	data->user_data = user_data;
 	data->buf = g_string_new("");
+	data->query = e_book_query_any_field_contains("");
+	data->ebooks = open_ebooks();
 
 	if (err)
-		*err = 0;
+		*err = data->ebooks == NULL ? -EIO : 0;
 
 	return data;
 }
@@ -561,9 +558,7 @@ int phonebook_pull_read(void *request)
 	if (!data)
 		return -ENOENT;
 
-	data->query = e_book_query_any_field_contains("");
-
-	ebook = ebooks;
+	ebook = data->ebooks;
 	while (ebook != NULL) {
 		if (e_book_is_opened(ebook->data) == TRUE) {
 			ret = e_book_get_contacts_async(ebook->data,
@@ -594,8 +589,9 @@ void *phonebook_get_entry(const char *folder, const char *id,
 	data->params = params;
 	data->user_data = user_data;
 	data->id = g_strdup(id);
+	data->ebooks = open_ebooks();
 
-	ebook = ebooks;
+	ebook = data->ebooks;
 	while (ebook != NULL) {
 		if (e_book_is_opened(ebook->data) == TRUE) {
 			ret = e_book_get_contact_async(ebook->data, data->id,
@@ -627,7 +623,6 @@ void *phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
 	EVCardAttribute *attrib;
 	char *uid, *tel, *cname;
 
-
 	if (g_strcmp0("/telecom/pb", name) != 0) {
 		if (err)
 			*err = -ENOENT;
@@ -644,6 +639,7 @@ void *phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
 	data->ready_cb = ready_cb;
 	data->user_data = user_data;
 	data->query = query;
+	data->ebooks = open_ebooks();
 
 	/* Add 0.vcf */
 	if (e_book_get_self(&me, &eb, &gerr) == FALSE) {
@@ -678,7 +674,7 @@ void *phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
 	g_object_unref(eb);
 
 next:
-	ebook = ebooks;
+	ebook = data->ebooks;
 	while (ebook != NULL) {
 		if (e_book_is_opened(ebook->data) == TRUE) {
 			ret = e_book_get_contacts_async(ebook->data, query,
-- 
1.7.5.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH obexd 4/5] Fix no phone number in vCard listing
  2011-07-13 22:10 [PATCH obexd 1/5] Fix pulling vCard for multiple ebooks Bartosz Szatkowski
  2011-07-13 22:10 ` [PATCH obexd 2/5] Fix ebook opening error handling Bartosz Szatkowski
  2011-07-13 22:10 ` [PATCH obexd 3/5] Add opening ebooks before each operation Bartosz Szatkowski
@ 2011-07-13 22:10 ` Bartosz Szatkowski
  2011-07-13 22:10 ` [PATCH obexd 5/5] Fix freeing buffer before transfer is complete Bartosz Szatkowski
  2011-07-14 13:45 ` [PATCH obexd 1/5] Fix pulling vCard for multiple ebooks Johan Hedberg
  4 siblings, 0 replies; 6+ messages in thread
From: Bartosz Szatkowski @ 2011-07-13 22:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bartosz Szatkowski

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

diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
index 2ee5473..06ae1ec 100644
--- a/plugins/phonebook-ebook.c
+++ b/plugins/phonebook-ebook.c
@@ -314,7 +314,7 @@ static void cache_cb(EBook *book, const GError *gerr, GList *contacts,
 			continue;
 
 		attrib = e_vcard_get_attribute(evcard, EVC_TEL);
-		if (!attrib)
+		if (attrib)
 			tel = e_vcard_attribute_get_value(attrib);
 		else
 			tel = g_strdup("");
-- 
1.7.5.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH obexd 5/5] Fix freeing buffer before transfer is complete
  2011-07-13 22:10 [PATCH obexd 1/5] Fix pulling vCard for multiple ebooks Bartosz Szatkowski
                   ` (2 preceding siblings ...)
  2011-07-13 22:10 ` [PATCH obexd 4/5] Fix no phone number in vCard listing Bartosz Szatkowski
@ 2011-07-13 22:10 ` Bartosz Szatkowski
  2011-07-14 13:45 ` [PATCH obexd 1/5] Fix pulling vCard for multiple ebooks Johan Hedberg
  4 siblings, 0 replies; 6+ messages in thread
From: Bartosz Szatkowski @ 2011-07-13 22:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bartosz Szatkowski

phonebook_req_finalize is called before actual transport taking place,
so buffers kept in user_data may cause invalid reads in valgrind and
prevent transport being completed.
---
 plugins/phonebook-ebook.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
index 06ae1ec..4a14eca 100644
--- a/plugins/phonebook-ebook.c
+++ b/plugins/phonebook-ebook.c
@@ -208,9 +208,15 @@ done:
 	DBG("collected %d vcards", count);
 
 	data->queued_calls--;
-	if (data->queued_calls == 0)
-		data->contacts_cb(data->buf->str, data->buf->len, data->count,
+	if (data->queued_calls == 0) {
+		GString *buf = data->buf;
+		data->buf = NULL;
+
+		data->contacts_cb(buf->str, buf->len, data->count,
 						0, TRUE, data->user_data);
+
+		g_string_free(buf, TRUE);
+	}
 }
 
 static void ebook_entry_cb(EBook *book, const GError *gerr,
-- 
1.7.5.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH obexd 1/5] Fix pulling vCard for multiple ebooks
  2011-07-13 22:10 [PATCH obexd 1/5] Fix pulling vCard for multiple ebooks Bartosz Szatkowski
                   ` (3 preceding siblings ...)
  2011-07-13 22:10 ` [PATCH obexd 5/5] Fix freeing buffer before transfer is complete Bartosz Szatkowski
@ 2011-07-14 13:45 ` Johan Hedberg
  4 siblings, 0 replies; 6+ messages in thread
From: Johan Hedberg @ 2011-07-14 13:45 UTC (permalink / raw)
  To: Bartosz Szatkowski; +Cc: linux-bluetooth

Hi Bartosz,

On Wed, Jul 13, 2011, Bartosz Szatkowski wrote:
> Problem occurred when there were multiple ebooks and the last one was
> empty. Reported vCard number was 0 and no data was transported.
> ---
>  plugins/phonebook-ebook.c |   15 ++++++++-------
>  1 files changed, 8 insertions(+), 7 deletions(-)

All five patches have been applied. Thanks.

Johan

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-07-14 13:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-13 22:10 [PATCH obexd 1/5] Fix pulling vCard for multiple ebooks Bartosz Szatkowski
2011-07-13 22:10 ` [PATCH obexd 2/5] Fix ebook opening error handling Bartosz Szatkowski
2011-07-13 22:10 ` [PATCH obexd 3/5] Add opening ebooks before each operation Bartosz Szatkowski
2011-07-13 22:10 ` [PATCH obexd 4/5] Fix no phone number in vCard listing Bartosz Szatkowski
2011-07-13 22:10 ` [PATCH obexd 5/5] Fix freeing buffer before transfer is complete Bartosz Szatkowski
2011-07-14 13:45 ` [PATCH obexd 1/5] Fix pulling vCard for multiple ebooks Johan Hedberg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.