[pulseaudio-discuss] [PATCH reserve 2/2] Initialize monitor's busy status to false if we own the device.

[Tanu Kaskinen]

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