From 7fb3ecc12ccd8e633c6a3bfb6ffe3456f1d95bba Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Wed, 28 Oct 2015 10:45:20 -0400 Subject: [PATCH] MetaLauncher: Don't g_error() on failure g_error() is the wrong thing to do when, for example, we can't find the DRM device, since Mutter should just fail to start rather than reporting a bug into automatic bug tracking systems. Rather than trying to decipher which errors are "expected" and which not, just make all failure paths in meta_launcher_new() return a GError out to the caller - which we make exit(1). https://bugzilla.gnome.org/show_bug.cgi?id=757311 --- src/backends/native/meta-backend-native.c | 11 +- src/backends/native/meta-launcher.c | 151 ++++++++++++++-------- src/backends/native/meta-launcher.h | 2 +- src/org.freedesktop.login1.xml | 2 + 4 files changed, 111 insertions(+), 55 deletions(-) diff --git a/src/backends/native/meta-backend-native.c b/src/backends/native/meta-backend-native.c index 96eb9fc0e..d22bed10a 100644 --- a/src/backends/native/meta-backend-native.c +++ b/src/backends/native/meta-backend-native.c @@ -37,6 +37,8 @@ #include "meta-cursor-renderer-native.h" #include "meta-launcher.h" +#include + struct _MetaBackendNativePrivate { MetaLauncher *launcher; @@ -327,8 +329,15 @@ static void meta_backend_native_init (MetaBackendNative *native) { MetaBackendNativePrivate *priv = meta_backend_native_get_instance_private (native); + GError *error = NULL; + + priv->launcher = meta_launcher_new (&error); + if (priv->launcher == NULL) + { + g_warning ("Can't initialize KMS backend: %s\n", error->message); + exit (1); + } - priv->launcher = meta_launcher_new (); priv->barrier_manager = meta_barrier_manager_native_new (); priv->up_client = up_client_new (); diff --git a/src/backends/native/meta-launcher.c b/src/backends/native/meta-launcher.c index d7da9e87c..54ab85c7c 100644 --- a/src/backends/native/meta-launcher.c +++ b/src/backends/native/meta-launcher.c @@ -54,30 +54,22 @@ struct _MetaLauncher gboolean session_active; }; -static void -report_error_and_die (const char *prefix, - GError *error) -{ - /* if a function returns due to g_return_val_if_fail, - * then the error may not be set */ - if (error) - g_error ("%s: %s", prefix, error->message); - else - g_error ("%s", prefix); - - /* the error is not freed, but it is ok as g_error aborts the process */ -} - static Login1Session * -get_session_proxy (GCancellable *cancellable) +get_session_proxy (GCancellable *cancellable, + GError **error) { char *proxy_path; char *session_id; Login1Session *session_proxy; - GError *error = NULL; if (sd_pid_get_session (getpid (), &session_id) < 0) - return NULL; + { + g_set_error (error, + G_IO_ERROR, + G_IO_ERROR_NOT_FOUND, + "Could not get session ID: %m"); + return NULL; + } proxy_path = get_escaped_dbus_path ("/org/freedesktop/login1/session", session_id); @@ -85,9 +77,9 @@ get_session_proxy (GCancellable *cancellable) G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, "org.freedesktop.login1", proxy_path, - cancellable, &error); + cancellable, error); if (!session_proxy) - report_error_and_die ("Failed getting session proxy", error); + g_prefix_error(error, "Could not get session proxy: "); free (proxy_path); @@ -95,16 +87,16 @@ get_session_proxy (GCancellable *cancellable) } static Login1Seat * -get_seat_proxy (GCancellable *cancellable) +get_seat_proxy (GCancellable *cancellable, + GError **error) { - GError *error = NULL; Login1Seat *seat = login1_seat_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM, G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, "org.freedesktop.login1", "/org/freedesktop/login1/seat/self", - cancellable, &error); + cancellable, error); if (!seat) - report_error_and_die ("Could not get seat proxy", error); + g_prefix_error(error, "Could not get seat proxy: "); return seat; } @@ -357,69 +349,114 @@ out: return path; } -static void +static gboolean get_kms_fd (Login1Session *session_proxy, - const gchar *seat_id, - int *fd_out) + const gchar *seat_id, + int *fd_out, + GError **error) { int major, minor; int fd; - gchar *path; - GError *error = NULL; - path = get_primary_gpu_path (seat_id); + g_autofree gchar *path = get_primary_gpu_path (seat_id); if (!path) - g_error ("could not find drm kms device"); + { + g_set_error (error, + G_IO_ERROR, + G_IO_ERROR_NOT_FOUND, + "could not find drm kms device"); + return FALSE; + } if (!get_device_info_from_path (path, &major, &minor)) - g_error ("Could not stat %s: %m", path); + { + g_set_error (error, + G_IO_ERROR, + G_IO_ERROR_NOT_FOUND, + "Could not get device info for path %s: %m", path); + return FALSE; + } - g_free (path); - - if (!take_device (session_proxy, major, minor, &fd, NULL, &error)) - report_error_and_die ("Could not open DRM device", error); + if (!take_device (session_proxy, major, minor, &fd, NULL, error)) + { + g_prefix_error (error, "Could not open DRM device: "); + return FALSE; + } *fd_out = fd; + + return TRUE; } static gchar * -get_seat_id (void) +get_seat_id (GError **error) { - char *session_id, *seat_id = NULL; + char *session_id, *seat_id; + int r; - if (sd_pid_get_session (0, &session_id) < 0) - return NULL; + r = sd_pid_get_session (0, &session_id); + if (r < 0) + { + g_set_error (error, + G_IO_ERROR, + G_IO_ERROR_NOT_FOUND, + "Could not get session for PID: %s", g_strerror (-r)); + return NULL; + } - /* on error the seat_id will remain NULL */ - sd_session_get_seat (session_id, &seat_id); + r = sd_session_get_seat (session_id, &seat_id); free (session_id); + if (r < 0) + { + g_set_error (error, + G_IO_ERROR, + G_IO_ERROR_NOT_FOUND, + "Could not get seat for session: %s", g_strerror (-r)); + return NULL; + } + return seat_id; } MetaLauncher * -meta_launcher_new (void) +meta_launcher_new (GError **error) { MetaLauncher *self = NULL; - Login1Session *session_proxy; - char *seat_id; - GError *error = NULL; + Login1Session *session_proxy = NULL; + Login1Seat *seat_proxy = NULL; + char *seat_id = NULL; + gboolean have_control = FALSE; int kms_fd; - session_proxy = get_session_proxy (NULL); - if (!login1_session_call_take_control_sync (session_proxy, FALSE, NULL, &error)) - report_error_and_die ("Could not take control", error); + session_proxy = get_session_proxy (NULL, error); + if (!session_proxy) + goto fail; - seat_id = get_seat_id (); + if (!login1_session_call_take_control_sync (session_proxy, FALSE, NULL, error)) + { + g_prefix_error (error, "Could not take control: "); + goto fail; + } + + have_control = TRUE; + + seat_id = get_seat_id (error); if (!seat_id) - g_error ("Failed getting seat id"); + goto fail; + + seat_proxy = get_seat_proxy (NULL, error); + if (!seat_proxy) + goto fail; + + if (!get_kms_fd (session_proxy, seat_id, &kms_fd, error)) + goto fail; - get_kms_fd (session_proxy, seat_id, &kms_fd); free (seat_id); self = g_slice_new0 (MetaLauncher); self->session_proxy = session_proxy; - self->seat_proxy = get_seat_proxy (NULL); + self->seat_proxy = seat_proxy; self->session_active = TRUE; @@ -429,8 +466,16 @@ meta_launcher_new (void) self); g_signal_connect (self->session_proxy, "notify::active", G_CALLBACK (on_active_changed), self); - return self; + + fail: + if (have_control) + login1_session_call_release_control_sync (session_proxy, NULL, NULL); + g_clear_object (&session_proxy); + g_clear_object (&seat_proxy); + free (seat_id); + + return NULL; } void diff --git a/src/backends/native/meta-launcher.h b/src/backends/native/meta-launcher.h index 16f65972a..c8790a553 100644 --- a/src/backends/native/meta-launcher.h +++ b/src/backends/native/meta-launcher.h @@ -24,7 +24,7 @@ typedef struct _MetaLauncher MetaLauncher; -MetaLauncher *meta_launcher_new (void); +MetaLauncher *meta_launcher_new (GError **error); void meta_launcher_free (MetaLauncher *self); gboolean meta_launcher_activate_session (MetaLauncher *self, diff --git a/src/org.freedesktop.login1.xml b/src/org.freedesktop.login1.xml index 924e397a4..765475132 100644 --- a/src/org.freedesktop.login1.xml +++ b/src/org.freedesktop.login1.xml @@ -9,6 +9,8 @@ + +