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
This commit is contained in:
Owen W. Taylor 2015-10-28 10:45:20 -04:00
parent 4c9af7267d
commit 7fb3ecc12c
4 changed files with 111 additions and 55 deletions

View File

@ -37,6 +37,8 @@
#include "meta-cursor-renderer-native.h" #include "meta-cursor-renderer-native.h"
#include "meta-launcher.h" #include "meta-launcher.h"
#include <stdlib.h>
struct _MetaBackendNativePrivate struct _MetaBackendNativePrivate
{ {
MetaLauncher *launcher; MetaLauncher *launcher;
@ -327,8 +329,15 @@ static void
meta_backend_native_init (MetaBackendNative *native) meta_backend_native_init (MetaBackendNative *native)
{ {
MetaBackendNativePrivate *priv = meta_backend_native_get_instance_private (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->barrier_manager = meta_barrier_manager_native_new ();
priv->up_client = up_client_new (); priv->up_client = up_client_new ();

View File

@ -54,30 +54,22 @@ struct _MetaLauncher
gboolean session_active; 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 * static Login1Session *
get_session_proxy (GCancellable *cancellable) get_session_proxy (GCancellable *cancellable,
GError **error)
{ {
char *proxy_path; char *proxy_path;
char *session_id; char *session_id;
Login1Session *session_proxy; Login1Session *session_proxy;
GError *error = NULL;
if (sd_pid_get_session (getpid (), &session_id) < 0) if (sd_pid_get_session (getpid (), &session_id) < 0)
{
g_set_error (error,
G_IO_ERROR,
G_IO_ERROR_NOT_FOUND,
"Could not get session ID: %m");
return NULL; return NULL;
}
proxy_path = get_escaped_dbus_path ("/org/freedesktop/login1/session", session_id); 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, G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START,
"org.freedesktop.login1", "org.freedesktop.login1",
proxy_path, proxy_path,
cancellable, &error); cancellable, error);
if (!session_proxy) if (!session_proxy)
report_error_and_die ("Failed getting session proxy", error); g_prefix_error(error, "Could not get session proxy: ");
free (proxy_path); free (proxy_path);
@ -95,16 +87,16 @@ get_session_proxy (GCancellable *cancellable)
} }
static Login1Seat * 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, Login1Seat *seat = login1_seat_proxy_new_for_bus_sync (G_BUS_TYPE_SYSTEM,
G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START,
"org.freedesktop.login1", "org.freedesktop.login1",
"/org/freedesktop/login1/seat/self", "/org/freedesktop/login1/seat/self",
cancellable, &error); cancellable, error);
if (!seat) if (!seat)
report_error_and_die ("Could not get seat proxy", error); g_prefix_error(error, "Could not get seat proxy: ");
return seat; return seat;
} }
@ -357,69 +349,114 @@ out:
return path; return path;
} }
static void static gboolean
get_kms_fd (Login1Session *session_proxy, get_kms_fd (Login1Session *session_proxy,
const gchar *seat_id, const gchar *seat_id,
int *fd_out) int *fd_out,
GError **error)
{ {
int major, minor; int major, minor;
int fd; 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) 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)) 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))
{
if (!take_device (session_proxy, major, minor, &fd, NULL, &error)) g_prefix_error (error, "Could not open DRM device: ");
report_error_and_die ("Could not open DRM device", error); return FALSE;
}
*fd_out = fd; *fd_out = fd;
return TRUE;
} }
static gchar * 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) 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; return NULL;
}
/* on error the seat_id will remain NULL */ r = sd_session_get_seat (session_id, &seat_id);
sd_session_get_seat (session_id, &seat_id);
free (session_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; return seat_id;
} }
MetaLauncher * MetaLauncher *
meta_launcher_new (void) meta_launcher_new (GError **error)
{ {
MetaLauncher *self = NULL; MetaLauncher *self = NULL;
Login1Session *session_proxy; Login1Session *session_proxy = NULL;
char *seat_id; Login1Seat *seat_proxy = NULL;
GError *error = NULL; char *seat_id = NULL;
gboolean have_control = FALSE;
int kms_fd; int kms_fd;
session_proxy = get_session_proxy (NULL); session_proxy = get_session_proxy (NULL, error);
if (!login1_session_call_take_control_sync (session_proxy, FALSE, NULL, &error)) if (!session_proxy)
report_error_and_die ("Could not take control", error); 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) 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); free (seat_id);
self = g_slice_new0 (MetaLauncher); self = g_slice_new0 (MetaLauncher);
self->session_proxy = session_proxy; self->session_proxy = session_proxy;
self->seat_proxy = get_seat_proxy (NULL); self->seat_proxy = seat_proxy;
self->session_active = TRUE; self->session_active = TRUE;
@ -429,8 +466,16 @@ meta_launcher_new (void)
self); self);
g_signal_connect (self->session_proxy, "notify::active", G_CALLBACK (on_active_changed), self); g_signal_connect (self->session_proxy, "notify::active", G_CALLBACK (on_active_changed), self);
return 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 void

View File

@ -24,7 +24,7 @@
typedef struct _MetaLauncher MetaLauncher; typedef struct _MetaLauncher MetaLauncher;
MetaLauncher *meta_launcher_new (void); MetaLauncher *meta_launcher_new (GError **error);
void meta_launcher_free (MetaLauncher *self); void meta_launcher_free (MetaLauncher *self);
gboolean meta_launcher_activate_session (MetaLauncher *self, gboolean meta_launcher_activate_session (MetaLauncher *self,

View File

@ -9,6 +9,8 @@
<method name="TakeControl"> <method name="TakeControl">
<arg name="force" type="b"/> <arg name="force" type="b"/>
</method> </method>
<method name="ReleaseControl">
</method>
<method name="TakeDevice"> <method name="TakeDevice">
<annotation name="org.gtk.GDBus.C.UnixFD" value="true"/> <annotation name="org.gtk.GDBus.C.UnixFD" value="true"/>
<arg name="major" type="u" direction="in"/> <arg name="major" type="u" direction="in"/>