[RFC][Patch] DBus Service C library

[Shahar Frank]

Hi Havoc,

> From: Havoc Pennington [mailto:hp at redhat.com]
> Subject: Re: [RFC][Patch] DBus Service C library
> 
> Hi,
> 
> Shahar Frank wrote:
>  > So, I decided to
> > implement simple and more "traditional" C oriented Server library.
The
> > library is not yet finished but I decided to contribute it hoping
that I
> > can get some valuable feedback, advices, comments, idea, etc.
> 
> It's great to see work in this area. Did you see:
> http://lists.freedesktop.org/archives/dbus/2007-July/008159.html
> 
> I think there are some ideas there you could incorporate.
> 
> The code there is a slightly more elaborated version of
>
http://svn.mugshot.org/local-export-daemon/trunk/src/hippo-dbus-helper.h
> 
> example usage is:
> http://svn.mugshot.org/local-export-daemon/trunk/src/session-api.c

Thanks for the links, I will look into them.

> > I made an effort to do things in the "dbus way", but as I learned
dbus
> > low level only while writing this library, I missed many mechanisms
and
> > I have made quiet a few implementation mistakes. For example I
> > implemented a new logging system, didn't use dbus portable function
> > wrappers, and didn't use dbus internal abstract data structures,
such as
> > dbus link lists. I really want to have your opinions on these
subjects
> > (or any other).
> 
> If you use the internal dbus utilities, we essentially have to make
this
> a new feature of libdbus (in the same shared object). That is IMO
> probably the right thing to do.
>
> During a testing/experimentation phase we could keep it separate,
though
> it will mean some weird build system setup.
> 
> I think there's a lot of work left here to get a stable API.

So, if I understand correctly, there are two options for me: 1. don't
rely on the dbus internal utilities, and build the project as totally
high level project, and 2. use the dbus internals and build the project
as an extension to libdbus.
I admit I didn't pay enough attention to this issue, so I assumed a
standalone library model. If you think that the final target for such
project should be tight integration with libdbus, it would eventually
require a stable API and good enough integration with the current
library code. If this is *not* the direction, I should refrain from
using dbus internals even at the cost of re-implementing some
mechanisms. I will be happy to get some input on this subject,
especially when I admit I do not entirely understand the line between
the low level internals and the public API (or more exactly, what should
be the line).

> > If you feel it may interest others, I will happy to move the project
to
> > your repository and work on it from there.
> 
> I'm not sure yet we'd definitely want it in the main dbus package,
> though it seems possible. The first step here would be to get an
account
> on freedesktop.org in the dbus group, and then you could at least set
up
> your own git repository if you want, and put things in CVS later if we
> decide that makes sense. To get an account you have to file a bug on
> bugs.freedesktop.org, the wiki probably has instructions on there
> somewhere.

I asked about the repository because I saw that some binding libraries
are hosted at freedesktop.org. Anyhow, I will open a people account and
continue from there.

> Some code comments, probably premature:

Not premature at all. Sorry for asking so trivial question, but is there
some documentation about the dbus coding style? Google found too many
coding style in freedesktop.org. Anyhow, I will fix the thinks you
already commented about, and I will try to follow these guide lines for
now on.

> ...
> I think you are stuffing the API and interesting-to-user-of-library
> stuff and internal-to-library stuff into one struct.
> 
> I would suggest having one struct that is used by the library user to
> declare their stuff in a static array, and a separate (private, not in
> the header) struct that is used for library-internal data structures.
> 

Seems to be a good idea. I will try to separate the code in the above
way.

> I'm not sure you need most of these Signal/Interface/Param/etc. types
in
> the public API - maybe look at how I did it in the stuff I linked to
at
> the start of the mail.
> 

I will look into it.

> > +char *dbus_service_emit_signal(DBusService *service, DBusMethod
> > *emitter);
> 
> Signal emission in dbus is conceptually from an object, rather than
from
> a bus name.
> 

I understand that, but it seems cleaner to define that operation on the
service because the service is the object that maintains the connection
to the bus. The "emitter" represents the object/interface/member binding
as it is requested by the spec. If that seems confusing, I am open to
suggestions.

> > +char *dbus_service_connect(DBusService * service, int bus_session);
> 
> enum typedef should be used, like DBusBusType, rather than int
> 
> > +static inline char *dbus_method_arg(DBusMethod *method, char *name,
> > void *value)
> > +{
> > +	return dbus_method_get_param(method, name, 1, value);
> > +}
> 
> inline isn't portable (unless we "#define inline" so it vanishes on
some
> platforms), but I think the larger issue here is that an inline or
macro
> ruins the ability to change the implementation in the future without
> rebuilding all apps. So I'd just make these normal library functions.

True. I will change that.

> > +	if (method->flags & DBUS_METHOD_SIGNAL_RECIEVER)
> 
> I before E except after C ;-)
> 

Oops, sorry.

> Anyway, this is a promising and high-value problem to tackle, nice
work.
> 
> Havoc

Thanks for the detailed feedback. It helps a lot.

Shahar