> > `geo_master` doesn't need to be a global variable: it's only ever used in
> geo_start.
>
> The issue is that geoclue_master_create_client_async() populates its private
> GeoclueMasterAsyncData struct with a pointer to that master without reffing
> it. That's why datetime-service keeps its own ref.
That should be fixed in geoclue.
> > Why are you calling geo_stop unconditionally in on_use_geoclue_changed? I'd
> put it in the `else` below, in case the callback is called even though the
> value hasn't changed (which may happen according to the docs).
>
> It's to ensure that even if the callback triggered multiple calls to
> geo_start(), we would have a call to geo_stop() in between to guarantee that
> geo_master, geo_client, and geo_address were cleaned up.
> > `geo_master` doesn't need to be a global variable: it's only ever used in master_ create_ client_ async() populates its private yncData struct with a pointer to that master without reffing
> geo_start.
>
> The issue is that geoclue_
> GeoclueMasterAs
> it. That's why datetime-service keeps its own ref.
That should be fixed in geoclue.
> > Why are you calling geo_stop unconditionally in on_use_ geoclue_ changed? I'd
> put it in the `else` below, in case the callback is called even though the
> value hasn't changed (which may happen according to the docs).
>
> It's to ensure that even if the callback triggered multiple calls to
> geo_start(), we would have a call to geo_stop() in between to guarantee that
> geo_master, geo_client, and geo_address were cleaned up.
Sounds reasonable.