util-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stanislav Brabec <sbrabec@suse.cz>
To: Lubomir Rintel <lkundrak@v3.sk>, Karel Zak <kzak@redhat.com>
Cc: util-linux@vger.kernel.org
Subject: Re: [PATCH v2] agetty: don't put the VC into canonical mode
Date: Fri, 19 Oct 2018 22:08:17 +0200	[thread overview]
Message-ID: <b7a4fca3-6ee1-f81d-7db1-0c069cd4e56b@suse.cz> (raw)
In-Reply-To: <0d9d07b6b3f45e069fefa4e9e008822a3aa7a135.camel@v3.sk>

I just ported your patch to the current head and did some experiments.

I verified that input indeed works in the non-canonical mode, and
Backspace is handled. I didn't understand why it works and why my
implementation did not work.

So I started to debug it in deep. We all overseen that the non-canonical +
non-echo mode processing is already implemented in agetty.c: get_logname()
in a previously dead part of the code that was never called:

2180					if ((tp->c_lflag & ECHO) == 0)

This code implements both echo and Backspace processing!

There is one small piece for improvement:
When you press letter and then Backspace, reloads remain blocked.
I guess it is acceptable and fixable later.

Many thanks for your patch!

Note that the patch works even with less aggressive
+		tp->c_lflag &= ~(ICANON | ECHO | ECHOE | ECHOK | ECHOKE);
I am not sure whether setting c_lflag to 0 is completely safe.

Subject: [PATCH] agetty: don't put the VC into canonical mode

The wait_for_term_input()'s select() needs to be tripped when the user
starts typing. Otherwise the reloads can abort an already in-progress login.

Coupled with \4 and \6 expansions that happen to be there on Fedora Server,
this means reload on every netlink event. With a couple of IPv6 routers
announcing their networks and temporary addresses in use can make it
sometimes virtually impossible to log in.

Seems like zero lflags do the job just fine on a Linux VT. Reset it to
canonical mode before running login.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
Signed-off-by: Stanislav Brabec <sbrabec@suse.cz>

---
Changes since v1:
- Unreversed the logic in termio_final()/reset_vc() conditional

Tested on a vc and serial console on a stock Fedora installation:

  /sbin/agetty -o -p -- \u --noclear tty1 linux
  /sbin/agetty -o -p -- \u --keep-baud 115200,38400,9600 ttyS2 vt220
---
 term-utils/agetty.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/term-utils/agetty.c b/term-utils/agetty.c
index 59122ca45..cfcdffe93 100644
--- a/term-utils/agetty.c
+++ b/term-utils/agetty.c
@@ -316,7 +316,7 @@ static void parse_speeds(struct options *op, char *arg);
 static void update_utmp(struct options *op);
 static void open_tty(char *tty, struct termios *tp, struct options *op);
 static void termio_init(struct options *op, struct termios *tp);
-static void reset_vc (const struct options *op, struct termios *tp);
+static void reset_vc(const struct options *op, struct termios *tp, int canon);
 static void auto_baud(struct termios *tp);
 static void list_speeds(void);
 static void output_special_char (struct issue *ie, unsigned char c, struct options *op,
@@ -501,13 +501,14 @@ int main(int argc, char **argv)
 	if (options.timeout)
 		alarm(0);
 
-	if ((options.flags & F_VCONSOLE) == 0) {
-		/* Finalize the termios settings. */
+	/* Finalize the termios settings. */
+	if ((options.flags & F_VCONSOLE) == 0)
 		termio_final(&options, &termios, &chardata);
+	else
+		reset_vc(&options, &termios, 1);
 
-		/* Now the newline character should be properly written. */
-		write_all(STDOUT_FILENO, "\r\n", 2);
-	}
+	/* Now the newline character should be properly written. */
+	write_all(STDOUT_FILENO, "\r\n", 2);
 
 	sigaction(SIGQUIT, &sa_quit, NULL);
 	sigaction(SIGINT, &sa_int, NULL);
@@ -1250,7 +1251,7 @@ static void termio_init(struct options *op, struct termios *tp)
 		setlocale(LC_CTYPE, "POSIX");
 		op->flags &= ~F_UTF8;
 #endif
-		reset_vc(op, tp);
+		reset_vc(op, tp, 0);
 
 		if ((tp->c_cflag & (CS8|PARODD|PARENB)) == CS8)
 			op->flags |= F_EIGHTBITS;
@@ -1360,7 +1361,7 @@ static void termio_init(struct options *op, struct termios *tp)
 }
 
 /* Reset virtual console on stdin to its defaults */
-static void reset_vc(const struct options *op, struct termios *tp)
+static void reset_vc(const struct options *op, struct termios *tp, int canon)
 {
 	int fl = 0;
 
@@ -1369,6 +1370,15 @@ static void reset_vc(const struct options *op, struct termios *tp)
 
 	reset_virtual_console(tp, fl);
 
+#ifdef AGETTY_RELOAD
+	/*
+	 * Discard all the flags that makes the line go canonical with echoing.
+	 * We need to know when the user starts typing.
+	 */
+	if (canon == 0)
+		tp->c_lflag = 0;
+#endif
+
 	if (tcsetattr(STDIN_FILENO, TCSADRAIN, tp))
 		log_warn(_("setting terminal attributes failed: %m"));
 
-- 
2.19.1

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: sbrabec@suse.com
Křižíkova 148/34 (Corso IIa)                  tel: +49 911 7405384547
186 00 Praha 8-Karlín                          fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

  reply	other threads:[~2018-10-19 20:08 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-16  6:41 [PATCH v2] agetty: don't put the VC into canonical mode Lubomir Rintel
2018-10-16  7:40 ` Karel Zak
2018-10-18 19:25   ` Stanislav Brabec
2018-10-19  7:53     ` Karel Zak
2018-10-19  6:09   ` Lubomir Rintel
2018-10-19 13:08     ` Stanislav Brabec
2018-10-19 13:48       ` Lubomir Rintel
2018-10-19 20:08         ` Stanislav Brabec [this message]
2018-10-22  9:23           ` Karel Zak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b7a4fca3-6ee1-f81d-7db1-0c069cd4e56b@suse.cz \
    --to=sbrabec@suse.cz \
    --cc=kzak@redhat.com \
    --cc=lkundrak@v3.sk \
    --cc=util-linux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).