Bug 675211 – email-details.vala:55: Empty e-mail address passed to EmailFieldDetails

After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.

Bug 675211 - email-details.vala:55: Empty e-mail address passed to EmailFieldDetails


Summary:	email-details.vala:55: Empty e-mail address passed to EmailFieldDetails


Status:	RESOLVED FIXED

Product:	folks
Classification:	Platform
Component:	Telepathy backend
Version:	git master
Hardware:	Other Linux

Importance:	Normal normal
Target Milestone:	Unset
Assigned To:	folks-maint
QA Contact:	folks-maint

URL:
Whiteboard:

Depends on:
Blocks:

Reported:	2012-05-01 05:45 UTC by Guillaume Desmottes
Modified:	2012-05-03 08:31 UTC

See Also:
GNOME target:	---
GNOME version:	---

Attachments
persona-store-cache: don't create empty field details (1.72 KB, patch) 2012-05-01 06:06 UTC, Guillaume Desmottes	needs-work	Details \| Review
persona-store-cache: don't create empty field details (2.06 KB, patch) 2012-05-02 08:23 UTC, Guillaume Desmottes	committed	Details \| Review

Description Guillaume Desmottes 2012-05-01 05:45:27 UTC

I just got this warning with Folks master.

I first thought it was a simliar issue as bug #675144 but here the EmailFieldDetails is created in the tpf-persona-store-cache.
Is it a side effect of bug #675144 as empty fields have been stored in the cache? We may just have to ignore empty values.



(empathy:14995): folks-WARNING **: email-details.vala:55: Empty e-mail address passed to EmailFieldDetails.

+ Trace 230155

#0 g_logv
at gmessages.c line 758
#1 g_log
at gmessages.c line 792
#2 folks_email_field_details_construct
at /home/cassidy/gnome/folks/folks/email-details.vala line 55
#3 folks_email_field_details_new
at /home/cassidy/gnome/folks/folks/email-details.vala line 50
#4 ___lambda3_
at /home/cassidy/gnome/folks/backends/telepathy/lib/tpf-persona-store-cache.vala line 337
#5 ____lambda3__tpf_persona_store_cache_afd_deserialisation_callback
at /home/cassidy/gnome/folks/backends/telepathy/lib/tpf-persona-store-cache.vala line 334
#6 tpf_persona_store_cache_deserialise_abstract_field_details
#7 tpf_persona_store_cache_real_deserialise_object
at /home/cassidy/gnome/folks/backends/telepathy/lib/tpf-persona-store-cache.vala line 334
#8 folks_object_cache_deserialise_object
at /home/cassidy/gnome/folks/folks/object-cache.vala line 106
#9 folks_object_cache_load_objects_co
at /home/cassidy/gnome/folks/folks/object-cache.vala line 308
#10 folks_object_cache_load_objects_ready
at /home/cassidy/gnome/folks/folks/object-cache.vala line 197
#11 g_simple_async_result_complete
at gsimpleasyncresult.c line 767
#12 load_contents_close_callback
at gfile.c line 6325
#13 async_ready_close_callback_wrapper
at ginputstream.c line 484
#14 g_simple_async_result_complete
at gsimpleasyncresult.c line 767
#15 complete_in_idle_cb_for_thread
at gsimpleasyncresult.c line 835
#16 g_idle_dispatch
at gmain.c line 4657
#17 g_main_dispatch
at gmain.c line 2539
#18 g_main_context_dispatch
at gmain.c line 3075
#19 g_main_context_iterate
at gmain.c line 3146
#20 g_main_context_iteration
at gmain.c line 3207
#21 g_application_run
at gapplication.c line 1496
#22 main
at empathy.c line 837

Comment 1 Guillaume Desmottes 2012-05-01 06:06:20 UTC

Created attachment 213164 [details] [review]
persona-store-cache: don't create empty field details

Comment 2 Philip Withnall 2012-05-01 20:17:04 UTC

Review of attachment 213164 [details] [review]:

::: backends/telepathy/lib/tpf-persona-store-cache.vala
@@ +336,3 @@
                 {
+                  if (v != null)
+                    email_address_set.add (new EmailFieldDetails (v, p));

Are you sure this works? v should never be null; iirc GVariant guarantees that strings are never null (since we don’t use maybe types). My guess is that you should be checking for “!= ""” here.

Also (in any case), please put braces around the if block, even if it’s only one line long.

It might also be helpful to add a comment saying that this check is only necessary to deal with broken cache files. :-)

Comment 3 Guillaume Desmottes 2012-05-02 08:16:58 UTC

You're right, it should be != "".
Do you have an equivalent of tp_str_empty()? Would make things easier.

Comment 4 Guillaume Desmottes 2012-05-02 08:23:12 UTC

Created attachment 213260 [details] [review]
persona-store-cache: don't create empty field details

Comment 5 Philip Withnall 2012-05-02 22:16:12 UTC

Review of attachment 213260 [details] [review]:

Looks good to me. Please commit to master.

> Do you have an equivalent of tp_str_empty()? Would make things easier.

We don’t , and it’s not really relevant here because the values can never be null. I guess a tp_str_empty() equivalent could be added in future if it becomes useful.

Comment 6 Guillaume Desmottes 2012-05-03 08:31:09 UTC

Attachment 213260 [details] pushed as 2f1c60b - persona-store-cache: don't create empty field details