GBus discussion

[Havoc Pennington]

Hi,

Fan Wu wrote:
> If that is the case you can go further by getting rid of all the
> register/unregister calls, and ask user to fill in a big table which
> has all the callback/type/path/name information, and then pass the
> table to a dbus_start() which will set up the loop and get the
> connection and call the callbacks if needed.

It's worth exploring, but I would build it on the register/unregister 
primitives for individual things, rather than getting rid of those. The 
current way looks like:

  register_connection_handler(connected_handler, disconnected_handler)

  connected_handler(Connection *cnxn)
  {
     register_bus_name_owner(cnxn, name_owned_handler, 
name_not_owned_handler)
  }

  disconnected_handler(Connection *cnxn)
  {
   unregister_bus_name_owner(cnxn, name_owned_handler, 
name_not_owned_handler);
  }

  name_owned_handler(Connection *cnxn)
  {
     register_relevant_signals_and_objects(cnxn)
  }

  name_not_owned_handler(Connection *cnxn)
  {
     unregister_relevant_signals_and_objects(cnxn)
  }

Note that for this to work well, when the connection is disconnected you 
would call the callbacks in reverse order from when connecting:
  - first mark all the names not owned and call those callbacks
  - next call disconnected handler

I think this is pretty good, but as you say can be improved.

But even in the code already in the gbus directory, there are some 
"multiple steps at once" for example the single_instance functions 
register a connection handler which on connect registers a bus name 
owner which on getting the name finally calls into the app.

So already there's an example of building a higher-level primitive that 
hides several nested callbacks from the app - the app only has to write 
the owned/not_owned handlers.

There's room to combine more steps and avoid intermediate callbacks that 
would contain only further registration calls.

I would keep the register/unregister API in any case, but then build 
additional layers on top of it that do register/unregister "chains" such 
as the single instance chain from connected->name owned->install objects

>> The point is that you could register multiple NameOwner objects and
>> data for the same owned_name. So only the first one matching the
>> passed-in NameOwner and data would be unregistered.
> 
> Why would you need multiple NameOwner objects for one name?

Think of name acquired/lost as a signal; you might have two callbacks in 
different parts of the code that want to do something when losing a 
particular name.

> The difference between NameLost and FailedToAcquire is, for NameLost
> most likely something needs to be cleaned up before exit; while for
> FailedToAcquire, probably nothing has been started yet so the clean up
> can be saved. I would say some application might appreciate the extra
> information, if just for logging purpose.

Remember that we might also have a sequence like this:

  1. connect
  2. request name
  3. got name, call name-owned callback
  4. disconnected, call name-not-owned callback
  5. reconnect
  6. request name
  7. did not get name, call name-not-owned callback

So the state the app would need to know in this scenario is whether it 
had ever gotten a "name owned" - the app does not care _how_ the library 
knows that the name is not owned.

You could put a "dbus_bool_t first_time" flag in the callback, but I 
would advise against it, because as a rule having any 
not-absolutely-essential args to callbacks makes them more annoying and 
less convenient. The app can always store what it needs to know in the 
application data structure (which is the callback data pointer). In this 
case, for example, the app would generally be written like:

typedef struct { dbus_bool_t initialized; } Application

main()
{
   Application *app = app_new();
   register_single_instance(owned_callback, not_owned_callback, app);
}

void
owned_callback(DBusConnection *connection,
                void           *data)
{
   Application *app = data;
   if (!app->initialized)
    {
      app_initialize(app);
    }
}

void
not_owned_callback(DBusConnection *connection,
                    void           *data)
{
   Application *app = data;
   if (app->initialized)
    {
      app_uninitialize(app);
    }
}

I would suggest guaranteeing in the API docs that these two callbacks 
are always called alternating (you never get two "owned" in a row or two 
"not owned" in a row). However, if you guarantee that owned_callback is 
always called first, I think there's nothing good to do if the name is 
never owned once.

Havoc