Index: ChangeLog
===================================================================
--- ChangeLog	(revision 5142)
+++ ChangeLog	(working copy)
@@ -1,3 +1,25 @@
+2007-08-09  Brian Cameron <brian.cameron@sun.com>
+
+	* common/gdm-log.h, daemon/misc.h, daemon/misc.c: Revert to
+	  previous gdm_fail function since we really want this
+	  function to cleanup and exit, not just log a message.  This
+	  was broken in the 2.19 cleanup.  Fixes bug #461822.
+	* daemon/gdm.c: Fixes to support gdm_fail and also better 
+	  string error handling for GET_XSERVER_DETAILS command.
+	* daemon/gdm-config.c, daemon/server.h: Revert code to the way
+	  the 2.18 code managed the xserver and display lists.  This
+	  also got broken in the cleanup.  It is not appropriate to 
+	  save the display in the server array because more than one
+	  display can share the same server name.  There is one 
+	  change in behavior still.  In 2.18 GDM would load all 
+	  [server-foo] sections even if they weren't actually being
+	  used.  Now it only loads the ones that are used by a
+	  display in the [servers] section.  This means gdmsetup is
+	  a bit broken for editing the server-foo sections, so 
+	  probably need to fix this later.  Still at least now it
+	  should manage the displays properly if you have multiple
+	  displays defined to use the same [server-foo].
+
 2007-08-08  Brian Cameron <brian.cameron@sun.com>
 
 	* gui/gdmsetup.c: Now properly set "Use 24 Hour Clock" to
Index: common/gdm-log.h
===================================================================
--- common/gdm-log.h	(revision 5142)
+++ common/gdm-log.h	(working copy)
@@ -37,7 +37,6 @@
 void      gdm_log_shutdown        (void);
 
 /* compatibility */
-#define   gdm_fail               g_critical
 #define   gdm_error              g_warning
 #define   gdm_info               g_message
 #define   gdm_debug              g_debug
Index: daemon/misc.h
===================================================================
--- daemon/misc.h	(revision 5142)
+++ daemon/misc.h	(working copy)
@@ -25,6 +25,8 @@
 #include "gdm.h"
 #include "display.h"
 
+void gdm_fail   (const gchar *format, ...) G_GNUC_PRINTF (1, 2);
+
 void gdm_fdprintf  (int fd, const gchar *format, ...) G_GNUC_PRINTF (2, 3);
 int gdm_fdgetc     (int fd);
 char *gdm_fdgets   (int fd);
Index: daemon/gdm.c
===================================================================
--- daemon/gdm.c	(revision 5142)
+++ daemon/gdm.c	(working copy)
@@ -109,9 +109,10 @@
 /* Global vars */
 
 gint flexi_servers         = 0; /* Number of flexi servers */
-static pid_t extra_process = 0; /* An extra process.  Used for quickie
+pid_t extra_process = 0;        /* An extra process.  Used for quickie
                                    processes, so that they also get whacked */
 static int extra_status    = 0; /* Last status from the last extra process */
+pid_t gdm_main_pid         = 0; /* PID of the main daemon */
 
 gboolean gdm_wait_for_go         = FALSE; /* wait for a GO in the fifo */
 static gboolean print_version    = FALSE; /* print version number and quit */
@@ -209,6 +210,8 @@
 		exit (EXIT_SUCCESS);
 	}
 
+	gdm_main_pid = getpid ();
+
 	if G_UNLIKELY (pid < 0)
 		gdm_fail (_("%s: fork () failed!"), "gdm_daemonify");
 
@@ -1523,6 +1526,8 @@
 	/* semi init pseudorandomness */
 	gdm_random_tick ();
 
+	gdm_main_pid = getpid ();
+
 	/* We here ensure descriptors 0, 1 and 2 */
 	ensure_desc_012 ();
 
@@ -3402,17 +3407,20 @@
 	gchar       **splitstr = g_strsplit (server, " ", 2);
 	GdmXserver   *svr      = NULL;
 
-	if (splitstr != NULL && splitstr[0] != NULL) {
+	if (gdm_vector_len (splitstr) == 2) {
 		svr = gdm_daemon_config_find_xserver ((gchar *)splitstr[0]);
 	}
 
-	if (svr != NULL && splitstr[1] != NULL) {
+	if (svr != NULL) {
 		if (g_strcasecmp (splitstr[1], "ID") == 0)
-			gdm_connection_printf (conn, "OK %s\n", svr->id);
+			gdm_connection_printf (conn, "OK %s\n",
+				svr->id ? svr->id : "(null)");
 		else if (g_strcasecmp (splitstr[1], "NAME") == 0)
-			gdm_connection_printf (conn, "OK %s\n", svr->name);
+			gdm_connection_printf (conn, "OK %s\n",
+				svr->name ? svr->name : "(null)");
 		else if (g_strcasecmp (splitstr[1], "COMMAND") == 0)
-			gdm_connection_printf (conn, "OK %s\n", svr->command);
+			gdm_connection_printf (conn, "OK %s\n",
+				svr->command ? svr->command : "(null)");
 		else if (g_strcasecmp (splitstr[1], "PRIORITY") == 0)
 			gdm_connection_printf (conn, "OK %d\n", svr->priority);
 		else if (g_strcasecmp (splitstr[1], "FLEXIBLE") == 0 &&
Index: daemon/gdm-daemon-config.c
===================================================================
--- daemon/gdm-daemon-config.c	(revision 5142)
+++ daemon/gdm-daemon-config.c	(working copy)
@@ -1068,8 +1068,7 @@
 static void
 gdm_daemon_config_load_xserver (GdmConfig  *config,
 				const char *key,
-				const char *name,
-				const char *device)
+				const char *name)
 {
 	GdmXserver     *svr;
 	int             n;
@@ -1084,12 +1083,7 @@
 
 	svr = g_new0 (GdmXserver, 1);
 	svr->id = g_strdup (name);
-	svr->device = g_strdup (device);
 
-	if (isdigit (*key)) {
-		svr->number = atoi (key);
-	}
-
 	group = g_strdup_printf ("server-%s", name);
 
 	/* string */
@@ -1102,11 +1096,6 @@
 		svr->command = g_strdup (gdm_config_value_get_string (value));
 	}
 
-	if (device != NULL)
-		gdm_debug ("Adding new server id=%s name=%s command=%s device=%s", name, svr->name, svr->command, svr->device);
-	else
-		gdm_debug ("Adding new server id=%s name=%s command=%s", name, svr->name, svr->command);
-
 	/* bool */
 	res = gdm_config_get_value (config, group, "flexible", &value);
 	if (res) {
@@ -1166,7 +1155,6 @@
 		g_free (xsvr->id);
 		g_free (xsvr->name);
 		g_free (xsvr->command);
-		g_free (xsvr->device);
 	}
 
 	if (xservers != NULL) {
@@ -1185,7 +1173,6 @@
 		svr->id        = g_strdup (GDM_STANDARD);
 		svr->name      = g_strdup ("Standard server");
 		svr->command   = g_strdup (X_SERVER);
-		svr->device    = NULL;
 		svr->flexible  = TRUE;
 		svr->choosable = TRUE;
 		svr->handled   = TRUE;
@@ -1198,37 +1185,38 @@
 static void
 load_xservers_group (GdmConfig *config)
 {
-        char     **keys;
-        gsize      len;
-        int        i;
+	char     **keys;
+	gsize      len;
+	int        i;
 
-        keys = gdm_config_get_keys_for_group (config, GDM_CONFIG_GROUP_SERVERS, &len, NULL);
+	keys = gdm_config_get_keys_for_group (config,
+		GDM_CONFIG_GROUP_SERVERS, &len, NULL);
 
-        /* now construct entries for these groups */
-        for (i = 0; i < len; i++) {
-                GdmConfigEntry  entry;
-                GdmConfigValue *value;
-                gboolean        res;
-                int             j;
-                char           *new_group;
-        	char          **value_list;
-        	const char     *display_value;
-        	const char     *name;
-        	const char     *device_name = NULL;
+	/* now construct entries for these groups */
+	for (i = 0; i < len; i++) {
+		GdmConfigEntry   entry;
+		GdmConfigValue  *value;
+		const char      *display_value;
+		const char      *name;
+		char           **value_list;
+		char            *new_group;
+		int              j;
+		gboolean         res;
 
-                entry.group = GDM_CONFIG_GROUP_SERVERS;
-                entry.key = keys[i];
-                entry.type = GDM_CONFIG_VALUE_STRING;
-                entry.default_value = NULL;
-                entry.id = GDM_CONFIG_INVALID_ID;
+	        entry.group         = GDM_CONFIG_GROUP_SERVERS;
+		entry.key           = keys[i];
+		entry.type          = GDM_CONFIG_VALUE_STRING;
+		entry.default_value = NULL;
+		entry.id            = GDM_CONFIG_INVALID_ID;
 
-                gdm_config_add_entry (config, &entry);
-                gdm_config_process_entry (config, &entry, NULL);
+		gdm_config_add_entry (config, &entry);
+		gdm_config_process_entry (config, &entry, NULL);
 
-                res = gdm_config_get_value (config, entry.group, entry.key, &value);
-                if (! res) {
-                        continue;
-                }
+		res = gdm_config_get_value (config, entry.group, entry.key,
+			&value);
+		if (! res) {
+			continue;
+		}
 
 		display_value = gdm_config_value_get_string (value);
 		value_list = g_strsplit (display_value, " ", -1);
@@ -1240,17 +1228,6 @@
 		
 		name = value_list[0];
 
-		/*
-		 * Allow an optional device to be passed in as a 2nd argument
-		 * with the format "device=/dev/foo".
- 		 * In the future, if more per-display configuration is desired, 
-		 * this can be made more sophisticated to handle additional
-		 * arguments.
-		 */
-		if (value_list[1] != NULL &&
-		    strncmp (value_list[1], "device=", strlen ("device=")) == 0)
-			device_name = value_list[1] + strlen ("device=");
-
 		/* Skip servers marked as inactive */
 		if (name == NULL || name[0] == '\0' ||
 		    g_ascii_strcasecmp (name, "inactive") == 0) {
@@ -1259,22 +1236,22 @@
 			continue;
 		}
 
-                new_group = g_strdup_printf ("server-%s", name);
-                for (j = 0; j < G_N_ELEMENTS (gdm_daemon_server_config_entries); j++) {
-                        GdmConfigEntry *srv_entry;
-                        if (gdm_daemon_server_config_entries[j].key == NULL) {
-                                continue;
-                        }
-                        srv_entry = gdm_config_entry_copy (&gdm_daemon_server_config_entries[j]);
-                        g_free (srv_entry->group);
-                        srv_entry->group = g_strdup (new_group);
-                        gdm_config_process_entry (config, srv_entry, NULL);
-                        gdm_config_entry_free (srv_entry);
-                }
+		new_group = g_strdup_printf ("server-%s", name);
+		for (j = 0; j < G_N_ELEMENTS (gdm_daemon_server_config_entries); j++) {
+			GdmConfigEntry *srv_entry;
+			if (gdm_daemon_server_config_entries[j].key == NULL) {
+				continue;
+			}
+			srv_entry = gdm_config_entry_copy (&gdm_daemon_server_config_entries[j]);
+			g_free (srv_entry->group);
+			srv_entry->group = g_strdup (new_group);
+			gdm_config_process_entry (config, srv_entry, NULL);
+			gdm_config_entry_free (srv_entry);
+		}
 		g_free (new_group);
 
 		/* Now we can add this server */
-		gdm_daemon_config_load_xserver (config, entry.key, name, device_name);
+		gdm_daemon_config_load_xserver (config, entry.key, name);
                 gdm_config_value_free (value);
 		g_strfreev (value_list);
         }
@@ -1482,31 +1459,73 @@
 static void
 gdm_daemon_config_load_displays (GdmConfig *config)
 {
-	GSList *l;
+	char       **keys;
+	const char *display_value;
+	gsize      len;
+	int        i;
 
-	for (l = xservers; l != NULL; l = l->next) {
-		GdmXserver *xserver;
-		GdmDisplay *disp;
+	keys = gdm_config_get_keys_for_group (config,
+		GDM_CONFIG_GROUP_SERVERS, &len, NULL);
 
-		xserver = l->data;
+        /* now construct entries for these groups */
+        for (i = 0; i < len; i++) {
+		GdmDisplay     *disp;
+		GdmConfigValue *value;
+		const char      *name        = NULL;
+		const char      *device_name = NULL;
+		char           **value_list;
+		int             keynum;
+		gboolean        res;
 
-		gdm_debug ("Loading display for key '%d'", xserver->number);
+		name   = keys[i];
 
-		gdm_debug ("Got name for server: %s", xserver->id);
-		/* Do not add if already in the list */
-		if (have_display_for_number (xserver->number)) {
+		if (!isdigit (*name)) {
 			continue;
 		}
 
-		disp = gdm_display_alloc (xserver->number, xserver->id, xserver->device);
+		keynum = atoi (name);
+
+                res = gdm_config_get_value (config, GDM_CONFIG_GROUP_SERVERS,
+			keys[i], &value);
+		if (! res) {
+			continue;
+		}
+
+                display_value = gdm_config_value_get_string (value);
+		value_list    = g_strsplit (display_value, " ", -1);
+
+		if (value_list == NULL || value_list[0] == '\0') {
+			gdm_config_value_free (value);
+			g_strfreev (value_list);
+			continue;
+		}
+
+		/*
+		 * Allow an optional device to be passed in as a 2nd argument
+		 * with the format "device=/dev/foo".
+		 * In the future, if more per-display configuration is desired, 
+		 * this can be made more sophisticated to handle additional
+		 * arguments.
+		 */
+		if (value_list[1] != NULL &&
+		    strncmp (value_list[1], "device=", strlen ("device=")) == 0)
+		        device_name = value_list[1] + strlen ("device=");
+
+		gdm_debug ("Loading display for key '%d'", keynum);
+
+		disp = gdm_display_alloc (keynum, name, device_name);
 		if (disp == NULL) {
+			g_strfreev (value_list);
 			continue;
 		}
 
-		displays = g_slist_insert_sorted (displays, disp, gdm_daemon_config_compare_displays);
-		if (xserver->number > high_display_num) {
-			high_display_num = xserver->number;
+		displays = g_slist_insert_sorted (displays, disp,
+			gdm_daemon_config_compare_displays);
+		if (keynum > high_display_num) {
+			high_display_num = keynum;
 		}
+
+		g_strfreev (value_list);
 	}
 }
 
Index: daemon/server.h
===================================================================
--- daemon/server.h	(revision 5142)
+++ daemon/server.h	(working copy)
@@ -30,12 +30,10 @@
 	char *id;
 	char *name;
 	char *command;
-	char *device;
 	gboolean flexible;
 	gboolean choosable; /* not implemented yet */
 	gboolean chooser; /* instead of greeter, run chooser */
 	gboolean handled;
-	int number;
 	int priority;
 };
 
Index: daemon/misc.c
===================================================================
--- daemon/misc.c	(revision 5142)
+++ daemon/misc.c	(working copy)
@@ -62,6 +62,8 @@
 #include "gdm-daemon-config.h"
 
 extern char **environ;
+extern pid_t gdm_main_pid;
+extern pid_t extra_process;
 
 #ifdef ENABLE_IPV6
 
@@ -2168,4 +2170,52 @@
 #endif
 }
 
+/**
+ * gdm_fail:
+ * @format: printf style format string
+ * @...: Optional arguments
+ *
+ * Logs fatal error condition and aborts master daemon.  Also sleeps
+ * for 30 seconds to avoid looping if gdm is started by init.
+ */
+void
+gdm_fail (const gchar *format, ...)
+{
+    va_list args;
+    char *s;
+
+    va_start (args, format);
+    s = g_strdup_vprintf (format, args);
+    va_end (args);
+
+    /* Log to both syslog and stderr */
+    gdm_error (s);
+    if (getpid () == gdm_main_pid) {
+	    gdm_fdprintf (2, "%s\n", s);
+    }
+
+    g_free (s);
+
+    /* If main process do final cleanup to kill all processes */
+    if (getpid () == gdm_main_pid) {
+            gdm_final_cleanup ();
+    } else if ( ! gdm_slave_final_cleanup ()) {
+            /* If we weren't even a slave do some random cleanup only */
+            /* FIXME: is this all fine? */
+            gdm_sigchld_block_push ();
+            if (extra_process > 1 && extra_process != getpid ()) {
+                    /* we sigterm extra processes, and we don't wait */
+                    kill (-(extra_process), SIGTERM);
+                    extra_process = 0;
+            }
+            gdm_sigchld_block_pop ();
+    }
+
+    /* Slow down respawning if we're started from init */
+    if (getppid () == 1)
+        sleep (30);
+
+    exit (EXIT_FAILURE);
+}
+
 /* EOF */
