Currently TpBase*Class have inconsistent way of letting implementation tell which dbus interfaces are implemented. Historically they had a const gchar * const *interfaces; field, but it has the problem that subsubclass can't define additional interfaces. For example we have TpBaseChannel->TpBaseCallChannel->TpBaseMediaCallChannel->GabbleCallChannel and at each level we may have additional interfaces. To Fix that issue we deprecated the "interfaces" field from TpBaseChannel and added GPtrArray *get_interfaces (TpBaseChannel *self); so subclasses can override that method, first chainup to get the GPtrArray of interfaces implemented by parents then simply g_ptr_array_add() extra ifaces and return that. TpBaseChannel, TpBaseCallContent and TpBaseCallStream use that system, but not TpBaseConnection, TpBaseConnectionManager, TpBaseRoomConfig and TpBaseProtocol. This is current inconsistent, so I suggest using the get_interfaces everywhere.
Okay I worked on this in master. Once merged, I'll remove all the old stuff in next. (In reply to comment #0) > TpBaseChannel, TpBaseCallContent and TpBaseCallStream use that system, but not > TpBaseConnection, Done. > TpBaseConnectionManager, Done. > TpBaseRoomConfig and This doesn't actually need this? I did fix a comment referring to TpBaseChannel.interfaces though. > TpBaseProtocol. This already has a get_interfaces() which returns a newly-allocated GStrv (why, I wonder?) This makes is harder to chain-up. Do you think another get_interfaces_array (which uses a GPtrArray) should be added for master, then renamed in next?
(In reply to comment #1) > Okay I worked on this in master. Once merged, I'll remove all the old stuff in > next. Perfect! > (In reply to comment #0) > > TpBaseRoomConfig and > > This doesn't actually need this? I did fix a comment referring to > TpBaseChannel.interfaces though. True > > TpBaseProtocol. > > This already has a get_interfaces() which returns a newly-allocated GStrv (why, > I wonder?) This makes is harder to chain-up. Do you think another > get_interfaces_array (which uses a GPtrArray) should be added for master, then > renamed in next? Yeah, this one is sad... I would vote for doing get_interfaces_array() in master and rename it in next. I would also use _TP_SEAL(interfaces) to make them easily catchable at build time, but that means they need to be out of the doc, see http://cgit.freedesktop.org/telepathy/telepathy-glib/commit/?id=e8f9a27a388e2123a3281d610acbcd6a4b17d9a1 tp_base_connection_add_interfaces(): doing a g_strv_length() to set the size of the array is totally overkill IMO. It means you iterate the array twice with probably no benefit since GArray already pre-allocate more than needed. We are not dealing with hundred of ifaces here, GArray's minimum allocation is already 8 elements, we probably will never have more ifaces than that anyway.
(In reply to comment #2) > Yeah, this one is sad... I would vote for doing get_interfaces_array() in > master and rename it in next. Alright, done. > I would also use _TP_SEAL(interfaces) to make them easily catchable at build > time, but that means they need to be out of the doc, see > http://cgit.freedesktop.org/telepathy/telepathy-glib/commit/?id=e8f9a27a388e2123a3281d610acbcd6a4b17d9a1 Done. > tp_base_connection_add_interfaces(): > doing a g_strv_length() to set the size of the array is totally overkill IMO. > It means you iterate the array twice with probably no benefit since GArray > already pre-allocate more than needed. We are not dealing with hundred of > ifaces here, GArray's minimum allocation is already 8 elements, we probably > will never have more ifaces than that anyway. I' simplified this function.
+1
(In reply to comment #4) > +1 Cool, thanks. I've merged this to master. I'll remove all the deprecated stuff when merging into next.
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.