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

[Bastien Nocera <hadess@hadess.net>]

On Wed, 2017-11-22 at 15:38 +0200, Luiz Augusto von Dentz wrote:
> Hi,
> 
> On Wed, Nov 22, 2017 at 11:14 AM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> > 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;
> > > > 
> 
> Applied.

You could at least have applied fixes to the 2 typos I mentioned :(