On Tue, 2013-01-15 at 09:50 +0100, David Henningsson wrote:
> On 01/12/2013 07:22 PM, Tanu Kaskinen wrote:
> > Bug found by David Henningsson.
> > ---
> > reserve-monitor.c | 74 +++++++++++++++++++++++++++++++++++++++++++----------
> > 1 file changed, 60 insertions(+), 14 deletions(-)
> >
> > diff --git a/reserve-monitor.c b/reserve-monitor.c
> > index 4097a6f..39a0c77 100644
> > --- a/reserve-monitor.c
> > +++ b/reserve-monitor.c
> > @@ -59,6 +59,22 @@ struct rm_monitor {
> > "member='NameOwnerChanged'," \
> > "arg0='%s'"
> >
> > +static unsigned get_busy(
> > + DBusConnection *c,
> > + const char *name_owner) {
> > +
> > + const char *un;
> > +
> > + if (!name_owner || !*name_owner)
> > + return FALSE;
> > +
> > + if ((un = dbus_bus_get_unique_name(c)))
> > + if (strcmp(name_owner, un) == 0)
> > + return FALSE;
> > +
> > + return TRUE;
> > +}
> > +
> > static DBusHandlerResult filter_handler(
> > DBusConnection *c,
> > DBusMessage *s,
> > @@ -87,16 +103,7 @@ static DBusHandlerResult filter_handler(
> > if (strcmp(name, m->service_name) == 0) {
> > unsigned old_busy = m->busy;
> >
> > - m->busy = !!(new && *new);
> > -
> > - /* If we ourselves own the device, then don't consider this 'busy' */
>
> This comment got lost. Should be moved to the get_busy function
Ok.
>
> > - if (m->busy) {
> > - const char *un;
> > -
> > - if ((un = dbus_bus_get_unique_name(c)))
> > - if (strcmp(new, un) == 0)
> > - m->busy = FALSE;
> > - }
> > + m->busy = get_busy(c, new);
> >
> > if (m->busy != old_busy && m->change_cb) {
> > m->ref++;
> > @@ -115,11 +122,13 @@ invalid:
> > int rm_watch(
> > rm_monitor **_m,
> > DBusConnection *connection,
> > - const char*device_name,
> > + const char *device_name,
> > rm_change_cb_t change_cb,
> > DBusError *error) {
> >
> > rm_monitor *m = NULL;
> > + DBusMessage *msg = NULL, *reply = NULL;
> > + const char *name_owner;
> > int r;
> > DBusError _error;
> >
> > @@ -178,13 +187,44 @@ int rm_watch(
> >
> > m->matching = 1;
> >
> > - m->busy = dbus_bus_name_has_owner(m->connection, m->service_name, error);
> > + if (!(msg = dbus_message_new_method_call(DBUS_SERVICE_DBUS, DBUS_PATH_DBUS, DBUS_INTERFACE_DBUS, "GetNameOwner"))) {
> > + r = -ENOMEM;
> > + goto fail;
> > + }
>
> The rm_watch function is long enough already - I'd prefer if you move
> the new code to a separate get_name_ower function (even if that would
> mean a strdup).
Will do.
--
Tanu
