On Fri, Apr 29, 2011 at 02:38:13PM +0200, Dr. Werner Fink wrote:
> On Fri, Apr 29, 2011 at 12:55:30PM +0200, Karel Zak wrote:
> > On Thu, Apr 28, 2011 at 04:56:31PM +0200, Werner Fink wrote:
> > > Ensure a proper session on the terminal line, that is do a
> > > vhangup() and become the controlling terminal. After this
> >
> > Is it really necessary? login(1) calls vhangup(), wouldn't be enough
> > to make the vhangup() call optional?
>
> Hmmm ... yes this is true as long as the standard login is used.
The default is to use the standard login(1).
> >
> > > + sa.sa_handler = SIG_IGN;
> > > + sa.sa_flags = SA_RESTART;
> > > + sigemptyset (&sa.sa_mask);
> > > + sigaction(SIGHUP, &sa, &sa_hup);
> > > + sigaction(SIGQUIT, &sa, &sa_quit);
> > > + sigaction(SIGINT, &sa, &sa_int);
> >
> > Everyone who will read this your patch will not understand why sa_int
> > and sa_quit are necessary, it's used in another (next) patch...
>
> Not catching SIGHUP and using vhangup() does a hangup
> to the parent of the calling process.
I'm talking about sa_int and sa_quit variables :-)
sigaction(SIGQUIT, &sa, NULL);
sigaction(SIGINT, &sa, NULL);
will do the same job. The original SIGQUIT and SIGINT setting is not
restored in _this_ patch. Maybe it would be better to move sigactions
from the "[PATCH 6/8] Add an autologin feature" patch to this patch to
keep the patches more consistent.
I know that it seems like a pedantic nitpicking, but don't forget that
one day someone will do:
$ git blame -L <lines> term-utils/agetty.c
$ git show <sha1>
and then he will be pretty confused, because the patch does not make
sense.
> > > + if ((op->flags & F_NOHANGUP) == 0) {
> > > + /*
> > > + * vhangup() will replace all open file descriptors in the kernel
> > > + * that point to our controlling tty by a dummy that will deny
> > > + * further reading/writing to our device. It will also reset the
> > > + * tty to sane defaults, so we don't have to modify the tty device
> >
> > Hmm.. "reset the tty to sane defaults" .. is it always expected
> > behavior? I can imagine environment where the tty setting is already
> > modified before getty is called.
> >
> > For example login(1) saves and restores tty attributes when vhangup()
> > is called.
> >
> > For serial lines is this gray zone :-) I'd like to be very
> > conservative... with vhangup() we are changing the current default
> > behavior. It would be really better to have --hangup (or
> > --control-tty) rather than --nohangup to be backwardly compatible.
>
> Should we make a difference between a serial and virtual console
> line?
I don't think so. I think that the hangup feature could be attractive
for the both groups (serial and virtual). The feature should be
unnecessary for normal system (with normal login(1)), so disabled by
default.
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
