* [PATCH 3/3] agetty: Reload only if it is really needed
@ 2018-10-10 17:26 Stanislav Brabec
2018-10-11 12:23 ` Karel Zak
0 siblings, 1 reply; 4+ messages in thread
From: Stanislav Brabec @ 2018-10-10 17:26 UTC (permalink / raw)
To: util-linux
If netlink event arrives and related escapes are part of issue, agetty
reloads and re-display the prompt. Reload is triggered not only by IP
address change, but also by IPv6 RAs. In some environments it causes
reload several times in a minute, and even complicates the login.
To prevent this, reload only if a real change appears.
This consists of:
split print_issue_file() to several functions:
eval_issue_file() prints issue to memory. It does not affect terminal in
any way.
print_issue_file() prints issue file from memory.
cmp_issue_file() compares the issue file and returns true, if reload is
needed.
The implementation requires additional change:
do_prompt() does not evaluate the issue file. It is responsibility of
calling function.
Test suite:
Use issue that contais \4 and/or \6 escape.
After installing new instance, restart agetty by typing a letter and then
Enter 6 times.
To check whether reload happens, type a letter. When reload happens,
letter disappears.
1. Unplug network cable. Wait a while and re-plug network cable.
You should see 2 reloads on single stack and 3 reloads on dual stack.
2. Run a loop
while : ; do
sed -i '$areload_test' /etc/issue
agetty --reload
sleep 3
sed -i '/reload_test/d' /etc/issue
agetty --reload
sleep 3
done
You should see regular reload every 3 seconds.
3. Run a loop
while : ; do
agetty --reload
sleep 3
done
Before: You see regular reload every 3 seconds.
After: No reloads.
4. Run a loop
while : ; do
ifconfig lo 127.0.0.1 netmask 255.0.0.0
sleep 3
ifconfig lo 127.0.0.2 netmask 255.0.0.0
sleep 3
done
Before: You see regular reload every 3 seconds.
After: No reloads.
Signed-off-by: Stanislav Brabec <sbrabec@suse.cz>
---
term-utils/agetty.c | 141 +++++++++++++++++++++++++++++++-------------
1 file changed, 100 insertions(+), 41 deletions(-)
diff --git a/term-utils/agetty.c b/term-utils/agetty.c
index 27e97091b..94f517419 100644
--- a/term-utils/agetty.c
+++ b/term-utils/agetty.c
@@ -28,6 +28,7 @@
#include <utmpx.h>
#include <getopt.h>
#include <time.h>
+#include <stdbool.h>
#include <sys/socket.h>
#include <langinfo.h>
#include <grp.h>
@@ -328,6 +329,7 @@ static void check_username (const char* nm);
static void login_options_to_argv(char *argv[], int *argc, char *str, char *username);
static void reload_agettys(void);
static void print_issue_file(struct options *op, struct termios *tp);
+static void eval_issue_file(struct options *op, struct termios *tp);
/* Fake hostname for ut_host specified on command line. */
static char *fakehost;
@@ -463,10 +465,12 @@ int main(int argc, char **argv)
}
if (options.flags & F_NOPROMPT) { /* --skip-login */
+ eval_issue_file(&options, &termios);
print_issue_file(&options, &termios);
} else { /* regular (auto)login */
if (options.autolog) {
/* Autologin prompt */
+ eval_issue_file(&options, &termios);
do_prompt(&options, &termios);
printf(_("%s%s (automatic login)\n"), LOGIN, options.autolog);
} else {
@@ -1721,9 +1725,66 @@ static void print_issue_file(struct options *op, struct termios *tp __attribute_
write_all(STDOUT_FILENO, "\r\n", 2);
}
}
+static void eval_issue_file(struct options *op __attribute__((__unused__)), struct termios *tp __attribute__((__unused__)))
+{
+}
#else /* ISSUE_SUPPORT */
+static FILE *issue_f;
+static char *issue_mem = NULL;
+static size_t issue_mem_sz;
+static bool do_tcsetattr;
+static bool do_tcrestore;
+#ifdef AGETTY_RELOAD
+static char *issue_mem_old = NULL;
+#endif
+static bool cmp_issue_file(void) {
+ if (issue_mem_old && issue_mem) {
+ if (!strcmp(issue_mem_old, issue_mem)) {
+ free(issue_mem_old);
+ issue_mem_old = issue_mem;
+ issue_mem = NULL;
+ return false;
+ }
+ } else
+ return true;
+ return true;
+}
static void print_issue_file(struct options *op, struct termios *tp)
+{
+ int oflag = tp->c_oflag; /* Save current setting. */
+
+ if ((op->flags & F_NONL) == 0) {
+ /* Issue not in use, start with a new line. */
+ write_all(STDOUT_FILENO, "\r\n", 2);
+ }
+
+ if (do_tcsetattr) {
+ if ((op->flags & F_VCONSOLE) == 0) {
+ /* Map new line in output to carriage return & new line. */
+ tp->c_oflag |= (ONLCR | OPOST);
+ tcsetattr(STDIN_FILENO, TCSADRAIN, tp);
+ }
+ }
+
+ write_all(STDOUT_FILENO, issue_mem, issue_mem_sz);
+ if (do_tcrestore) {
+ /* Restore settings. */
+ tp->c_oflag = oflag;
+ /* Wait till output is gone. */
+ tcsetattr(STDIN_FILENO, TCSADRAIN, tp);
+ }
+#ifdef AGETTY_RELOAD
+ free(issue_mem_old);
+ issue_mem_old = issue_mem;
+ issue_mem = NULL;
+#else
+ free(issue_mem);
+ issue_mem = NULL;
+#endif
+}
+
+static void eval_issue_file(struct options *op, struct termios *tp)
{
const char *filename, *dirname = NULL;
FILE *f = NULL;
@@ -1734,10 +1795,6 @@ static void print_issue_file(struct options *op, struct termios *tp)
#ifdef AGETTY_RELOAD
netlink_groups = 0;
#endif
- if ((op->flags & F_NONL) == 0) {
- /* Issue not in use, start with a new line. */
- write_all(STDOUT_FILENO, "\r\n", 2);
- }
if (!(op->flags & F_ISSUE))
return;
@@ -1768,6 +1825,7 @@ static void print_issue_file(struct options *op, struct termios *tp)
dirname = _PATH_ISSUEDIR;
}
+ issue_f = open_memstream(&issue_mem, &issue_mem_sz);
#ifdef ISSUEDIR_SUPPORT
if (dirname) {
dd = open(dirname, O_RDONLY|O_CLOEXEC|O_DIRECTORY);
@@ -1782,13 +1840,9 @@ static void print_issue_file(struct options *op, struct termios *tp)
f = fopen(filename, "r");
if (f || dirname) {
- int c, oflag = tp->c_oflag; /* Save current setting. */
+ int c;
- if ((op->flags & F_VCONSOLE) == 0) {
- /* Map new line in output to carriage return & new line. */
- tp->c_oflag |= (ONLCR | OPOST);
- tcsetattr(STDIN_FILENO, TCSADRAIN, tp);
- }
+ do_tcsetattr = true;
do {
#ifdef ISSUEDIR_SUPPORT
@@ -1801,7 +1855,7 @@ static void print_issue_file(struct options *op, struct termios *tp)
if (c == '\\')
output_special_char(getc(f), op, tp, f);
else
- putchar(c);
+ putc(c, issue_f);
}
fclose(f);
f = NULL;
@@ -1809,12 +1863,8 @@ static void print_issue_file(struct options *op, struct termios *tp)
fflush(stdout);
- if ((op->flags & F_VCONSOLE) == 0) {
- /* Restore settings. */
- tp->c_oflag = oflag;
- /* Wait till output is gone. */
- tcsetattr(STDIN_FILENO, TCSADRAIN, tp);
- }
+ if ((op->flags & F_VCONSOLE) == 0)
+ do_tcrestore = true;
}
#ifdef ISSUEDIR_SUPPORT
@@ -1828,6 +1878,7 @@ static void print_issue_file(struct options *op, struct termios *tp)
if (netlink_groups != 0)
open_netlink();
#endif
+ fclose(issue_f);
}
#endif /* ISSUE_SUPPORT */
@@ -1842,11 +1893,14 @@ again:
if (op->flags & F_LOGINPAUSE) {
puts(_("[press ENTER to login]"));
#ifdef AGETTY_RELOAD
+ /* reload issue */
if (!wait_for_term_input(STDIN_FILENO)) {
- /* reload issue */
- if (op->flags & F_VCONSOLE)
- termio_clear(STDOUT_FILENO);
- goto again;
+ eval_issue_file(op, tp);
+ if (cmp_issue_file()) {
+ if (op->flags & F_VCONSOLE)
+ termio_clear(STDOUT_FILENO);
+ goto again;
+ }
}
#endif
getc(stdin);
@@ -1967,17 +2021,22 @@ static char *get_logname(struct options *op, struct termios *tp, struct chardata
bp = logname;
*bp = '\0';
+ eval_issue_file(op, tp);
while (*logname == '\0') {
/* Write issue file and prompt */
do_prompt(op, tp);
#ifdef AGETTY_RELOAD
+ no_reload:
if (!wait_for_term_input(STDIN_FILENO)) {
/* refresh prompt -- discard input data, clear terminal
* and call do_prompt() again
*/
if ((op->flags & F_VCONSOLE) == 0)
sleep(1);
+ eval_issue_file(op, tp);
+ if (!cmp_issue_file())
+ goto no_reload;
tcflush(STDIN_FILENO, TCIFLUSH);
if (op->flags & F_VCONSOLE)
termio_clear(STDOUT_FILENO);
@@ -2365,7 +2424,7 @@ static void print_addr(sa_family_t family, void *addr)
char buff[INET6_ADDRSTRLEN + 1];
inet_ntop(family, addr, buff, sizeof(buff));
- printf("%s", buff);
+ fprintf(issue_f, "%s", buff);
}
/*
@@ -2487,36 +2546,36 @@ static void output_special_char(unsigned char c, struct options *op,
if (get_escape_argument(fp, escname, sizeof(escname))) {
const char *esc = color_sequence_from_colorname(escname);
if (esc)
- fputs(esc, stdout);
+ fputs(esc, issue_f);
} else
- fputs("\033", stdout);
+ fputs("\033", issue_f);
break;
}
case 's':
uname(&uts);
- printf("%s", uts.sysname);
+ fprintf(issue_f, "%s", uts.sysname);
break;
case 'n':
uname(&uts);
- printf("%s", uts.nodename);
+ fprintf(issue_f, "%s", uts.nodename);
break;
case 'r':
uname(&uts);
- printf("%s", uts.release);
+ fprintf(issue_f, "%s", uts.release);
break;
case 'v':
uname(&uts);
- printf("%s", uts.version);
+ fprintf(issue_f, "%s", uts.version);
break;
case 'm':
uname(&uts);
- printf("%s", uts.machine);
+ fprintf(issue_f, "%s", uts.machine);
break;
case 'o':
{
char *dom = xgetdomainname();
- fputs(dom ? dom : "unknown_domain", stdout);
+ fputs(dom ? dom : "unknown_domain", issue_f);
free(dom);
break;
}
@@ -2536,7 +2595,7 @@ static void output_special_char(unsigned char c, struct options *op,
(canon = strchr(info->ai_canonname, '.')))
dom = canon + 1;
}
- fputs(dom ? dom : "unknown_domain", stdout);
+ fputs(dom ? dom : "unknown_domain", issue_f);
if (info)
freeaddrinfo(info);
free(host);
@@ -2555,19 +2614,19 @@ static void output_special_char(unsigned char c, struct options *op,
break;
if (c == 'd') /* ISO 8601 */
- printf("%s %s %d %d",
+ fprintf(issue_f, "%s %s %d %d",
nl_langinfo(ABDAY_1 + tm->tm_wday),
nl_langinfo(ABMON_1 + tm->tm_mon),
tm->tm_mday,
tm->tm_year < 70 ? tm->tm_year + 2000 :
tm->tm_year + 1900);
else
- printf("%02d:%02d:%02d",
+ fprintf(issue_f, "%02d:%02d:%02d",
tm->tm_hour, tm->tm_min, tm->tm_sec);
break;
}
case 'l':
- printf ("%s", op->tty);
+ fprintf (issue_f, "%s", op->tty);
break;
case 'b':
{
@@ -2576,7 +2635,7 @@ static void output_special_char(unsigned char c, struct options *op,
for (i = 0; speedtab[i].speed; i++) {
if (speedtab[i].code == speed) {
- printf("%ld", speedtab[i].speed);
+ fprintf(issue_f, "%ld", speedtab[i].speed);
break;
}
}
@@ -2591,18 +2650,18 @@ static void output_special_char(unsigned char c, struct options *op,
var = read_os_release(op, varname);
if (var) {
if (strcmp(varname, "ANSI_COLOR") == 0)
- printf("\033[%sm", var);
+ fprintf(issue_f, "\033[%sm", var);
else
- fputs(var, stdout);
+ fputs(var, issue_f);
}
/* \S */
} else if ((var = read_os_release(op, "PRETTY_NAME"))) {
- fputs(var, stdout);
+ fputs(var, issue_f);
/* \S and PRETTY_NAME not found */
} else {
uname(&uts);
- fputs(uts.sysname, stdout);
+ fputs(uts.sysname, issue_f);
}
free(var);
@@ -2620,9 +2679,9 @@ static void output_special_char(unsigned char c, struct options *op,
users++;
endutxent();
if (c == 'U')
- printf(P_("%d user", "%d users", users), users);
+ fprintf(issue_f, P_("%d user", "%d users", users), users);
else
- printf ("%d ", users);
+ fprintf (issue_f, "%d ", users);
break;
}
case '4':
--
2.19.0
--
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 3/3] agetty: Reload only if it is really needed
2018-10-10 17:26 [PATCH 3/3] agetty: Reload only if it is really needed Stanislav Brabec
@ 2018-10-11 12:23 ` Karel Zak
2018-10-11 19:37 ` Stanislav Brabec
0 siblings, 1 reply; 4+ messages in thread
From: Karel Zak @ 2018-10-11 12:23 UTC (permalink / raw)
To: Stanislav Brabec; +Cc: util-linux
On Wed, Oct 10, 2018 at 07:26:34PM +0200, Stanislav Brabec wrote:
> If netlink event arrives and related escapes are part of issue, agetty
> reloads and re-display the prompt. Reload is triggered not only by IP
> address change, but also by IPv6 RAs. In some environments it causes
> reload several times in a minute, and even complicates the login.
>
> To prevent this, reload only if a real change appears.
Applied, thanks.
I have applied another changes too:
- move all issue stuff to 'struct issue'
- rename a few things
- make code more robust
> This consists of:
> split print_issue_file() to several functions:
>
> eval_issue_file() prints issue to memory. It does not affect terminal in
> any way.
>
> print_issue_file() prints issue file from memory.
>
> cmp_issue_file() compares the issue file and returns true, if reload is
> needed.
Let's hope people do not use \4 and \6 together with 't' because it
means time string in the output :-)
> Test suite:
Can you retest the current master branch with my changes? I do not
expect any issue, but for sure...
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 3/3] agetty: Reload only if it is really needed
2018-10-11 12:23 ` Karel Zak
@ 2018-10-11 19:37 ` Stanislav Brabec
2018-10-12 8:36 ` Karel Zak
0 siblings, 1 reply; 4+ messages in thread
From: Stanislav Brabec @ 2018-10-11 19:37 UTC (permalink / raw)
To: Karel Zak; +Cc: util-linux
Karel Zak wrote:
> Can you retest the current master branch with my changes? I do not
> expect any issue, but for sure...
Works as expected.
There is one remaining issue:
Reload happens even if user already started to type logname. It is very
uncomfortable, and it can even lead to problems with login. I tried to
fix that: Once any letter is already entered, reload will be blocked
and postponed.
To fix this, we would need to disable reloads once any character was
entered. It would require switch to character mode and custom handling
of backspace. I am not sure, whether we want this.
--
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 3/3] agetty: Reload only if it is really needed
2018-10-11 19:37 ` Stanislav Brabec
@ 2018-10-12 8:36 ` Karel Zak
0 siblings, 0 replies; 4+ messages in thread
From: Karel Zak @ 2018-10-12 8:36 UTC (permalink / raw)
To: Stanislav Brabec; +Cc: util-linux
On Thu, Oct 11, 2018 at 09:37:07PM +0200, Stanislav Brabec wrote:
> Karel Zak wrote:
> > Can you retest the current master branch with my changes? I do not
> > expect any issue, but for sure...
> Works as expected.
>
> There is one remaining issue:
>
> Reload happens even if user already started to type logname. It is very
> uncomfortable
I thought about it too.
> and it can even lead to problems with login. I tried to
> fix that: Once any letter is already entered, reload will be blocked
> and postponed.
>
> To fix this, we would need to disable reloads once any character was
> entered. It would require switch to character mode and custom handling
> of backspace. I am not sure, whether we want this.
We already had implementation based on character mode for the select()
and it was pretty fragile (see commit message 2a14beb4e9c6cdf4466993741d86e45dd57ddef3).
The problem was that the code tried to switch back to line mode within
get_logname() to manage control keys (del/backspace) by terminal. The
switch between the modes is really bad idea in time you read from the
terminal.
The character mode for all get_logname() is definitely good idea for
the reload issue, the question is how tricky will be to manage login
name string on terminal. You need to refresh all the output line on
del/backspace, etc.
Send patch ;-)
Karel
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-10-12 8:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-10 17:26 [PATCH 3/3] agetty: Reload only if it is really needed Stanislav Brabec
2018-10-11 12:23 ` Karel Zak
2018-10-11 19:37 ` Stanislav Brabec
2018-10-12 8:36 ` Karel Zak
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).