Bug 19164 - [0.11] TpChannel:identifier represents "no identifier" both as NULL and as ""
Summary: [0.11] TpChannel:identifier represents "no identifier" both as NULL and as ""
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/sm...
Whiteboard: review+
Keywords: patch
Depends on: 24116
Blocks:
  Show dependency treegraph
 
Reported: 2008-12-18 10:05 UTC by Will Thompson
Modified: 2010-04-23 09:58 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Will Thompson 2008-12-18 10:05:03 UTC
TpChannel:identifier for anonymous channels is NULL if the underlying channel doesn't implement the TargetID property, but "" if it does. TpChannel should be consistent with whatever TpContact:identifier does.
Comment 2 Simon McVittie 2009-09-23 06:53:41 UTC
Too risky for a stable branch. 0.9 material.
Comment 3 Simon McVittie 2010-03-30 13:00:38 UTC
Xavier, is this branch still what you want / suitable for review?
Comment 4 Simon McVittie 2010-04-12 10:53:29 UTC
What TpContact does turns out to be irrelevant, since a TpContact always has a valid identifier.

Xavier's branch implements the following semantics:

* if identifier is not known yet, return NULL
* if identifier is known to be irrelevant due to TargetHandleType = None, return ""

This is consistent with the previous behaviour of channels with TargetID support (i.e. increasingly many). However, I wonder whether returning a non-NULL string from the start makes more sense?

I'm unsure about the appropriateness of the implementation: getting the immutable properties dict will still yield an unspecified TargetID, but we can do better.  _tp_channel_get_identifier() already has a special case for THT=None; that special case could just be extended to set the TargetID using _tp_channel_maybe_set_identifier before continuing.
Comment 5 Simon McVittie 2010-04-12 10:56:15 UTC
From a quick skim through Empathy, I think we're right to avoid NULL; many callers effectively assume a non-NULL return. I'll do an alternative branch.
Comment 6 Simon McVittie 2010-04-12 11:28:07 UTC
How's this? I'd particularly appreciate review from Will and Xavier, who've looked at this before.

http://git.collabora.co.uk/?p=user/smcv/telepathy-glib-smcv.git;a=shortlog;h=refs/heads/null-ident
Comment 7 Will Thompson 2010-04-21 10:45:56 UTC
(In reply to comment #6)
> How's this? I'd particularly appreciate review from Will and Xavier, who've
> looked at this before.
> 
> http://git.collabora.co.uk/?p=user/smcv/telepathy-glib-smcv.git;a=shortlog;h=refs/heads/null-ident

Looks fine.
Comment 8 Simon McVittie 2010-04-23 09:58:27 UTC
Merged, will be in 0.11.4.


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.