Bug 31581

Summary:	tp_group_mixin_remove_members_with_reason breaks API guarantees
Product:	Telepathy	Reporter:	Jonny Lamb <jonny.lamb>
Component:	tp-glib	Assignee:	Simon McVittie <smcv>
Status:	RESOLVED FIXED	QA Contact:	Telepathy bugs list <telepathy-bugs>
Severity:	normal
Priority:	medium	CC:	youness.alaoui
Version:	0.12	Keywords:	patch
Hardware:	Other
OS:	All
URL:	http://git.collabora.co.uk/?p=user/smcv/telepathy-glib-smcv.git;a=shortlog;h=refs/heads/012-actually-const
Whiteboard:	review+
i915 platform:		i915 features:

Description Jonny Lamb 2010-11-12 06:54:05 UTC

I told my grandmother that she was fine to trust the highly documented telepathy-glib library's API guarantees and that Simon McVittie knew what const meant. I don't like lying to my grandmother, but lying to my grandmother is what I have done.

tp_group_mixin_remove_members_with_reason takes a const GArray *contacts but then modifies the contents of this const GArray*. Correct me if I'm wrong, but const is for more than looking pretty in function arguments.

Perhaps you meant const GArray * const * const * GArray * const?

(btw, the offending commit is:

commit a36f21cbec16a1c9a92075d6c26827861e8ab53f
Author: Simon McVittie <simon.mcvittie@collabora.co.uk>
Date:   Fri May 21 13:16:33 2010 +0100

    TpGroupMixin: when removing members, don't raise an error if it's a no-op

    This is awkward if two clients both try to remove the same member at the
    same time, for instance.
)

Comment 1 Simon McVittie 2010-11-15 04:36:33 UTC

(In reply to comment #0)
> tp_group_mixin_remove_members_with_reason takes a const GArray *contacts but
> then modifies the contents of this const GArray*.

Oops. I'll get on that.

Comment 2 Simon McVittie 2010-11-15 04:52:36 UTC

I was surprised the compiler didn't catch this, but it turns out to be because g_array_index contains a cast to (TpHandle *), which casts away the constness at the same time as correcting the type.

Comment 3 Jonny Lamb 2010-11-15 05:30:01 UTC

(In reply to comment #2)
> I was surprised the compiler didn't catch this, but it turns out to be because
> g_array_index contains a cast to (TpHandle *), which casts away the constness
> at the same time as correcting the type.

Okay, I'll let you off this time.

Comment 4 Simon McVittie 2010-11-16 03:29:39 UTC

Fixed in git for 0.12.4 and 0.13.6

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.