After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 764039 - [review] dcbw/dupe-pc-signals-bgo764039: fix duplicate PropertiesChanged notifications and values
[review] dcbw/dupe-pc-signals-bgo764039: fix duplicate PropertiesChanged noti...
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks: nm-review nm-1-2
 
 
Reported: 2016-03-22 18:24 UTC by Dan Williams
Modified: 2016-03-29 22:14 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Dan Williams 2016-03-22 18:24:25 UTC
Due to the GDBus port there are various issues with the NM-specific PropertiesChanged signals.
Comment 1 Thomas Haller 2016-03-22 20:51:00 UTC
lgtm. Pushed one suggestion/fixup.
Comment 2 Dan Williams 2016-03-23 16:19:37 UTC
Nice fixup;  I wondered about that and looked into the classinfo but never got as far as figuring out it doesn't get freed.
Comment 3 Beniamino Galvani 2016-03-25 10:35:50 UTC
+       while (g_hash_table_iter_next (&hash_iter, (gpointer) &dbus_property_name, (gpointer) &variant)) {
+               g_variant_builder_add (&notifies, "{sv}",
+                                      dbus_property_name,
+                                      g_variant_ref (variant))

If I'm not mistaken, g_variant_build_add() calls g_variant_new() which
takes its own reference to @variant; so the g_variant_ref() is not
needed here.
Comment 4 Dan Williams 2016-03-29 15:00:47 UTC
(In reply to Beniamino Galvani from comment #3)
> +       while (g_hash_table_iter_next (&hash_iter, (gpointer)
> &dbus_property_name, (gpointer) &variant)) {
> +               g_variant_builder_add (&notifies, "{sv}",
> +                                      dbus_property_name,
> +                                      g_variant_ref (variant))
> 
> If I'm not mistaken, g_variant_build_add() calls g_variant_new() which
> takes its own reference to @variant; so the g_variant_ref() is not
> needed here.

You're entirely correct.  Fixed.
Comment 5 Dan Williams 2016-03-29 22:14:34 UTC
tested and merged to git master