Hi Bastien, On Wed, Nov 22, 2017 at 12:05 AM, Bastien Nocera <hadess@xxxxxxxxxx> 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@xxxxxxxxx> >> >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html