Bug 34716 - Split wocky_porter_register_handler up to make sender matching more explicit.
Summary: Split wocky_porter_register_handler up to make sender matching more explicit.
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: gabble (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/wj...
Whiteboard:
Keywords: patch
Depends on:
Blocks: 33911
  Show dependency treegraph
 
Reported: 2011-02-25 05:24 UTC by Will Thompson
Modified: 2011-03-10 02:49 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Will Thompson 2011-02-25 05:24:41 UTC
Currently we have:

+guint wocky_porter_register_handler (WockyPorter *self,
+    WockyStanzaType type,
+    WockyStanzaSubType sub_type,
+    const gchar *from,
+    guint priority,
+    WockyPorterHandlerFunc callback,
+    gpointer user_data,
+    ...) G_GNUC_NULL_TERMINATED;

If 'from' is NULL, stanzas from anyone are matched. There is no way to match stanza from the server.

The attached branch splits this function into three:

• wocky_porter_register_handler_from(), which accepts a non-NULL 'const gchar *from' argument;
• wocky_porter_register_handler_from_anyone(), which matches stanzas from any sender;
• wocky_porter_register_handler_from_server(), which matches stanzas from the server, using the same logic as matching IQ replies from the server.

It also fixes a bug where matching JIDs without a node part, like "example.com", would actually match stanzas from anyone.
Comment 1 Jonny Lamb 2011-02-25 07:23:12 UTC
If you'd put your register_handler_va implementation AFTER the old register_handler implementation it would have been a lot easier to review. :-(

Anyway, it looks fine. I like the way there are now three functions for the three cases, it makes things really explicit what's going on.

I didn't really review all the test changes in depth, but yay go go go. Who's going to update gabble? :-P
Comment 2 Will Thompson 2011-02-25 07:26:34 UTC
Merged. I'm moving the bug to Gabble, which needs updating.
Comment 3 Will Thompson 2011-02-28 13:05:51 UTC
hi, here's a gabble branch!
Comment 4 Jonny Lamb 2011-03-03 06:53:45 UTC
32ddfbaf559dbb1 confuses me. I read the commit message and the comment in the test and it appears to want to check that gabble doesn't crash when a roster push with no id attribute comes in. That's great.

But it also appears to say that Will has added code to discard such stanzas, except in the test you check that the roster push has succeeded? What am I missing?

fyi I opened bug #34975 about the unknown IQ handler in GabbleConnection. I didn't realise the porter didn't do this already!

POW, REVIEW DONE!
Comment 5 Will Thompson 2011-03-10 02:49:46 UTC
Jonny approved an amended version of that patch with a clearer commit message: <http://cgit.freedesktop.org/telepathy/telepathy-gabble/commit/?id=99b609ad16d0345394661cc10a3e4202c79b7e42>

So, I've merged the branch. Hooray! <http://cgit.freedesktop.org/telepathy/telepathy-gabble/commit/?id=b2f292263f188d88b83c610d8a7f560bddf85e38>


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.