Re: [PATCH BlueZ] gdbus: Fix not emiting PropertiesChanged

[Luiz Augusto von Dentz <luiz.dentz@gmail.com>]

Hi Bastien,

On Wed, Nov 22, 2017 at 12:05 AM, Bastien Nocera <hadess@hadess.net> wrote:
>> Re: [PATCH BlueZ] gdbus: Fix not emiting PropertiesChanged
>
> emitting.
>
> On Tue, 2017-11-21 at 22:04 +0200, Luiz Augusto von Dentz wrote:
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> If and interface is removed while properties are pending it would cause
>
> "an interface".
>
>> process_properties_from_interface to clear data->pending_prop when it
>> should only clear the iface->pending_prop.
>
> I prefer my version, which allows the pending properties to be cleared
> out if the pending ones were for that interface. I also prefer the
> subject line I used, describing the actual problem it fixes, and the
> commit message going more into details.

I guess this one is simpler to maintain, also notice that iterating in
the data->interfaces to check if there are no other pending_prop has
almost the same effect as process_property_changes, what would make a
difference would be to prevent process_changes but that has to run
anyway to propagate the interfaces added/removed. Also notice that
data->pending_prop is clear earlier than the original fix for
duplicated signals, the one that introduced this problem, that way we
don't reintroduce the original problem.

Regarding the commit message, we usually avoid BlueZ specific
description there as gdbus is shared with other projects.

>
> But up to you.
>
> Cheers
>
>> ---
>>  gdbus/object.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdbus/object.c b/gdbus/object.c
>> index afb458764..5c8ad7a55 100644
>> --- a/gdbus/object.c
>> +++ b/gdbus/object.c
>> @@ -1659,8 +1659,6 @@ static void
>> process_properties_from_interface(struct generic_data *data,
>>       DBusMessageIter iter, dict, array;
>>       GSList *invalidated;
>>
>> -     data->pending_prop = FALSE;
>> -
>>       if (iface->pending_prop == NULL)
>>               return;
>>
>> @@ -1722,6 +1720,8 @@ static void process_property_changes(struct
>> generic_data *data)
>>  {
>>       GSList *l;
>>
>> +     data->pending_prop = FALSE;
>> +
>>       for (l = data->interfaces; l != NULL; l = l->next) {
>>               struct interface_data *iface = l->data;
>>



-- 
Luiz Augusto von Dentz