Util-Linux Archive on lore.kernel.org
 help / Atom feed
* [PATCH] agetty: don't put the VC into canonical mode
@ 2018-09-24 10:15 Lubomir Rintel
  2018-09-25 11:22 ` Karel Zak
  0 siblings, 1 reply; 3+ messages in thread
From: Lubomir Rintel @ 2018-09-24 10:15 UTC (permalink / raw)
  To: util-linux; +Cc: Lubomir Rintel

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 log=
in.

Coupled with \4 and \6 expansions that happen to be there on Fedora Serve=
r,
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 though.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 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 e22600e28..b82faab8d 100644
--- a/term-utils/agetty.c
+++ b/term-utils/agetty.c
@@ -303,7 +303,7 @@ static void parse_speeds(struct options *op, char *ar=
g);
 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 (unsigned char c, struct options *op,
@@ -482,13 +482,14 @@ int main(int argc, char **argv)
 	if (options.timeout)
 		alarm(0);
=20
-	if ((options.flags & F_VCONSOLE) =3D=3D 0) {
-		/* Finalize the termios settings. */
+	/* Finalize the termios settings. */
+	if ((options.flags & F_VCONSOLE) =3D=3D 0)
+		reset_vc(&options, &termios, 1);
+	else
 		termio_final(&options, &termios, &chardata);
=20
-		/* 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);
=20
 	sigaction(SIGQUIT, &sa_quit, NULL);
 	sigaction(SIGINT, &sa_int, NULL);
@@ -1231,7 +1232,7 @@ static void termio_init(struct options *op, struct =
termios *tp)
 		setlocale(LC_CTYPE, "POSIX");
 		op->flags &=3D ~F_UTF8;
 #endif
-		reset_vc(op, tp);
+		reset_vc(op, tp, 0);
=20
 		if ((tp->c_cflag & (CS8|PARODD|PARENB)) =3D=3D CS8)
 			op->flags |=3D F_EIGHTBITS;
@@ -1341,7 +1342,7 @@ static void termio_init(struct options *op, struct =
termios *tp)
 }
=20
 /* 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 c=
anon)
 {
 	int fl =3D 0;
=20
@@ -1350,6 +1351,15 @@ static void reset_vc(const struct options *op, str=
uct termios *tp)
=20
 	reset_virtual_console(tp, fl);
=20
+#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 =3D=3D 0)
+		tp->c_lflag =3D 0;
+#endif
+
 	if (tcsetattr(STDIN_FILENO, TCSADRAIN, tp))
 		log_warn(_("setting terminal attributes failed: %m"));
=20
--=20
2.19.0

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] agetty: don't put the VC into canonical mode
  2018-09-24 10:15 [PATCH] agetty: don't put the VC into canonical mode Lubomir Rintel
@ 2018-09-25 11:22 ` Karel Zak
  2018-10-16  6:29   ` Lubomir Rintel
  0 siblings, 1 reply; 3+ messages in thread
From: Karel Zak @ 2018-09-25 11:22 UTC (permalink / raw)
  To: Lubomir Rintel; +Cc: util-linux

On Mon, Sep 24, 2018 at 12:15:35PM +0200, Lubomir Rintel wrote:
> -	if ((options.flags & F_VCONSOLE) == 0) {
> -		/* Finalize the termios settings. */
> +	/* Finalize the termios settings. */
> +	if ((options.flags & F_VCONSOLE) == 0)
> +		reset_vc(&options, &termios, 1);
> +	else
>  		termio_final(&options, &termios, &chardata);

This completely changes logic of the code and removes termio_final()
from serial-line code path. Is it expected change? I don't think so.

> -		/* 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);

Why we need to always write \r\n ?

Maybe all you need is

    if ((options.flags & F_VCONSOLE) == 0) {
        termio_final(&options, &termios, &chardata);
        write_all(STDOUT_FILENO, "\r\n", 2);
    } else
        reset_vc(&options, &termios, 1);


  Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] agetty: don't put the VC into canonical mode
  2018-09-25 11:22 ` Karel Zak
@ 2018-10-16  6:29   ` Lubomir Rintel
  0 siblings, 0 replies; 3+ messages in thread
From: Lubomir Rintel @ 2018-10-16  6:29 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On Tue, 2018-09-25 at 13:22 +0200, Karel Zak wrote:
> On Mon, Sep 24, 2018 at 12:15:35PM +0200, Lubomir Rintel wrote:
> > -	if ((options.flags & F_VCONSOLE) == 0) {
> > -		/* Finalize the termios settings. */
> > +	/* Finalize the termios settings. */
> > +	if ((options.flags & F_VCONSOLE) == 0)
> > +		reset_vc(&options, &termios, 1);
> > +	else
> >  		termio_final(&options, &termios, &chardata);
> 
> This completely changes logic of the code and removes termio_final()
> from serial-line code path. Is it expected change? I don't think so.

Of course, you're right -- the conditional is reversed. I'll follow up
with a v2.

> 
> > -		/* 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);
> 
> Why we need to always write \r\n ?

I honestly have no idea. By switching to the vconsole to non-canonical
mode as well, I just stopped getting a newline after typing a login:

  xo login: lkundrakPassword:

I just thought it was sort of expected to get the same behavior as on
serial lines and removed the special case.

> Maybe all you need is
> 
>     if ((options.flags & F_VCONSOLE) == 0) {
>         termio_final(&options, &termios, &chardata);
>         write_all(STDOUT_FILENO, "\r\n", 2);
>     } else
>         reset_vc(&options, &termios, 1);

Yes (but with the write_all(\r\n) common for both serial and vc).

> 
> 
>   Karel

Thanks,
Lubo


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-24 10:15 [PATCH] agetty: don't put the VC into canonical mode Lubomir Rintel
2018-09-25 11:22 ` Karel Zak
2018-10-16  6:29   ` Lubomir Rintel

Util-Linux Archive on lore.kernel.org

Archives are clonable: git clone --mirror https://lore.kernel.org/util-linux/0 util-linux/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 util-linux util-linux/ https://lore.kernel.org/util-linux \
		util-linux@vger.kernel.org util-linux@archiver.kernel.org
	public-inbox-index util-linux


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.util-linux


AGPL code for this site: git clone https://public-inbox.org/ public-inbox