[Bug 34932] Use WockyMetaPorter

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=34932

--- Comment #10 from Jonny Lamb <jonny.lamb at collabora.co.uk> 2011-04-04 03:17:39 PDT ---
(In reply to comment #9)
> Why isn't the MUC manager just registering a higher-priority handler (and
> returning TRUE), rather than some other unrelated handler having to know
> details of Clique?

Fair, done.

> +      if (error != NULL)
> +        g_error_free (error);
> 
> FYI g_clear_error() is null-safe.

Done.

> +  if (force_called)
> +    tp_base_connection_finish_shutdown (base);
> 
> It looks like you're ensuring that finish_shutdown will be called exactly 0 or
> 2 times, and never once? closed_cb only calls it if disconnect_timer is not 0
> ... okay, so this is right, in which case why is the variable called
> "force_called"? I think it means exactly the opposite.

ha, half of that is true. force_called does indeed mean exactly the opposite,
but I had also got the conditional to call finish_shutdown the wrong way round
too, so if you replace that

  if (force_called) finish_shutdown()

with

  if (force_not_called) finish_shutdown()

That makes more sense, which is why it actually worked. Anyway, fixed.

> disco: only delete the request if it hasn't been already done for us
> 
> This looks completely wrong to me, I'm sorry to say. As far as I can tell,
> you'll *always* leak the SalutDiscoRequest if the cancellable is cancelled,
> given this patch.

Yeah I guess this was the wrong to go about fixing this problem and didn't
actually solve it in every case. I appended a patch.

> muc-manager: don't assert on no invite node
> 
> This patch looks wrong too. When the handler is registered, the pattern
> specifies that the <invite xmlns='...clique'/> element must exist. And the
> commit message is not English.

Fair. (I removed the assertion when I had removed the handler register stanza
in a local checkout etc.) Reverted.

>    /* release any references held by the object here */
> 
> -  if (self->handle != 0)
> -    tp_handle_unref (contact_repo, self->handle);
> +  if (self->handle != 0 && self->connection != NULL)
> +    {
> +      TpHandleRepoIface *contact_repo = tp_base_connection_get_handles
> +        ((TpBaseConnection *) self->connection, TP_HANDLE_TYPE_CONTACT);
> +      tp_handle_unref (contact_repo, self->handle);
> +    }
> 
> There's an argument that this code could just go away since tp_handle_unref is
> a no-op these days.

I think I'll just leave this for now.

> +        'nickname': re.sub(r'.*/', '', sys.argv[0]),
[...]
> +        name += ('-' + re.sub(r'.*/', '', sys.argv[0])[:-3])

Okay. I took this from your patch to do a similar thing in gabble
(d7ae6c73bce69117).

> Is there really no constant for this in the avahi library?

No.

Okaaaaay, pushed.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.