Re: [PATCH v2 2/3] dbus: Add disconnect watch support

[Jukka Rissanen <jukka.rissanen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2299 bytes --]

On pe, 2015-02-13 at 09:05 -0600, Denis Kenzior wrote:
> Hi Jukka,
> 
> On 02/13/2015 08:23 AM, Jukka Rissanen wrote:
> > On pe, 2015-02-13 at 08:01 -0600, Denis Kenzior wrote:
> >> Hi Jukka,
> >>
> >> <snip>
> >>
> >>   >>> +struct filter_data {
> >>>>> +	struct l_dbus *dbus;
> >>>>> +	l_dbus_message_func_t handle_func;
> >>>>> +	l_dbus_watch_func_t connect_func;
> >>>>> +	l_dbus_watch_func_t disconnect_func;
> >>>>> +	char *name;
> >>>>> +	char *owner;
> >>>>
> >>>> What is the exact difference between name and owner?  I don't see a
> >>>> distinction...
> >>>
> >>> name is something like "org.foobar" and owner is ":1.101"
> >>>
> >>> User registers watch using a name and dbus-daemon sends its
> >>> notifications about NameOwnerChanged signals using the owner format so
> >>> we need to track both formats.
> >>
> >> Yes, but you don't actually handle that anywhere.  So I don't see a
> >> distinction in the current code.  If this is TBD, then simply don't
> >> include this and mention somewhere that only unique bus names are supported.
> >
> > Hmm, owner and name are handled differently in the code. For example see
> > the format_rule() function.
> >
> 
> Then help me understand.  You have:
> 
> in format_rule you have:
> +       offset = snprintf(rule, size, "type='signal'");
> +       sender = data->name ? : data->owner;
> +
> +       if (sender)
> +               offset += snprintf(rule + offset, size - offset,
> +                               ",sender='%s'", sender);
> 
> in filter_data_get you have:
> +       if (sender) {
> +               if (sender[0] == ':')
> +                       owner = sender;
> +               else
> +                       name = sender;
> +       }
> 
> So why not replace everything by simply sender?  Right now none of this 
> makes sense to me since you're not traking the well-known-name -> unique 
> name mapping.
> 
> If you only support unique names, just say so and make the code reflect 
> that.  Don't try to sneak code in that is part of some future 
> functionality.  I'm not a mind reader.

The owner/name distinction is indeed needed only for watching dbus
connects, and as we do not implement the connect support yet I will
remove this code.


Jukka