[Bug 34932] Use WockyMetaPorter

[bugzilla-daemon at freedesktop.org]

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

--- Comment #2 from Jonny Lamb <jonny.lamb at collabora.co.uk> 2011-03-23 03:24:30 PDT ---
(In reply to comment #1)
> I think it's stupid that the application has to deal with adding timeouts and
> _force_close_async(). _close_async() should take a timeout. I've filed Bug
> 35548.

I'll look at that soon.

> +      AvahiIfIndex ifindex;
> +
> +      g_object_get (resolver, "interface", &ifindex, NULL);
> 
> … and 'ifindex' is never used. What's it meant to be for? Where's this code
> from?

Removed.

> This comment is misleading—either it's taken its own ref and we can release the
> one we own, or it's stolen ours and we can't. Which is it? I'm gonna guess the
> former, which I don't think needs commenting.

OK, removed.

> +          addresses = g_list_append (addresses, socket_address);
> 
> <http://en.wikipedia.org/wiki/Schlemiel_the_Painter's_algorithm>. Yeah, it's
> not a big deal here, but this is one reason I like stack-allocated GQueues: you
> can build GLists with repeated O(1) append. I guess this isn't new code though.

Yeah yeah, I've never been a fan of this and actually I'm not sure why. I guess
perhaps because it feels like such an epic hack to get around the terrible data
structure we're using. I guess we're doing worse things just by using GObject.

Fixed.

> avahi-contact-manager: add contacts to Wocky's contact factory too when created
> 
> Don't we also want to remove them when they're removed?

The contact factory keeps a weak ref on the objects and they're removed when
the object is disposed. In Salut's case, they're never disposed until
disconnection because the Salut contact manager keeps a ref to the contacts.

> salut_disco_request() documents its 'object' parameter  thusly:
> 
>  * @object: GObject to bind request to. the callback will not be
>  *          called if this object has been unrefed. NULL if not needed
> 
> But as far as I can tell, that's not true any more. If the object dies, the
> cancellable is cancelled, which will lead to the send_iq callback being called.
> _finish() will return an error, which will be happily handed to the callback.
> 
> It looks like the only code paths which cancel the cancellable are those which
> mean the callback shouldn't be called, so you could just bolt an is_cancelled()
> check into disco_request_sent_cb().

Good catch, fixed!

> +  if (wocky_node_get_child_ns (wocky_stanza_get_top_node (stanza), "invite",
> +        GIBBER_TELEPATHY_NS_CLIQUE) != NULL)
> +    /* discard Clique MUC invite */
>      return FALSE;
> 
> I know this comment isn't new, but it is misleading — we're not discarding it.

Fixed.

> In gibber-file-transfer.c:
> 
> +      case PROP_PORTER:
> +        {
> +          self->priv->porter = g_value_dup_object (value);
> +
> +          if (self->priv->porter != NULL)
> +            {
> +              self->priv->stanza_id =
> +                wocky_porter_register_handler_from_anyone (self->priv->porter,
> +                    WOCKY_STANZA_TYPE_IQ, WOCKY_STANZA_SUB_TYPE_NONE,
> +                    WOCKY_PORTER_HANDLER_PRIORITY_NORMAL, received_stanza_cb,
> +                    self, NULL);
> +            }
> 
> Why would it be NULL?

Yeah I guess you're right. I've added a g_return_val_if_fail to the only place
which creates it and it is construct-only after all. Fixed then.

Pushed fixes to my branch.

-- 
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.