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

[Martin Weinberg <martin weinberg comcast net>]

That patch fixed the problems!  Everything appears to be working fine
now.

In particular, as you probably guessed, I use the gdmflexiserver to
start a new X server for dual head or Xinerama as necessary.  That
feature is really great for my needs.

Thanks,

--Martin

On Fri, Jan 04, 2008 at 03:22:39PM -0600, Brian Cameron wrote: 
> 
> Martin:
> 
> Looking at the code a bit more closely, I noticed the code was
> allocating xserver_group once outside the loop, assigning it
> to srv_entry->group, then it gets freed in the gdm_config_entry_free().
> Then next time around the loop, it is already freed!
> 
> I think the attached patch (which applies on top of the previous patch)
> should fix the problem.  Now the code makes sure to call stdup each
> time it sets srv_entry->group.  Can you verify this avoids the crash?
> If so, then I'll commit this upstream.
> 
> Thanks,
> 
> Brian
> 
> 
> >>>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:
> >>I don't see any crashing problems here, but sometimes crashing problems
> >>can be OS-speicifc.
> >>
> >>You don't specify which line in the loop is line 1211.  In my copy of
> >>the file it is the gdm_config_entry_free line, but I suspect it is a
> >>different line in your file (since that would be an odd line for it
> >>to crash on just by looking at the code).
> >
> >Yes it really is in gdm_config_entry_free().  I'm attaching three
> >files:
> >	script.out	a "script" capture of the gdb session
> >	/etc/gdm.conf
> >	/usr/share/gdm/defaults.conf
> >
> >The latter two from the results of:
> >
> >gdmflexiserver --command=GET_CONFIG_FILE
> >gdmflexiserver --command=GET_CUSTOM_CONFIG_FILE
> >
> >from the unpatched version.
> >
> >>Also, could you try running gdm in the debugger and see if it gives any
> >>further info about what is causing the crash?  Since the crash is
> >>happening early it is probably a situation where you can actually run
> >>GDM in a debugger and get some useful information.  This isn't always
> >>the case, since GDM forks into slave daemons - so it is harder to use
> >>the debugger to find issues after such forks happen.
> >>
> >>So, what I'd recommend doing is:
> >>
> >>1) Run gdm-binary (not the gdm script) in the debugger like this:
> >>   gdb /usr/sbin/gdm-binary
> >>   run --nodaemon
> >>
> >>   The --nodaemon argument tells gdm-binary not to fork immediately
> >>   which is what you want when debugging.
> >>
> >>2) Then it should crash and tell you that the SEGV happened and drop
> >>   you into a prompt.  Try printing out various values like run
> >>   "print srv_entry" and see if the group variable is NULL or something
> >>   causing a crash when we try to access srv_entry->group.
> >>
> >>>			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.
> >>It would be helpful if you could run these commands:
> >>
> >>gdmflexiserver --command=GET_CONFIG_FILE
> >>gdmflexiserver --command=GET_CUSTOM_CONFIG_FILE
> >>
> >>And send me these two files as attachments.  Perhaps the crash is
> >>triggered by how your [servers] and [server-foo] sections are set up.
> >>If you can send me your files, I can see if I can trigger the crash
> >>on my system using your style of configuration.
> >>
> >>Brian
> >>
> >>------------------------------------------------------------------------
> >>
> >>_______________________________________________
> >>gdm-list mailing list
> >>gdm-list gnome org
> >>http://mail.gnome.org/mailman/listinfo/gdm-list
> 

> Index: gdm-daemon-config.c
> ===================================================================
> --- gdm-daemon-config.c	(revision 5599)
> +++ gdm-daemon-config.c	(working copy)
> @@ -1184,21 +1184,18 @@
>  {
>  	GPtrArray  *server_groups;
>  	char      **vname_array;
> -	char       *xserver_value;
> +	char       *xserver_group;
>  	int         i, j;
>  
>  	server_groups = gdm_config_get_server_groups (config);
>  
>  	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);
> +		xserver_group = g_ptr_array_index (server_groups, i);
> +		gdm_debug ("Processing server group <%s>", xserver_group);
>  
> -		if (g_str_has_prefix (xserver_value, "server-")) {
> -			char * xserver_group;
> +		if (g_str_has_prefix (xserver_group, "server-")) {
>  			char * xserver_name;
>  
> -			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) {
> @@ -1206,7 +1203,7 @@
>  				}
>  				srv_entry = gdm_config_entry_copy (&gdm_daemon_server_config_entries[j]);
>  				g_free (srv_entry->group);
> -				srv_entry->group = xserver_group;
> +				srv_entry->group = g_strdup (xserver_group);
>  				gdm_config_process_entry (config, srv_entry, NULL);
>  				gdm_config_entry_free (srv_entry);
>  			}