Wocky branch:
http://cgit.collabora.com/git/user/xclaesse/wocky.git/log/?h=see-other-host

Gabble branch:
http://cgit.collabora.com/git/user/xclaesse/telepathy-gabble.git/log/?h=see-other-host

I don't know if that's the best way to implement this, I'm not wocky/gabble expert. But at least it works with the WLM server.

This is pretty critical, since this morning, I couldn't connect to WLM anymore without those patches.

This looks pretty much how I would do it too.

> + connect_to_host_async (self, other_host, 5222);

Why are you specifying 5222? The other host might have specified a different port, no?

A Wocky test would be pretty cool... :-)

(In reply to comment #2)
> This looks pretty much how I would do it too.

Cool :)

> > + connect_to_host_async (self, other_host, 5222);
> 
> Why are you specifying 5222? The other host might have specified a different
> port, no?

That ends into g_socket_client_connect_to_host(). As I understand it, port is a fallback in the case host does not contain the port. WLM server does not set a port with the host.

> A Wocky test would be pretty cool... :-)

Actually the gabble patch breaks one of its unit test, because CM should not close the channel on the behalves of the handler. I'll work on a fix.

+ p = g_strstr_len (host_and_port, -1, ":");
+ if (p != NULL)
+   p = g_strstr_len (p + 1, -1, ":");
+ if (p != NULL)
+   uri_format = "%s://[%s]";

It took me a little while to work out what you were actually checking for here. Could you just add a small comment to point out you're looking for two ':' chars please?

- /* properties: */
- GIOStream *stream;
-

Why are you removing this? It looks completely unrelated to the rest of the patch. If it's unused feel free to remove it, but please split your patches.

I don't exactly understand what you are doing in the test but tbh if things still test properly and stuff passes and your test case is being executed then that's fine by me. :-)

(In reply to comment #4)
> + p = g_strstr_len (host_and_port, -1, ":");
> + if (p != NULL)
> +   p = g_strstr_len (p + 1, -1, ":");
> + if (p != NULL)
> +   uri_format = "%s://[%s]";
> 
> It took me a little while to work out what you were actually checking for here.
> Could you just add a small comment to point out you're looking for two ':'
> chars please?

Added /* if host_and_port contains 2 ':' it must be an ipv6 */

> - /* properties: */
> - GIOStream *stream;
> -
> 
> Why are you removing this? It looks completely unrelated to the rest of the
> patch. If it's unused feel free to remove it, but please split your patches.

It's unused, pushed that trivia to master already :)

> I don't exactly understand what you are doing in the test but tbh if things
> still test properly and stuff passes and your test case is being executed then
> that's fine by me. :-)

It though me a while to understand how those unit tests works. The idea of my change is to be able to run multiple servers, so the first will do a see-other-host error and tell to connect the the next one.

(In reply to comment #3)
> Actually the gabble patch breaks one of its unit test, because CM should not
> close the channel on the behalves of the handler. I'll work on a fix.

My branch now fix that as well. It's much more complex than I first though, because it can now happen that we have more than one TLS channel at the same time. This is because it can happen that wocky start connecting to next host (after a see-other-host) before handler of the previous TLS check called Close(). Actually with empathy that always happen because empathy never close TLS channels.

wocky branch pushed, gabble branch still needs review.

Regarding your gabble branch:

Personally I think a nicer way than having both

     GabbleServerTLSChannel *channel
     GList *completed_channels;

in your private structure, you could just have a GQueue *channels and peek the head when you mean priv->channel, no? I guess that would make referencing the GAsyncResult with channel harder. What do you think?

(In reply to comment #8)
> Regarding your gabble branch:
> 
> Personally I think a nicer way than having both
> 
>      GabbleServerTLSChannel *channel
>      GList *completed_channels;
> 
> in your private structure, you could just have a GQueue *channels and peek the
> head when you mean priv->channel, no? I guess that would make referencing the
> GAsyncResult with channel harder. What do you think?

I think there are a lot of priv->channel access in that code, modifying them all to g_queue_peek_head() is boring :p

IMO it's nice to have all the current operations stuff in the priv struct, then have a list for stuff from the past.

(In reply to comment #9)
> I think there are a lot of priv->channel access in that code, modifying them
> all to g_queue_peek_head() is boring :p

Hardly...

> IMO it's nice to have all the current operations stuff in the priv struct, then
> have a list for stuff from the past.

Oh well, up to you then, there are probably much worse things in gabble.

Merge away.

Backported to stable branch, released as 0.16.1.