[PATCH] agetty: map NL to CR-NL on output in initial termios settings

[PATCH] agetty: map NL to CR-NL on output in initial termios settings

[Dennis Jensen]

This fixes situations where messages "scroll horizontally", making
them unreadable, because agetty has opened and initialized the
terminal.

From: Jacoby Hickerson <hickersonjl@gmail.com>
Signed-off-by: Dennis Jensen <dennis.h.jensen@siemens.com>
---
 term-utils/agetty.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/term-utils/agetty.c b/term-utils/agetty.c
index 041e1f7..27edac5 100644
--- a/term-utils/agetty.c
+++ b/term-utils/agetty.c
@@ -1091,7 +1091,8 @@ static void termio_init(struct options *op, struct termios *tp)
 #else
 	tp->c_iflag = 0;
 #endif
-	tp->c_lflag = tp->c_oflag = 0;
+	tp->c_lflag = 0;
+	tp->c_oflag = OPOST | ONLCR;
 
 	if ((op->flags & F_KEEPCFLAGS) == 0)
 		tp->c_cflag = CS8 | HUPCL | CREAD | (tp->c_cflag & CLOCAL);
-- 
1.7.0.4

Re: [PATCH] agetty: map NL to CR-NL on output in initial termios settings

[Karel Zak]

[CC: to Werner Fink]

On Wed, Nov 16, 2011 at 10:54:50AM +0100, Dennis Jensen wrote:
> This fixes situations where messages "scroll horizontally", making

Do you mean /etc/issue and prompt strings? The terminal end-of-line
should be detected in get_logname() and then the proper flags should
be set in termio_final().

> them unreadable, because agetty has opened and initialized the
> terminal.
> 
> From: Jacoby Hickerson <hickersonjl@gmail.com>
> Signed-off-by: Dennis Jensen <dennis.h.jensen@siemens.com>
> ---
>  term-utils/agetty.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/term-utils/agetty.c b/term-utils/agetty.c
> index 041e1f7..27edac5 100644
> --- a/term-utils/agetty.c
> +++ b/term-utils/agetty.c
> @@ -1091,7 +1091,8 @@ static void termio_init(struct options *op, struct termios *tp)
>  #else
>  	tp->c_iflag = 0;
>  #endif
> -	tp->c_lflag = tp->c_oflag = 0;
> +	tp->c_lflag = 0;
> +	tp->c_oflag = OPOST | ONLCR;

Is it correct to always set ONLCR? Would be better to follow the
current (kernel) setting? I mean:

  tp->c_oflag = (tp->c_oflag & POST) | (tp->c_oflag & ONLCR);

Can you try it?

We use ONLCR for virtual consoles, but for serial lines it depends on
user's input (usersname string).

    Karel

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

Re: [PATCH] agetty: map NL to CR-NL on output in initial termiossettings

[Dennis Jensen]

On Wed, 2011-11-23 at 12:08 +0100, Karel Zak wrote:
> [CC: to Werner Fink]
> 
> On Wed, Nov 16, 2011 at 10:54:50AM +0100, Dennis Jensen wrote:
> > This fixes situations where messages "scroll horizontally", making
> 
> Do you mean /etc/issue and prompt strings? The terminal end-of-line
> should be detected in get_logname() and then the proper flags should
> be set in termio_final().

I mean something like this, after enabling DEBUG: 

-wL8ttyS019200linuxaterm_io 2
                             before autobaud
                                            waiting for cr-lf

So if agetty is on the console and messages are written to the console
while it is waiting for cr-lf, before get_logname(), they will scroll
like above.
> 
> > them unreadable, because agetty has opened and initialized the
> > terminal.
> > 
> > From: Jacoby Hickerson <hickersonjl@gmail.com>
> > Signed-off-by: Dennis Jensen <dennis.h.jensen@siemens.com>
> > ---
> >  term-utils/agetty.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/term-utils/agetty.c b/term-utils/agetty.c
> > index 041e1f7..27edac5 100644
> > --- a/term-utils/agetty.c
> > +++ b/term-utils/agetty.c
> > @@ -1091,7 +1091,8 @@ static void termio_init(struct options *op, struct termios *tp)
> >  #else
> >  	tp->c_iflag = 0;
> >  #endif
> > -	tp->c_lflag = tp->c_oflag = 0;
> > +	tp->c_lflag = 0;
> > +	tp->c_oflag = OPOST | ONLCR;
> 
> Is it correct to always set ONLCR? Would be better to follow the
> current (kernel) setting? I mean:
> 
>   tp->c_oflag = (tp->c_oflag & POST) | (tp->c_oflag & ONLCR);
> 
> Can you try it?

Sure, that also works for me. This seems as safe to me as setting the
output flags to 0. Here's the new patch.

Regards,
Dennis

From: Jacoby Hickerson <hickersonjl@gmail.com>
Signed-off-by: Dennis Jensen <dennis.h.jensen@siemens.com>
---
 term-utils/agetty.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/term-utils/agetty.c b/term-utils/agetty.c
index 041e1f7..079a737 100644
--- a/term-utils/agetty.c
+++ b/term-utils/agetty.c
@@ -1091,7 +1091,8 @@ static void termio_init(struct options *op, struct termios *tp)
 #else
 	tp->c_iflag = 0;
 #endif
-	tp->c_lflag = tp->c_oflag = 0;
+	tp->c_lflag = 0;
+	tp->c_oflag &= OPOST | ONLCR;
 
 	if ((op->flags & F_KEEPCFLAGS) == 0)
 		tp->c_cflag = CS8 | HUPCL | CREAD | (tp->c_cflag & CLOCAL);
-- 
1.7.0.4

Re: [PATCH] agetty: map NL to CR-NL on output in initial termiossettings

[Karel Zak]

On Thu, Nov 24, 2011 at 06:55:57PM +0100, Dennis Jensen wrote:
>  term-utils/agetty.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)

 Applied, thanks.

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

Re: [PATCH] agetty: map NL to CR-NL on output in initial termiossettings

[Dr. Werner Fink]

On Thu, Nov 24, 2011 at 06:55:57PM +0100, Dennis Jensen wrote:
> On Wed, 2011-11-23 at 12:08 +0100, Karel Zak wrote:
> > [CC: to Werner Fink]
> > 
> > On Wed, Nov 16, 2011 at 10:54:50AM +0100, Dennis Jensen wrote:
> > > This fixes situations where messages "scroll horizontally", making
> > 
> > Do you mean /etc/issue and prompt strings? The terminal end-of-line
> > should be detected in get_logname() and then the proper flags should
> > be set in termio_final().
> 
> I mean something like this, after enabling DEBUG: 
> 
> -wL8ttyS019200linuxaterm_io 2
>                              before autobaud
>                                             waiting for cr-lf

[...]

> 
> diff --git a/term-utils/agetty.c b/term-utils/agetty.c
> index 041e1f7..079a737 100644
> --- a/term-utils/agetty.c
> +++ b/term-utils/agetty.c
> @@ -1091,7 +1091,8 @@ static void termio_init(struct options *op, struct termios *tp)
>  #else
>  	tp->c_iflag = 0;
>  #endif
> -	tp->c_lflag = tp->c_oflag = 0;
> +	tp->c_lflag = 0;
> +	tp->c_oflag &= OPOST | ONLCR;
>  
>  	if ((op->flags & F_KEEPCFLAGS) == 0)
>  		tp->c_cflag = CS8 | HUPCL | CREAD | (tp->c_cflag & CLOCAL);


Indeed this should work even on some strange serial consoles.
Maybe this is also required for my patch set for sulogin as
with this set sulogin supports not only virtual consoles but
also serial consoles as well.


     Werner

-- 
  "Having a smoking section in a restaurant is like having
          a peeing section in a swimming pool." -- Edward Burr

Re: [PATCH] agetty: map NL to CR-NL on output in initial termiossettings

[Karel Zak]

On Mon, Dec 10, 2012 at 01:34:39PM +0100, Dr. Werner Fink wrote:
> > --- a/term-utils/agetty.c
> > +++ b/term-utils/agetty.c
> > @@ -1091,7 +1091,8 @@ static void termio_init(struct options *op, struct termios *tp)
> >  #else
> >  	tp->c_iflag = 0;
> >  #endif
> > -	tp->c_lflag = tp->c_oflag = 0;
> > +	tp->c_lflag = 0;
> > +	tp->c_oflag &= OPOST | ONLCR;
> >  
> >  	if ((op->flags & F_KEEPCFLAGS) == 0)
> >  		tp->c_cflag = CS8 | HUPCL | CREAD | (tp->c_cflag & CLOCAL);
> 
> 
> Indeed this should work even on some strange serial consoles.
> Maybe this is also required for my patch set for sulogin as

 OK, applied to sulogin too.

> with this set sulogin supports not only virtual consoles but
> also serial consoles as well.

 It seems that we duplicate some code in sulogin and agetty. 

 See: 
    term-utils/agetty.c: termio_final()
    login-utils/sulogin.c: tcfinal()

    (and maybe also tcinit() in sulogin.c)

 would be possible to merge this code and move it to include/ttyutils.h 
 (or lib/ttyutils.c). I see small differences in the code, but not sure 
 which version is better (I guess agetty as this code is tested on more
 places).

 It would be nice to cleanup this before the next release to have only
 one place (code) where we initialize terminal attributes.
 
    Karel

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

Re: [util-linux] [PATCH] agetty: map NL to CR-NL on output in initial termiossettings

[Dr. Werner Fink]

On Wed, Dec 19, 2012 at 10:33:46AM +0100, Karel Zak wrote:
> On Mon, Dec 10, 2012 at 01:34:39PM +0100, Dr. Werner Fink wrote:
> > > --- a/term-utils/agetty.c
> > > +++ b/term-utils/agetty.c
> > > @@ -1091,7 +1091,8 @@ static void termio_init(struct options *op, struct termios *tp)
> > >  #else
> > >  	tp->c_iflag = 0;
> > >  #endif
> > > -	tp->c_lflag = tp->c_oflag = 0;
> > > +	tp->c_lflag = 0;
> > > +	tp->c_oflag &= OPOST | ONLCR;
> > >  
> > >  	if ((op->flags & F_KEEPCFLAGS) == 0)
> > >  		tp->c_cflag = CS8 | HUPCL | CREAD | (tp->c_cflag & CLOCAL);
> > 
> > 
> > Indeed this should work even on some strange serial consoles.
> > Maybe this is also required for my patch set for sulogin as
> 
>  OK, applied to sulogin too.
> 
> > with this set sulogin supports not only virtual consoles but
> > also serial consoles as well.
> 
>  It seems that we duplicate some code in sulogin and agetty. 
> 
>  See: 
>     term-utils/agetty.c: termio_final()
>     login-utils/sulogin.c: tcfinal()
> 
>     (and maybe also tcinit() in sulogin.c)
> 
>  would be possible to merge this code and move it to include/ttyutils.h 
>  (or lib/ttyutils.c). I see small differences in the code, but not sure 
>  which version is better (I guess agetty as this code is tested on more
>  places).
> 
>  It would be nice to cleanup this before the next release to have only
>  one place (code) where we initialize terminal attributes.

This may require to move `struct chardata' from term-utils/agetty.c and
also from include/consoles.h as well as init_chardata from term-utils/agetty.c to
a new include/ttyutils.h ...  then the function caps_lock() from term-utils/agetty.c
could be moved to lib/ttyutils.c ... after this termio_final() could become
the core of tcfinal() after the two boolean.  Similar holds true for tcinit()
and termio_init().


     Werner

-- 
  "Having a smoking section in a restaurant is like having
          a peeing section in a swimming pool." -- Edward Burr

[PATCH] Move struct chardata and init_chardata to ttyutils

[Werner Fink]

Hi,

this one moves the struct chardata and init_chardata to include/ttyutils.h
as well as to lib/include/ttyutils.c.  Also the macros CTL/CTRL are fixed
in agetty.c and sulogin.c to use the XOR variant CTL.

Signed-off-by: Werner Fink <werner@suse.de>
---
 include/consoles.h    |    9 ++-------
 include/ttyutils.h    |   50 +++++++++++++++++++++++++++++++++++++++++++++++--
 lib/consoles.c        |    8 +-------
 lib/ttyutils.c        |   28 +++++++++++++++++++++++++++
 login-utils/sulogin.c |    4 ----
 term-utils/agetty.c   |   34 ---------------------------------
 6 files changed, 79 insertions(+), 54 deletions(-)

diff --git include/consoles.h include/consoles.h
index 2544263..5d87156 100644
--- include/consoles.h
+++ include/consoles.h
@@ -26,14 +26,9 @@
 #include <stdint.h>
 #include <stdio.h>
 #include <termios.h>
-#include <list.h>
+#include "list.h"
+#include "ttyutils.h"
 
-struct chardata {
-	uint8_t	erase;
-	uint8_t kill;
-	uint8_t eol;
-	uint8_t parity;
-};
 struct console {
 	struct list_head entry;
 	char *tty;
diff --git include/ttyutils.h include/ttyutils.h
index 93e8934..75d48c7 100644
--- include/ttyutils.h
+++ include/ttyutils.h
@@ -1,3 +1,23 @@
+/*
+ * ttyutils.h       Header file for routines to detect and initialize
+ *                  terminal lines
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *  
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program (see the file COPYING); if not, write to the
+ * Free Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ *
+ */
 #ifndef UTIL_LINUX_TTYUTILS_H
 #define UTIL_LINUX_TTYUTILS_H
 
@@ -8,6 +28,34 @@
 #include <sys/ioctl.h>
 #endif
 
+/* Some shorthands for control characters. */
+#define CTL(x)		((x) ^ 0100)	/* Assumes ASCII dialect */
+#define CR		CTL('M')	/* carriage return */
+#define NL		CTL('J')	/* line feed */
+#define BS		CTL('H')	/* back space */
+#define DEL		CTL('?')	/* delete */
+
+/* Defaults for line-editing etc. characters; you may want to change these. */
+#define DEF_ERASE	DEL		/* default erase character */
+#define DEF_INTR	CTL('C')	/* default interrupt character */
+#define DEF_QUIT	CTL('\\')	/* default quit char */
+#define DEF_KILL	CTL('U')	^/* default kill char */
+#define DEF_EOF		CTL('D')	/* default EOF char */
+#define DEF_EOL		0
+#define DEF_SWITCH	0		/* default switch char */
+
+/* Storage for things detected while the login name was read. */
+struct chardata {
+	int erase;		/* erase character */
+	int kill;		/* kill character */
+	int eol;		/* end-of-line character */
+	int parity;		/* what parity did we see */
+	int capslock;		/* upper case without lower case */
+};
+
+/* Initial values for the above. */
+extern const struct chardata init_chardata;
+
 extern int get_terminal_width(void);
 extern int get_terminal_name(const char **path, const char **name, const char **number);
 
@@ -82,6 +130,4 @@ static inline void reset_virtual_console(struct termios *tp, int flags)
 	tp->c_cc[VEOL2]    = _POSIX_VDISABLE;
 }
 
-
-
 #endif /* UTIL_LINUX_TTYUTILS_H */
diff --git lib/consoles.c lib/consoles.c
index 7175a08..75fb082 100644
--- lib/consoles.c
+++ lib/consoles.c
@@ -277,12 +277,6 @@ __attribute__((__nonnull__,__hot__))
 #endif
 int append_console(struct list_head *consoles, const char *name)
 {
-	static const struct chardata initcp = {
-		.erase	= CERASE,
-		.kill	= CKILL,
-		.eol	= CTRL('r'),
-		.parity = 0
-	};
 	struct console *restrict tail;
 	struct console *last = NULL;
 
@@ -307,7 +301,7 @@ int append_console(struct list_head *consoles, const char *name)
 	tail->id = last ? last->id + 1 : 0;
 	tail->pid = 0;
 	memset(&tail->tio, 0, sizeof(tail->tio));
-	memcpy(&tail->cp, &initcp, sizeof(struct chardata));
+	tail->cp = init_chardata;
 
 	return 0;
 }
diff --git lib/ttyutils.c lib/ttyutils.c
index 15d5389..b9236fa 100644
--- lib/ttyutils.c
+++ lib/ttyutils.c
@@ -1,9 +1,37 @@
+/*
+ * ttyutils.c       Common routines to detect/initialize terminal lines
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program (see the file COPYING); if not, write to the
+ * Free Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
+ * MA 02110-1301, USA.
+ *
+ */
 
 #include <ctype.h>
 
 #include "c.h"
 #include "ttyutils.h"
 
+/* Initial values for the above. */
+static const struct chardata init_chardata = {
+	DEF_ERASE,		/* default erase character */
+	DEF_KILL,		/* default kill character */
+	13,			/* default eol char */
+	0,			/* space parity */
+	0,			/* no capslock */
+};
+
 int get_terminal_width(void)
 {
 #ifdef TIOCGSIZE
diff --git login-utils/sulogin.c login-utils/sulogin.c
index 810168d..14a865c 100644
--- login-utils/sulogin.c
+++ login-utils/sulogin.c
@@ -58,10 +58,6 @@
 #include "consoles.h"
 #define CONMAX		16
 
-#define BS		CTRL('h')
-#define NL		CTRL('j')
-#define CR		CTRL('m')
-
 static unsigned int timeout;
 static int profile;
 static volatile uint32_t openfd;		/* Remember higher file descriptors */
diff --git term-utils/agetty.c term-utils/agetty.c
index 5df150a..cc1a9ab 100644
--- term-utils/agetty.c
+++ term-utils/agetty.c
@@ -106,22 +106,6 @@
 #define LOGIN		"login: "
 #define LOGIN_ARGV_MAX	16		/* Numbers of args for login */
 
-/* Some shorthands for control characters. */
-#define CTL(x)		(x ^ 0100)	/* Assumes ASCII dialect */
-#define	CR		CTL('M')	/* carriage return */
-#define	NL		CTL('J')	/* line feed */
-#define	BS		CTL('H')	/* back space */
-#define	DEL		CTL('?')	/* delete */
-
-/* Defaults for line-editing etc. characters; you may want to change these. */
-#define DEF_ERASE	DEL		/* default erase character */
-#define DEF_INTR	CTL('C')	/* default interrupt character */
-#define DEF_QUIT	CTL('\\')	/* default quit char */
-#define DEF_KILL	CTL('U')	/* default kill char */
-#define DEF_EOF		CTL('D')	/* default EOF char */
-#define DEF_EOL		0
-#define DEF_SWITCH	0		/* default switch char */
-
 /*
  * When multiple baud rates are specified on the command line, the first one
  * we will try is the first one specified.
@@ -178,24 +162,6 @@ struct options {
 #define serial_tty_option(opt, flag)	\
 	(((opt)->flags & (F_VCONSOLE|(flag))) == (flag))
 
-/* Storage for things detected while the login name was read. */
-struct chardata {
-	int erase;		/* erase character */
-	int kill;		/* kill character */
-	int eol;		/* end-of-line character */
-	int parity;		/* what parity did we see */
-	int capslock;		/* upper case without lower case */
-};
-
-/* Initial values for the above. */
-static const struct chardata init_chardata = {
-	DEF_ERASE,		/* default erase character */
-	DEF_KILL,		/* default kill character */
-	13,			/* default eol char */
-	0,			/* space parity */
-	0,			/* no capslock */
-};
-
 struct Speedtab {
 	long speed;
 	speed_t code;
-- 
1.7.9.2

Re: [PATCH] Move struct chardata and init_chardata to ttyutils

[Karel Zak]

On Wed, Dec 19, 2012 at 03:28:41PM +0100, Werner Fink wrote:
> this one moves the struct chardata and init_chardata to include/ttyutils.h
> as well as to lib/include/ttyutils.c.  Also the macros CTL/CTRL are fixed
> in agetty.c and sulogin.c to use the XOR variant CTL.

 Merged, I have replaced the global variable with INIT_CHARDATA macro.

    Karel

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