pkgsrc-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[pkgsrc/trunk]: pkgsrc/sysutils/upower Fix segfaults when upower is not running.



details:   https://anonhg.NetBSD.org/pkgsrc/rev/ac76e6cc18ca
branches:  trunk
changeset: 382942:ac76e6cc18ca
user:      bsiegert <bsiegert%pkgsrc.org@localhost>
date:      Wed Jul 18 19:18:07 2018 +0000

description:
Fix segfaults when upower is not running.

On my system, these patches stop xfsettingsd (from XFCE) from crashing
and dumping core during session startup. Thus, it can actually apply the
settings it is supposed to.

I just wish upstream had done a bugfix release, the patches are from 2016 :(

Fixes PR pkg/53455, which I filed :)
ok youri@

diffstat:

 sysutils/upower/Makefile                                 |    4 +-
 sysutils/upower/distinfo                                 |    5 +-
 sysutils/upower/patches/patch-libupower-glib_up-client.c |  169 +++++++++++++++
 sysutils/upower/patches/patch-libupower-glib_up-client.h |   39 +++
 sysutils/upower/patches/patch-tools_up-tool.c            |   35 +++
 5 files changed, 249 insertions(+), 3 deletions(-)

diffs (282 lines):

diff -r 31903210edc5 -r ac76e6cc18ca sysutils/upower/Makefile
--- a/sysutils/upower/Makefile  Wed Jul 18 17:55:54 2018 +0000
+++ b/sysutils/upower/Makefile  Wed Jul 18 19:18:07 2018 +0000
@@ -1,7 +1,7 @@
-# $NetBSD: Makefile,v 1.13 2018/06/05 19:14:29 youri Exp $
+# $NetBSD: Makefile,v 1.14 2018/07/18 19:18:07 bsiegert Exp $
 
 DISTNAME=      upower-0.99.4
-PKGREVISION=   3
+PKGREVISION=   4
 CATEGORIES=    sysutils
 MASTER_SITES=  https://upower.freedesktop.org/releases/
 EXTRACT_SUFX=  .tar.xz
diff -r 31903210edc5 -r ac76e6cc18ca sysutils/upower/distinfo
--- a/sysutils/upower/distinfo  Wed Jul 18 17:55:54 2018 +0000
+++ b/sysutils/upower/distinfo  Wed Jul 18 19:18:07 2018 +0000
@@ -1,6 +1,9 @@
-$NetBSD: distinfo,v 1.4 2016/03/12 11:50:08 ryoon Exp $
+$NetBSD: distinfo,v 1.5 2018/07/18 19:18:07 bsiegert Exp $
 
 SHA1 (upower-0.99.4.tar.xz) = 70beb18c218e758586fb5d98d79b5121cc4a47b1
 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
diff -r 31903210edc5 -r ac76e6cc18ca sysutils/upower/patches/patch-libupower-glib_up-client.c
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/sysutils/upower/patches/patch-libupower-glib_up-client.c  Wed Jul 18 19:18:07 2018 +0000
@@ -0,0 +1,169 @@
+$NetBSD: patch-libupower-glib_up-client.c,v 1.1 2018/07/18 19:18:07 bsiegert Exp $
+
+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;
++}
++
diff -r 31903210edc5 -r ac76e6cc18ca sysutils/upower/patches/patch-libupower-glib_up-client.h
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/sysutils/upower/patches/patch-libupower-glib_up-client.h  Wed Jul 18 19:18:07 2018 +0000
@@ -0,0 +1,39 @@
+$NetBSD: patch-libupower-glib_up-client.h,v 1.1 2018/07/18 19:18:07 bsiegert Exp $
+
+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
+
diff -r 31903210edc5 -r ac76e6cc18ca sysutils/upower/patches/patch-tools_up-tool.c
--- /dev/null   Thu Jan 01 00:00:00 1970 +0000
+++ b/sysutils/upower/patches/patch-tools_up-tool.c     Wed Jul 18 19:18:07 2018 +0000
@@ -0,0 +1,35 @@
+$NetBSD: patch-tools_up-tool.c,v 1.1 2018/07/18 19:18:07 bsiegert Exp $
+
+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;



Home | Main Index | Thread Index | Old Index