Re: DBus methods without name grabbing

[Jan Moringen]

Hi Michael,

sorry for the sloppy patch. I couldn't put as much time into it as I
would have liked.

> > --- doc/misc/ChangeLog      2011-01-04 12:38:33 +0000
> > +++ doc/misc/ChangeLog      2011-01-08 05:37:49 +0000
> > +2011-01-08  Jan Moringen  <address@hidden>
> > +
> > +   * dbus.texi (Receiving Method Calls): rearranged node to mention
> > +   dbus-register-service and dbus-unregister-service first
> > +
> >  2011-01-04  Jan Moringen  <address@hidden>
> 
> There are some conventions for ChangeLog entries: Start with capital
> letter, speak in active words ("Rearrange" instead of "Rearranged"),
> end with a period, etc.

I changed the entry.

> Do you intend to use different email addresses?

No, that wasn't intentional.

> > --- doc/misc/dbus.texi      2011-01-04 12:38:33 +0000
> > +++ doc/misc/dbus.texi      2011-01-08 05:37:49 +0000
> > address@hidden dbus-register-service bus service &rest flags
> > +Register the known name @var{service} on D-Bus @var{bus}.
> > +
> > address@hidden is either the symbol @code{:system} or the symbol
> > address@hidden:session}.
> > +
> > address@hidden is the service name to be registered on the D-Bus.  It
> > +must be a known name.
> > address@hidden defun
> 
> A description of FLAGS is missing, also return values.

Done.

> > address@hidden dbus-unregister-service bus service
> > +Unregister all objects from D-Bus @var{bus}, registered by Emacs for
> > address@hidden
> > +
> > address@hidden is either the symbol @code{:system} or the symbol
> > address@hidden:session}.
> > +
> > address@hidden is the D-Bus service name of the D-Bus.  It must be a
> > +known name.  Emacs releases its association to @var{service} from
> > +D-Bus.
> > address@hidden defun
> 
> Ditto for the return values.

Done.

> > --- lisp/ChangeLog  2011-01-04 19:50:21 +0000
> > +++ lisp/ChangeLog  2011-01-08 05:44:07 +0000
> > +2011-01-08  Jan Moringen  <address@hidden>
> > +
> > +   * net/dbus.el (dbus-unregister-service): translate returned
> > +   integer into a symbol
> > +
> > +   * net/dbus.el (dbus-register-property): use
> > +   `dbus-register-service' to do the name registration
> > +
> 
> It will be just one commit, so you can merge the entries.

Done.

> > --- lisp/net/dbus.el        2011-01-04 10:57:24 +0000
> > +++ lisp/net/dbus.el        2011-01-08 05:44:07 +0000
> > @@ -193,9 +193,14 @@
> >            (puthash key (delete elt value) dbus-registered-objects-table)
> >          (remhash key dbus-registered-objects-table))))))
> >     dbus-registered-objects-table)
> > -  (dbus-call-method
> > -   bus dbus-service-dbus dbus-path-dbus dbus-interface-dbus
> > -   "ReleaseName" service))
> > +  (let ((reply (dbus-call-method
> > +           bus dbus-service-dbus dbus-path-dbus dbus-interface-dbus
> > +           "ReleaseName" service)))
> > +    (case reply
> > +      (1 :released)
> > +      (2 :non-existent)
> > +      (3 :not-owner)
> > +      (t reply))))
> 
> In case reply is not 1, 2 or 3, there is a problem. We shall raise a
> dbus-error then.

Done.

> > @@ -917,14 +922,18 @@
> >    ;; Add the handler.  We use `dbus-service-emacs' as service name, in
> >    ;; order to let unregister SERVICE despite of this default handler.
> >    (dbus-register-method
> > +   bus service path dbus-interface-properties "Get"
> > +   'dbus-property-handler nil)
> > +  (dbus-register-method
> > +   bus service path dbus-interface-properties "GetAll" 
> > +   'dbus-property-handler nil)
> > +  (dbus-register-method
> > +   bus service path dbus-interface-properties "Set" 
> > +   'dbus-property-handler nil)
> > +
> > +  ;; Register the name SERVICE with BUS.
> > +  (unless dont-register-service
> > +    (dbus-register-service bus service))
>  
> I guess you mean t (or better 'dont-register) as last argument of the
> dbus-register-method calls.

I think it has to be t. The service is registered or not in the
following call to `dbus-register-service' depending on
`dont-register-service'.

> Btw, the comment can be shortened. "We use `dbus-service-emacs' ..."
> is not true anymore.

I updated the comment to read:

"Add handlers for the three property-related methods."

> > --- src/dbusbind.c  2011-01-04 11:11:43 +0000
> > +++ src/dbusbind.c  2011-01-08 05:38:56 +0000
> @@ -1835,6 +1847,114 @@
> > +The function returns a keyword, indicating the result of the
> > +operation.  The following keyword can be returned:
> 
> "One of the following keywords is returned:"

Done.

> > +    if (value == -1)
> > +      xsignal0 (Qdbus_error); 
> > +           //      "Unrecognized name request flag");
> 
> XD_SIGNAL2 (build_string ("Unrecognized name request flag"), args[i]);

I was wondering how to do that. Thanks.

> > +  /* Request the known name from the bus.  We can ignore the result,
> > +     it is set to -1 if there is an error - kind of redundancy.  */
> > +  dbus_error_init (&derror);
> > +  result = dbus_bus_request_name (connection, SDATA (service), flags, 
> > +                             &derror);
> > +  if (dbus_error_is_set (&derror))
> > +    XD_ERROR (derror);
> 
> The second sentence of the comment is not true anymore.

I removed it.

> > @@ -2028,18 +2149,9 @@
> >    /* Open a connection to the bus.  */
> >    connection = xd_initialize (bus, TRUE);
> >  
> > +  /* Request the name.  */
> > +  if (!(NILP(dont_register_service)))
> > +    Fdbus_register_service (2, args);
> 
> It must be
> 
> if (NILP (dont_register_service))

Oops.

I hope this increases the quality of the patch to bearable levels.

Kind regards,
Jan

dbus-un-register-service.patch
Description: Text Data