Re: pkg/53455: xfsettingsd crashes

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

I’m currently trekking so I can’t review but if it works please commit :) thanks !

Envoyé de mon iPhone

> Le 17 juil. 2018 à 22:50, Benny Siegert <bsiegert%netbsd.org@localhost> a écrit :
> 
> The following reply was made to PR pkg/53455; it has been noted by GNATS.
> 
> From: Benny Siegert <bsiegert%netbsd.org@localhost>
> To: gnats-bugs%netbsd.org@localhost, pkg-manager%netbsd.org@localhost, gnats-admin%netbsd.org@localhost, 
>    pkgsrc-bugs%netbsd.org@localhost
> Cc: 
> Subject: Re: pkg/53455: xfsettingsd crashes
> Date: Tue, 17 Jul 2018 22:46:50 +0200
> 
> Proposed patch:
> 
> ? work
> Index: Makefile
> ===================================================================
> 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 $
> 
>  DISTNAME= upower-0.99.4
> -PKGREVISION= 3
> +PKGREVISION= 4
>  CATEGORIES= sysutils
>  MASTER_SITES= https://upower.freedesktop.org/releases/
>  EXTRACT_SUFX= .tar.xz
> Index: distinfo
> ===================================================================
> 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) = 6cc312d44a19ffc604c3a3282d03b9cdb6aa3638
>  SHA512 (upower-0.99.4.tar.xz) =
> b3fdee5ccf5f4d0c69e227f543272f6952119132814e27bc8f112716b8d36b5e07741a87bcf02203e80ef910cad9ddffa1adecb338c9a9aaa5e1038b62be07f3
>  Size (upower-0.99.4.tar.xz) = 426292 bytes
> +SHA1 (patch-libupower-glib_up-client.c) =
> 9eab8b87649546ab7f4d16f009f464a97142836c
> +SHA1 (patch-libupower-glib_up-client.h) =
> d4c458a6f9ce07166a7e1f6c3ad757ca731b32b7
> +SHA1 (patch-tools_up-tool.c) = e7594be12597f47b3c4f2eb65c486004804539ee
> Index: patches/patch-libupower-glib_up-client.c
> ===================================================================
> 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$
> +
> +From 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 GInitable
> +interface and provide a new constructor up_client_new_full() which accepts 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 fails.
> +This provides a more well-defined behaviour in this case as clients can check
> +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 it
> +segfaulted due to the NULL D-Bus proxy, so client code had no chance to check
> +whether the UpClient object was really valid.
> +
> +https://bugs.freedesktop.org/show_bug.cgi?id=95350
> +
> +--- 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] = { 0 };
> + static gpointer up_client_object = 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 = NULL;
> +-
> ++ UpClient *client = UP_CLIENT (initable);
> + client->priv = 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 == NULL) {
> +- g_warning ("Couldn't connect to proxy: %s", error->message);
> +- g_error_free (error);
> +- return;
> +- }
> ++ cancellable,
> ++ error);
> ++ if (client->priv->proxy == 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 = 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 fails,
> ++ % 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 != NULL) {
> + g_object_ref (up_client_object);
> + } else {
> +- up_client_object = g_object_new (UP_TYPE_CLIENT, NULL);
> +- g_object_add_weak_pointer (up_client_object, &up_client_object);
> ++ up_client_object = 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 fails,
> ++ * 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 = NULL;
> ++ UpClient *client;
> ++ client = up_client_new_full (NULL, &error);
> ++ if (client == 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
> ===================================================================
> 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$
> +
> +From 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 GInitable
> +interface and provide a new constructor up_client_new_full() which accepts 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 fails.
> +This provides a more well-defined behaviour in this case as clients can check
> +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 it
> +segfaulted due to the NULL D-Bus proxy, so client code had no chance to check
> +whether the UpClient object was really valid.
> +
> +https://bugs.freedesktop.org/show_bug.cgi?id=95350
> +
> +--- 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
> ===================================================================
> 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$
> +
> +From 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 confuse
> +users or scripts trying to parse the output).
> +
> +Use the new up_client_new_full() constructor so that we get a proper GError.
> +
> +Side issue in https://bugs.freedesktop.org/show_bug.cgi?id=95350
> +
> +--- tools/up-tool.c
> ++++ tools/up-tool.c
> +@@ -285,7 +285,12 @@ main (int argc, char **argv)
> + g_option_context_free (context);
> +
> + loop = g_main_loop_new (NULL, FALSE);
> +- client = up_client_new ();
> ++ client = up_client_new_full (NULL, &error);
> ++ if (client == NULL) {
> ++ g_warning ("Cannot connect to upowerd: %s", error->message);
> ++ g_error_free (error);
> ++ return EXIT_FAILURE;
> ++ }
> +
> + if (opt_version) {
> + gchar *daemon_version;
>