[PATCH xinit] Remove workaround for xterm -L (#89653)

[Mark Kettenis]

> Date: Tue, 24 Mar 2015 11:33:28 +0100
> From: Hans de Goede <hdegoede at redhat.com>
> 
> Hi,
> 
> On 24-03-15 05:49, Peter Hutterer wrote:
> > The -L flag was removed in 1989.
> >
> > This enables the legacy keyboard driver again when the server is started
> > with -keeptty (bd6cacdd3661)
> >
> > X.Org Bug 89653 <http://bugs.freedesktop.org/show_bug.cgi?id=89653>
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> > No idea what exactly is going on here, I tried to bisect the server to get
> > some hints what triggers what there are too many moving targets. Either
> > way, with this patch applied, a basic xorg.conf that uses the kbd driver
> > works again with startx/xinit (provided a suid Xorg binary of course)
> >
> > Thanks to alanc for digging up the commit that removed the -L option.
> > http://cgit.freedesktop.org/~alanc/xc-historical/commit/xc/programs/xterm?id=46fc268c21d01cf0d664c84e5d03f785b2b2e5ce
> 
> I'm afraid that this causes a regression, with this patch the Xserver no
> longer cleanly exits when the last client disconnects.
> 
> Test-case:
> 
> -Fully up2date 64 bit Fedora-22 system
> -xinit rpm package build with this patch
> -do: "startx /usr/bin/xterm -title foo"
> -exit the shell in the xterm
> -now Xorg hangs using aprox 100% cpu, strace shows that it
>   sits in a select() loop

I think that one is easy to understand.  When the client exits, xinit
will try to kill the X server.  It does this by calling killpg().  With
Peter's diff, xinit no longer creates a process group for the server.
So the X server will stay around.  Not sure why the server spins, but
that's probably an unrelated X server bug of some sorts.

Without -keeptty the X server itself will (on some platforms) put
itself in its own process group.  In that case the server will be
killed.  Relying on this isn't a good idea though.  With privsep on
OpenBSD for example the X server forks itself before dropping
privileges and if it is the child that calls setpgid(), xinit will use
the wrong pgid and killing the server will still fail.

So I think the setpgid() call should stay.  Perhaps it was added to
work around an xterm -L, but more changes have been made that depend
on it.  The comment should probably be removed as it is clearly
misleading.

I'm not really familliar with the Linux console driver and how it
interacts with the keyboard.  Perhaps the xf86-input-keyboard driver
depends on being part of the foreground process group, and the problem
can be fixed by making the Linux-specific X server init code call
tcsetpgrp()?

> >   xinit.c | 6 +-----
> >   1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/xinit.c b/xinit.c
> > index 1b04911..74fda74 100644
> > --- a/xinit.c
> > +++ b/xinit.c
> > @@ -417,11 +417,7 @@ startServer(char *server_argv[])
> >            * at xinit when ready to accept connections
> >            */
> >           signal(SIGUSR1, SIG_IGN);
> > -        /*
> > -         * prevent server from getting sighup from vhangup()
> > -         * if client is xterm -L
> > -         */
> > -        setpgid(0,getpid());
> > +
> >           Execute(server_argv);
> >
> >           Error("unable to run server \"%s\"", server_argv[0]);
> >