[Bug 32702] TpContact should have properties for ContactGroups

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Mon Jan 31 11:44:14 CET 2011


https://bugs.freedesktop.org/show_bug.cgi?id=32702

--- Comment #6 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2011-01-31 02:44:11 PST ---
+    /* ContactGroups */
+    GPtrArray *contact_groups;
Please document the content of this array and own the memory is managed.

+ * @n_groups: the length of @groups, or -1 if @groups is %NULL-terminated
+ * @groups: (array length=n_groups) (allow-none): the set of groups which the
+ *  contact should be in (may be %NULL if @n_groups is 0)

Is this binding friendly? Maybe it is, I'm just wondering.


+  g_return_if_fail (groups != NULL || n_groups == 0);
strictly speaking, groups == NULL && n_groups == -1 isn't valid as well? :)

+ * Finishes an async set of @self contact groups. If the operation was
+ * successful, the contact's groups can be accessed using
+ * tp_contact_get_contact_groups().

This is a bit miss leading. tp_contact_get_contact_groups() can  still be used
even if the call failed. It will just return the old groups.

You should use _tp_implement_finish_* macros to implement your finish
functions.

   * This is set to %NULL if %TP_CONTACT_FEATURE_CONTACT_GROUPS is not set
+   * on this contact,
"is not prepared" ?

Did you valgrind your tests?

+      for (iter = removed; *iter != NULL; iter++)
+        for (j = 0; j < contact->priv->contact_groups->len; j++)
I'd 'add {} for extra clarity.

contact_maybe_set_contact_groups() arithmetic on pointers scares me, I would
have used an iterator, but I guess that's fine.


Can't contact_maybe_set_contact_groups() be called twice and so hit the
g_assert()?

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.



More information about the telepathy-bugs mailing list