GNOME.org
 

Re: [gdm-list] Help with GDM 2.20.2: gdm ignoring custom server config

Thanks!!

I tried your patch, the ChangeLog did not patch smoothly but no
matter, the source itself did.

However, the patch gdm dies with a seg fault in parsing the config in
gdm-daemon-config.c at line 1211 in load_xservers_group().  I compiled
with debug to see if there was something obvious to me.  Does this
help?  I don't think this is on my end (e.g. in the patching).  The
loop looks like:

			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 = xserver_group;
				gdm_config_process_entry (config, srv_entry, NULL);
				gdm_config_entry_free (srv_entry);
			}


BTW, the [servers] section in my gdm.conf is default, that is to say,
empty.

On Thu, Jan 03, 2008 at 12:33:10AM -0600, Brian Cameron wrote: 
> Martin:
> 
> >I recently upgraded to GDM 2.20.2 via Debian/Lenny.  I now notice that
> >my custom [server-foo] instances are being ignored without messages.
> >I turned on debug to look for a problem.
> 
> GDM 2.20 has a bug that [server-foo] sections are not read by the daemon
> unless they are actually used by a display in the [servers] section.
> Refer to bug #462613.
> 
>   http://bugzilla.gnome.org/show_bug.cgi?id=462613
> 
> For example, if you have the following in your [servers] section:
> 
> [servers]
> 0=Standard
> 1=Foo
> 
> Then *only* [server-Standard} and [server-Foo} will be read by the
> daemon and recognized.  If you have other "server-xyz" sections, they
> will be ignored.
> 
> This was recently fixed in the 2.20 branch so now it should work
> better and read in all the server-* sections even if they aren't
> used in the [servers] section.
> 
> You don't mention the contents of the "[servers]" section of your
> configuration file, so I'm not 100% sure this is your problem, but
> I'm guessing this known issue is what you are seeing?
> 
> Could you try the attached patch and see if it fixes your issues?
> This fix will go into the next release of GDM 2.20.
> 
> If this doesn't fix your problem, please provide additional details
> and we can look into the problems further.
> 
> Thanks,
> 
> Brian
> 
> 
> 
> >E.g 
> >
> >>gdmflexiserver -c GET_SERVER_LIST
> >OK Standard
> >
> >but my gdm.conf has:
> >
> >[server-Standard]
> >name=Standard server
> >command=/usr/bin/X -br -audit 0 -layout Single
> >flexible=true
> >
> >[server-Dual]
> >name=Dual Head
> >command=/usr/bin/X -br -audit 0 -layout Dual
> >flexible=true
> >
> >[server-Xinerama]
> >name=Xinerama
> >command=/usr/bin/X -br -audit 0 -layout Xinerama
> >flexible=true
> >
> >If I do:
> >
> >>gdmflexiserver -c "GET_SERVER_DETAILS Standard COMMAND"
> >OK /usr/bin/X -br -audit 0 -layout Single
> >
> >which shows my nonstandard command.  I tried changing order
> >of the stanzas while grabbing at straws.
> >
> >Is this a Debian problem? This all worked fine on an earlier version.
> >Does anyone know what I'm missing?  A new default feature?  I tried
> >scanning the changelogs but didn't find anything relevant.
> >
> >Thanks in advance!!
> >
> >--Martin
> >
> >
> >P.S. I'm not suscribed so please cc me . . . 
> >_______________________________________________
> >gdm-list mailing list
> >gdm-list gnome org
> >http://mail.gnome.org/mailman/listinfo/gdm-list
> 

> Index: ChangeLog
> ===================================================================
> --- ChangeLog	(revision 5598)
> +++ ChangeLog	(working copy)
> @@ -1,3 +1,20 @@
> +2008-01-02 Brian Cameron <brian cameron sun com>
> +
> +	* common/gdm-config.h, common/gdm-config.c:  Add function for
> +	  getting a list of server-foo sections from the configuration
> +	  files.  Also fix bug that was causing the comparison of
> +	  STRING_ARRAY keys to not work.  This was causing gdmsetup
> +	  to behave badly when changing the value of the
> +	  Halt/Shutdown/Reboot commands.  Fixes bug #502074.
> +	* daemon/gdm-daemon-config.c: Now use the above functions to
> +	  get the server-foo section names rather than just parsing the
> +	  ones used in the [server] section.  This makes gdmsetup work
> +	  better and fixes bug #462613.
> +	* daemon/gdm.c, daemon/gdm-daemon-config.c: Set debug as soon as
> +	  the configuration value is read, not after the configuration
> +	  file parsing is done.  This is useful for debugging problems
> +	  while parsing.
> +
>  2007-12-21 Brian Cameron <brian cameron sun com>
>  
>  	* daemon/gdm.c, daemon/slave.c: Fix some casting issues pointed out
> @@ -7,7 +24,12 @@
>  
>  	* gui/gdmflexiserver.c: Revert to the old logic for handling 
>  	  options.  This uses g_option_context* rather than gtk_init.
> -	  This fixes bug #438939.
> +	  Now we only call gtk_init when not handling the "--command"
> +	  argument.  This fixes bug #438939.  The problem here is that
> +	  you should be able to run gdmflexiserver with the --command
> +	  option even if running setuid or setgid, but the gtk_init
> +	  function doesn't allow this.  So we need to use the g_option
> +	  functions instead.
>  
>  2007-12-10 Brian Cameron <brian cameron sun com>
>  
> Index: common/gdm-config.h
> ===================================================================
> --- common/gdm-config.h	(revision 5598)
> +++ common/gdm-config.h	(working copy)
> @@ -126,6 +126,7 @@
>  							  const gchar     *group_name,
>  							  gsize           *length,
>  							  GError         **error);
> +GPtrArray *            gdm_config_get_server_groups      (GdmConfig       *config);
>  
>  gboolean               gdm_config_peek_value             (GdmConfig             *config,
>  							  const char            *group,
> Index: common/gdm-config.c
> ===================================================================
> --- common/gdm-config.c	(revision 5598)
> +++ common/gdm-config.c	(working copy)
> @@ -286,7 +286,7 @@
>  			int   res;
>  
>  			str_a = gdm_config_value_to_string (value_a);
> -			str_b = gdm_config_value_to_string (value_a);
> +			str_b = gdm_config_value_to_string (value_b);
>  			res = safe_strcmp (str_a, str_b);
>  			g_free (str_a);
>  			g_free (str_b);
> @@ -615,6 +615,71 @@
>  	g_slice_free (GdmConfig, config);
>  }
>  
> +static void
> +add_server_group_once (GPtrArray *server_groups, char *group)
> +{
> +	int i;
> +
> +	for (i=0; i < server_groups->len; i++) {
> +		if (strcmp (g_ptr_array_index (server_groups, i), group) == 0) {
> +			g_debug ("server group %s already exists, skipping",
> +				group);
> +			return;
> +		}
> +	}
> +	g_ptr_array_add (server_groups, g_strdup (group));
> +}
> +
> +GPtrArray *
> +gdm_config_get_server_groups (GdmConfig *config)
> +{
> +	GPtrArray       *server_groups;
> +	GError          *error;
> +	char           **groups;
> +	gsize            len;
> +	int              i;
> +	
> +	server_groups = g_ptr_array_new ();
> +
> +	if (config->mandatory_key_file != NULL) {
> +		groups = g_key_file_get_groups (config->mandatory_key_file, &len);
> +
> +		for (i = 0; i < len; i++)
> +		{
> +			if (g_str_has_prefix (groups[i], "server-")) {
> +				add_server_group_once (server_groups, groups[i]);
> +			}
> +		}
> +		g_strfreev (groups);
> +	}
> +
> +	if (config->default_key_file != NULL) {
> +		groups = g_key_file_get_groups (config->default_key_file, &len);
> +
> +		for (i = 0; i < len; i++)
> +		{
> +			if (g_str_has_prefix (groups[i], "server-")) {
> +				add_server_group_once (server_groups, groups[i]);
> +			}
> +		}
> +		g_strfreev (groups);
> +	}
> +
> +	if (config->custom_key_file != NULL) {
> +		groups = g_key_file_get_groups (config->custom_key_file, &len);
> +
> +		for (i = 0; i < len; i++)
> +		{
> +			if (g_str_has_prefix (groups[i], "server-")) {
> +				add_server_group_once (server_groups, groups[i]);
> +			}
> +		}
> +		g_strfreev (groups);
> +	}
> +
> +	return server_groups;
> +}
> +
>  const GdmConfigEntry *
>  gdm_config_lookup_entry (GdmConfig  *config,
>  			 const char *group,
> Index: daemon/gdm.c
> ===================================================================
> --- daemon/gdm.c	(revision 5598)
> +++ daemon/gdm.c	(working copy)
> @@ -1575,7 +1575,6 @@
>  	gdm_log_init ();
>  	/* Parse configuration file */
>  	gdm_daemon_config_parse (config_file, no_console);
> -	gdm_log_set_debug (gdm_daemon_config_get_bool_for_id (GDM_ID_DEBUG));
>  
>  	/* XDM compliant error message */
>  	if G_UNLIKELY (getuid () != 0) {
> Index: daemon/gdm-daemon-config.c
> ===================================================================
> --- daemon/gdm-daemon-config.c	(revision 5598)
> +++ daemon/gdm-daemon-config.c	(working copy)
> @@ -1067,12 +1067,11 @@
>   */
>  static void
>  gdm_daemon_config_load_xserver (GdmConfig  *config,
> -				const char *key,
> +				const char *group,
>  				const char *name)
>  {
>  	GdmXserver     *svr;
>  	int             n;
> -	char           *group;
>  	gboolean        res;
>  	GdmConfigValue *value;
>  
> @@ -1084,8 +1083,6 @@
>  	svr = g_new0 (GdmXserver, 1);
>  	svr->id = g_strdup (name);
>  
> -	group = g_strdup_printf ("server-%s", name);
> -
>  	/* string */
>  	res = gdm_config_get_value (config, group, "name", &value);
>  	if (res) {
> @@ -1141,8 +1138,6 @@
>  	}
>  
>  	xservers = g_slist_append (xservers, svr);
> -
> -	g_free (group);
>  }
>  
>  static void
> @@ -1187,78 +1182,45 @@
>  static void
>  load_xservers_group (GdmConfig *config)
>  {
> -	char     **keys;
> -	gsize      len;
> -	int        i;
> +	GPtrArray  *server_groups;
> +	char      **vname_array;
> +	char       *xserver_value;
> +	int         i, j;
>  
> -	keys = gdm_config_get_keys_for_group (config,
> -		GDM_CONFIG_GROUP_SERVERS, &len, NULL);
> +	server_groups = gdm_config_get_server_groups (config);
>  
> -	/* 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;
> +	for (i=0; i < server_groups->len; i++) {
> +		xserver_value = g_ptr_array_index (server_groups, i);
> +		gdm_debug ("Processing server group <%s>", xserver_value);
>  
> -	        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;
> +		if (g_str_has_prefix (xserver_value, "server-")) {
> +			char * xserver_group;
> +			char * xserver_name;
>  
> -		gdm_config_add_entry (config, &entry);
> -		gdm_config_process_entry (config, &entry, NULL);
> +			xserver_group = g_strdup (xserver_value);
> +			
> +			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 = xserver_group;
> +				gdm_config_process_entry (config, srv_entry, NULL);
> +				gdm_config_entry_free (srv_entry);
> +			}
>  
> -		res = gdm_config_get_value (config, entry.group, entry.key,
> -			&value);
> -		if (! res) {
> -			continue;
> -		}
> +			/* Strip "server-" prefix from name */
> +			xserver_name = xserver_group + strlen ("server-");
>  
> -		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;
> +			/* Now we can add this server */
> +			if (xserver_name != NULL)
> +				gdm_daemon_config_load_xserver (config, xserver_group, xserver_name);
>  		}
> -		
> -		name = value_list[0];
> -
> -		/* Skip servers marked as inactive */
> -		if (name == NULL || name[0] == '\0' ||
> -		    g_ascii_strcasecmp (name, "inactive") == 0) {
> -			gdm_config_value_free (value);
> -			g_strfreev (value_list);
> -			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);
> -		}
> -		g_free (new_group);
> -
> -		/* Now we can add this server */
> -		gdm_daemon_config_load_xserver (config, entry.key, name);
> -                gdm_config_value_free (value);
> -		g_strfreev (value_list);
>          }
>  
> -	g_strfreev (keys);
> +	g_ptr_array_free (server_groups, TRUE);
>  }
>  
>  static void
> @@ -1471,16 +1433,36 @@
>  	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++) {
> +	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;
> +
> +		gdm_config_add_entry (config, &entry);
> +		gdm_config_process_entry (config, &entry, NULL);
> +	}
> +
> +	/* Now construct entries for these groups */
> +	for (i = 0; i < len; i++) {
> +		char           **value_list;
>  		GString         *command     = NULL;
> -		GdmDisplay     *disp;
> -		GdmConfigValue *value;
> +		GdmDisplay      *disp;
> +		GdmConfigValue  *value;
>  		const char      *name        = NULL;
>  		const char      *device_name = NULL;
> -		char           **value_list;
> -		int             keynum;
> -		gboolean        res;
> +		int              keynum;
> +		gboolean         res;
>  
>  		name   = keys[i];
>  
> @@ -1490,15 +1472,20 @@
>  
>  		keynum = atoi (name);
>  
> -                res = gdm_config_get_value (config, GDM_CONFIG_GROUP_SERVERS,
> +		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);
> +		display_value = gdm_config_value_get_string (value);
>  
> +		/* Skip displays marked as inactive */
> +		if (g_ascii_strcasecmp (display_value, "inactive") == 0)
> +			continue;
> +
> +		value_list = g_strsplit (display_value, " ", -1);
> +
>  		if (value_list == NULL || value_list[0] == '\0') {
>  			gdm_config_value_free (value);
>  			g_strfreev (value_list);
> @@ -1524,7 +1511,7 @@
>  				g_string_append (command, " ");
>  			}
>  			j++;
> -                }
> +		}
>  
>  		gdm_debug ("Loading display for key '%d'", keynum);
>  
> @@ -1786,7 +1773,21 @@
>  	return TRUE;
>  }
>  
> +/* Cause debug to affect logging as soon as the config value is read */
>  static gboolean
> +validate_debug (GdmConfig          *config,
> +		GdmConfigSourceType source,
> +		GdmConfigValue     *value)
> +{
> +	gboolean debugval;
> +
> +	debugval = gdm_config_value_get_bool (value);
> +	gdm_log_set_debug (debugval);
> +
> +	return TRUE;
> +}
> +
> +static gboolean
>  validate_at_least_int (GdmConfig          *config,
>  		       GdmConfigSourceType source,
>  		       GdmConfigValue     *value,
> @@ -1814,6 +1815,9 @@
>  	res = TRUE;
>  
>          switch (id) {
> +        case GDM_ID_DEBUG:
> +		res = validate_debug (config, source, value);
> +		break;
>          case GDM_ID_PATH:
>  		res = validate_path (config, source, value);
>  		break;
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]