d/bus cleanups ...

[Havoc Pennington]

Hi,

Thanks for the hacking. I'm getting to your other mail also.

On Fri, 2004-03-26 at 11:07, Michael Meeks wrote:
> 	So - this patch (I hope) doesn't do much that's controversial, just
> cleans up some existing code; other bits later. It'd be nice to have
> someone like Hallski able to approve this sort of cleanup if that's
> possible (?) - so one can avoid having too much outstanding unrelated
> stuff un-committed.

Some of this is new API/implementation and such not just cleanup ;-) but
yeah in general I'm fine with cleanup-level patches going in as long as
any two people who've contributed a bit agree they are right,  they get
posted to the list so everyone's aware of them, and make check passes
and covers the newly-added code.

> 	+ implement signal emission code
> 		+ not in the right place prolly, but useful anyway
> 		+ untested, as yet, I'd like to get some other bits in
> 		  place first

We really need "make check" passing and testing the glib stuff - see the
mail from Olivier who has tracked this down. I'm not comfortable
accepting piles of patches without automated tests, though I understand
we have to get to minimal workingness before we can test.

"make check-coverage" makes it easy to see what the test coverage is...
esp. when combined with "decode-gcov foo.c"

> 		+ should we add some more exceptions:
> 			+ DBUS_ERROR_UNKNOWN_SIGNAL  to match
> 			  UNKNOWN_METHOD (?)
> 			+ DBUS_ERROR_BAD_PARAM - bad parameter (?)
> 		+ also, the signal handler code, currently returns a
> 		  value - I assume that's pretty pointless for an async
> 		  method - what is the canonical solution for this 
> 		  situation ?

I don't think replies of any kind to a signal make sense; you can't
return errors or return values.

Of course GObject allows return values on signals, for a signal marked
remote I think we have to disallow or ignore said value.

Some comments on the patch:
 
 I don't understand dbus_gobject_handle_signal() - it seems to emit 
 a signal from the GObject. DBUS_MESSAGE_TYPE_SIGNAL is a signal 
 emission, not a request to emit a signal. In dbus-gobject.c what 
 you want to do for signals is send DBUS_MESSAGE_TYPE_SIGNAL to 
 forward a GObject signal remotely (only for those signals annotated
 as "should be remoted"). In dbus-gproxy.c, what you want to do for 
 signals is emit received::Foo in response to getting D-BUS signal
 message Foo.

 It should be possible to register an object with multiple 
 DBusConnection, so the quarks approach has to be more 
 complex (store a list, essentially). 

 Multiple registration of the same object on the same connection 
 should _not_ be allowed, though.
 
 I don't think storing a copy of the path plus two bits of object
 data is really acceptable overhead; we should find some way 
 to avoid it. Perhaps an unregister_by_data or something?
 Though that is a linear search in dbus-object-tree.c. Maybe
 registration returns an ID cookie (which could just be a pointer
 to an internal dbus-object-tree structure). Needs thought.

 dbus-gvalue.c is a nice plan.

 error_printf, nice.

Havoc