Re: pkg/53455: xfsettingsd crashes

[Youri Mouton <youri.mout%gmail.com@localhost>]

The following reply was made to PR pkg/53455; it has been noted by GNATS.

From: Youri Mouton <youri.mout%gmail.com@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: youri%NetBSD.org@localhost, gnats-admin%netbsd.org@localhost, pkgsrc-bugs%netbsd.org@localhost,
 bsiegert%NetBSD.org@localhost
Subject: Re: pkg/53455: xfsettingsd crashes
Date: Wed, 18 Jul 2018 09:37:56 +0200

 I=E2=80=99m currently trekking so I can=E2=80=99t review but if it works ple=
 ase commit :) thanks !
 
 Envoy=C3=A9 de mon iPhone
 
 > Le 17 juil. 2018 =C3=A0 22:50, Benny Siegert <bsiegert%netbsd.org@localhost> a =C3=A9=
 crit :
 >=20
 > The following reply was made to PR pkg/53455; it has been noted by GNATS.
 >=20
 > From: Benny Siegert <bsiegert%netbsd.org@localhost>
 > To: gnats-bugs%netbsd.org@localhost, pkg-manager%netbsd.org@localhost, gnats-admin%netbsd.org@localhost,=
 =20
 >    pkgsrc-bugs%netbsd.org@localhost
 > Cc:=20
 > Subject: Re: pkg/53455: xfsettingsd crashes
 > Date: Tue, 17 Jul 2018 22:46:50 +0200
 >=20
 > Proposed patch:
 >=20
 > ? work
 > Index: Makefile
 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 > RCS file: /cvsroot/pkgsrc/sysutils/upower/Makefile,v
 > retrieving revision 1.13
 > diff -u -r1.13 Makefile
 > --- Makefile 5 Jun 2018 19:14:29 -0000 1.13
 > +++ Makefile 17 Jul 2018 20:46:08 -0000
 > @@ -1,7 +1,7 @@
 >  # $NetBSD: Makefile,v 1.13 2018/06/05 19:14:29 youri Exp $
 >=20
 >  DISTNAME=3D upower-0.99.4
 > -PKGREVISION=3D 3
 > +PKGREVISION=3D 4
 >  CATEGORIES=3D sysutils
 >  MASTER_SITES=3D https://upower.freedesktop.org/releases/
 >  EXTRACT_SUFX=3D .tar.xz
 > Index: distinfo
 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 > RCS file: /cvsroot/pkgsrc/sysutils/upower/distinfo,v
 > retrieving revision 1.4
 > diff -u -r1.4 distinfo
 > --- distinfo 12 Mar 2016 11:50:08 -0000 1.4
 > +++ distinfo 17 Jul 2018 20:46:08 -0000
 > @@ -4,3 +4,6 @@
 >  RMD160 (upower-0.99.4.tar.xz) =3D 6cc312d44a19ffc604c3a3282d03b9cdb6aa363=
 8
 >  SHA512 (upower-0.99.4.tar.xz) =3D
 > b3fdee5ccf5f4d0c69e227f543272f6952119132814e27bc8f112716b8d36b5e07741a87bc=
 f02203e80ef910cad9ddffa1adecb338c9a9aaa5e1038b62be07f3
 >  Size (upower-0.99.4.tar.xz) =3D 426292 bytes
 > +SHA1 (patch-libupower-glib_up-client.c) =3D
 > 9eab8b87649546ab7f4d16f009f464a97142836c
 > +SHA1 (patch-libupower-glib_up-client.h) =3D
 > d4c458a6f9ce07166a7e1f6c3ad757ca731b32b7
 > +SHA1 (patch-tools_up-tool.c) =3D e7594be12597f47b3c4f2eb65c486004804539ee=
 
 > Index: patches/patch-libupower-glib_up-client.c
 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 > RCS file: patches/patch-libupower-glib_up-client.c
 > diff -N patches/patch-libupower-glib_up-client.c
 > --- /dev/null 1 Jan 1970 00:00:00 -0000
 > +++ patches/patch-libupower-glib_up-client.c 17 Jul 2018 20:46:08 -0000
 > @@ -0,0 +1,169 @@
 > +$NetBSD$
 > +
 > +=46rom 932a6a39e35754be571e1274aec4730fd42dba13 Mon Sep 17 00:00:00 2001
 > +From: Martin Pitt <martin.pitt%ubuntu.com@localhost>
 > +Date: Wed, 18 May 2016 09:22:43 +0200
 > +Subject: [PATCH 1/9] lib: Add proper error and cancellable handling to
 > + UpClient constructor
 > +
 > +A GObject's _init() should never fail or block, but this is currently the=
  case
 > +as up_client_init() connects to upowerd on D-Bus. Convert this to the GIn=
 itable
 > +interface and provide a new constructor up_client_new_full() which accept=
 s a
 > +GCancellable and GError, so that clients can do proper error handling
 > +and reporting.
 > +
 > +This changes up_client_new() to return NULL when connecting to upowerd fa=
 ils.
 > +This provides a more well-defined behaviour in this case as clients can c=
 heck
 > +for this and our methods stop segfaulting as they have checks like
 > +
 > +   g_return_val_if_fail (UP_IS_CLIENT (client), ...)
 > +
 > +Previously we returned a valid object, but trying to call any method on i=
 t
 > +segfaulted due to the NULL D-Bus proxy, so client code had no chance to c=
 heck
 > +whether the UpClient object was really valid.
 > +
 > +https://bugs.freedesktop.org/show_bug.cgi?id=3D95350
 > +
 > +--- libupower-glib/up-client.c
 > ++++ libupower-glib/up-client.c
 > +@@ -39,9 +39,10 @@
 > + #include "up-daemon-generated.h"
 > + #include "up-device.h"
 > +
 > +-static void up_client_class_init (UpClientClass *klass);
 > +-static void up_client_init (UpClient *client);
 > +-static void up_client_finalize (GObject *object);
 > ++static void up_client_class_init (UpClientClass *klass);
 > ++static void up_client_initable_iface_init (GInitableIface *iface);
 > ++static void up_client_init (UpClient *client);
 > ++static void up_client_finalize (GObject *object);
 > +
 > + #define UP_CLIENT_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o),
 > UP_TYPE_CLIENT, UpClientPrivate))
 > +
 > +@@ -73,7 +74,8 @@ enum {
 > + static guint signals [UP_CLIENT_LAST_SIGNAL] =3D { 0 };
 > + static gpointer up_client_object =3D NULL;
 > +
 > +-G_DEFINE_TYPE (UpClient, up_client, G_TYPE_OBJECT)
 > ++G_DEFINE_TYPE_WITH_CODE (UpClient, up_client, G_TYPE_OBJECT,
 > ++                         G_IMPLEMENT_INTERFACE(G_TYPE_INITABLE,
 > up_client_initable_iface_init))
 > +
 > + /**
 > +  * up_client_get_devices:
 > +@@ -434,11 +436,10 @@ up_client_class_init (UpClientClass *klass)
 > +  * up_client_init:
 > +  * @client: This class instance
 > +  */
 > +-static void
 > +-up_client_init (UpClient *client)
 > ++static gboolean
 > ++up_client_initable_init (GInitable *initable, GCancellable
 > *cancellable, GError **error)
 > + {
 > +- GError *error =3D NULL;
 > +-
 > ++ UpClient *client =3D UP_CLIENT (initable);
 > + client->priv =3D UP_CLIENT_GET_PRIVATE (client);
 > +
 > + /* connect to main interface */
 > +@@ -446,13 +447,10 @@ up_client_init (UpClient *client)
 > + G_DBUS_PROXY_FLAGS_NONE,
 > + "org.freedesktop.UPower",
 > + "/org/freedesktop/UPower",
 > +- NULL,
 > +- &error);
 > +- if (client->priv->proxy =3D=3D NULL) {
 > +- g_warning ("Couldn't connect to proxy: %s", error->message);
 > +- g_error_free (error);
 > +- return;
 > +- }
 > ++ cancellable,
 > ++ error);
 > ++ if (client->priv->proxy =3D=3D NULL)
 > ++ return FALSE;
 > +
 > + /* all callbacks */
 > + g_signal_connect (client->priv->proxy, "device-added",
 > +@@ -461,6 +459,23 @@ up_client_init (UpClient *client)
 > +   G_CALLBACK (up_device_removed_cb), client);
 > + g_signal_connect (client->priv->proxy, "notify",
 > +   G_CALLBACK (up_client_notify_cb), client);
 > ++
 > ++ return TRUE;
 > ++}
 > ++
 > ++static void
 > ++up_client_initable_iface_init (GInitableIface *iface)
 > ++{
 > ++ iface->init =3D up_client_initable_init;
 > ++}
 > ++
 > ++/*
 > ++ * up_client_init:
 > ++ * @client: This class instance
 > ++ */
 > ++static void
 > ++up_client_init (UpClient *client)
 > ++{
 > + }
 > +
 > + /*
 > +@@ -482,23 +497,52 @@ up_client_finalize (GObject *object)
 > + }
 > +
 > + /**
 > +- * up_client_new:
 > ++ * up_client_new_full:
 > ++ * @cancellable: (allow-none): A #GCancellable or %NULL.
 > ++ * @error: Return location for error or %NULL.
 > +  *
 > +- * Creates a new #UpClient object.
 > ++ * Creates a new #UpClient object. If connecting to upowerd on D-Bus fai=
 ls,
 > ++ % this returns %NULL and sets @error.
 > +  *
 > +- * Return value: a new UpClient object.
 > ++ * Return value: a new UpClient object, or %NULL on failure.
 > +  *
 > +- * Since: 0.9.0
 > ++ * Since: 0.99.5
 > +  **/
 > + UpClient *
 > +-up_client_new (void)
 > ++up_client_new_full (GCancellable *cancellable, GError **error)
 > + {
 > + if (up_client_object !=3D NULL) {
 > + g_object_ref (up_client_object);
 > + } else {
 > +- up_client_object =3D g_object_new (UP_TYPE_CLIENT, NULL);
 > +- g_object_add_weak_pointer (up_client_object, &up_client_object);
 > ++ up_client_object =3D g_initable_new (UP_TYPE_CLIENT, cancellable, error=
 , NULL);
 > ++ if (up_client_object)
 > ++ g_object_add_weak_pointer (up_client_object, &up_client_object);
 > + }
 > + return UP_CLIENT (up_client_object);
 > + }
 > +
 > ++/**
 > ++ * up_client_new:
 > ++ *
 > ++ * Creates a new #UpClient object. If connecting to upowerd on D-Bus fai=
 ls,
 > ++ * this returns %NULL and prints out a warning with the error message.
 > ++ * Consider using up_client_new_full() instead which allows you to
 > handle errors
 > ++ * and cancelling long operations yourself.
 > ++ *
 > ++ * Return value: a new UpClient object, or %NULL on failure.
 > ++ *
 > ++ * Since: 0.9.0
 > ++ **/
 > ++UpClient *
 > ++up_client_new (void)
 > ++{
 > ++ GError *error =3D NULL;
 > ++ UpClient *client;
 > ++ client =3D up_client_new_full (NULL, &error);
 > ++ if (client =3D=3D NULL) {
 > ++ g_warning ("Couldn't connect to proxy: %s", error->message);
 > ++ g_error_free (error);
 > ++ }
 > ++ return client;
 > ++}
 > ++
 > Index: patches/patch-libupower-glib_up-client.h
 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 > RCS file: patches/patch-libupower-glib_up-client.h
 > diff -N patches/patch-libupower-glib_up-client.h
 > --- /dev/null 1 Jan 1970 00:00:00 -0000
 > +++ patches/patch-libupower-glib_up-client.h 17 Jul 2018 20:46:09 -0000
 > @@ -0,0 +1,39 @@
 > +$NetBSD$
 > +
 > +=46rom 932a6a39e35754be571e1274aec4730fd42dba13 Mon Sep 17 00:00:00 2001
 > +From: Martin Pitt <martin.pitt%ubuntu.com@localhost>
 > +Date: Wed, 18 May 2016 09:22:43 +0200
 > +Subject: [PATCH 1/9] lib: Add proper error and cancellable handling to
 > + UpClient constructor
 > +
 > +A GObject's _init() should never fail or block, but this is currently the=
  case
 > +as up_client_init() connects to upowerd on D-Bus. Convert this to the GIn=
 itable
 > +interface and provide a new constructor up_client_new_full() which accept=
 s a
 > +GCancellable and GError, so that clients can do proper error handling
 > +and reporting.
 > +
 > +This changes up_client_new() to return NULL when connecting to upowerd fa=
 ils.
 > +This provides a more well-defined behaviour in this case as clients can c=
 heck
 > +for this and our methods stop segfaulting as they have checks like
 > +
 > +   g_return_val_if_fail (UP_IS_CLIENT (client), ...)
 > +
 > +Previously we returned a valid object, but trying to call any method on i=
 t
 > +segfaulted due to the NULL D-Bus proxy, so client code had no chance to c=
 heck
 > +whether the UpClient object was really valid.
 > +
 > +https://bugs.freedesktop.org/show_bug.cgi?id=3D95350
 > +
 > +--- libupower-glib/up-client.h
 > ++++ libupower-glib/up-client.h
 > +@@ -72,6 +72,7 @@ typedef struct
 > + /* general */
 > + GType up_client_get_type (void);
 > + UpClient *up_client_new (void);
 > ++UpClient *up_client_new_full (GCancellable *cancellable, GError **error)=
 ;
 > +
 > + /* sync versions */
 > + UpDevice * up_client_get_display_device (UpClient *client);
 > +--
 > +2.8.1
 > +
 > Index: patches/patch-tools_up-tool.c
 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 > RCS file: patches/patch-tools_up-tool.c
 > diff -N patches/patch-tools_up-tool.c
 > --- /dev/null 1 Jan 1970 00:00:00 -0000
 > +++ patches/patch-tools_up-tool.c 17 Jul 2018 20:46:09 -0000
 > @@ -0,0 +1,35 @@
 > +$NetBSD$
 > +
 > +=46rom 4e83fabac13250fdc61ef5db817e82c32b7b301b Mon Sep 17 00:00:00 2001
 > +From: Martin Pitt <martin.pitt%ubuntu.com@localhost>
 > +Date: Tue, 17 May 2016 15:36:21 +0200
 > +Subject: [PATCH 2/9] up-tool: Exit early when connecting to upower fails
 > +
 > +This avoids spewing dozens of assertions like
 > +
 > +   libupower-glib-CRITICAL **: up_client_get_devices: assertion
 > 'UP_IS_CLIENT (client)' failed
 > +   libupower-glib-CRITICAL **: up_device_get_object_path: assertion
 > 'UP_IS_DEVICE (device)' failed
 > +
 > +and useless default values and then exiting successfully (which might con=
 fuse
 > +users or scripts trying to parse the output).
 > +
 > +Use the new up_client_new_full() constructor so that we get a proper GErr=
 or.
 > +
 > +Side issue in https://bugs.freedesktop.org/show_bug.cgi?id=3D95350
 > +
 > +--- tools/up-tool.c
 > ++++ tools/up-tool.c
 > +@@ -285,7 +285,12 @@ main (int argc, char **argv)
 > + g_option_context_free (context);
 > +
 > + loop =3D g_main_loop_new (NULL, FALSE);
 > +- client =3D up_client_new ();
 > ++ client =3D up_client_new_full (NULL, &error);
 > ++ if (client =3D=3D NULL) {
 > ++ g_warning ("Cannot connect to upowerd: %s", error->message);
 > ++ g_error_free (error);
 > ++ return EXIT_FAILURE;
 > ++ }
 > +
 > + if (opt_version) {
 > + gchar *daemon_version;
 >=20