Skip to content

Commit

Permalink
agetty: use CTRL+C to erase username
Browse files Browse the repository at this point in the history 
aggety(8) from the beginning ignores ^C (the small exception was
between 2.32 and 2.34 when this char has been misinterpreted).

This patch forces agetty to interpret ^C like ^U, it means to
erase the user's input and wait for a completely new username.
The small difference is that for ^C it does not set 'kill character'.

This change does not affect serial lines where ^C is still ignored like
in previous decades. I'd like to avoid any regression as I have
no clue if any serial lines do not send this control char in some
context ...

Fixes: #1399
References: #1046
Signed-off-by: Karel Zak <kzak@redhat.com>
  • Loading branch information
@karelzak
karelzak committed Jul 30, 2021
1 parent 37c6c57 commit 6eb1c01
Showing 1 changed file with 5 additions and 3 deletions.
8 changes: 5 additions & 3 deletions term-utils/agetty.c
View file
Expand Up @@ -2267,6 +2267,11 @@ static char *get_logname(struct issue *ie, struct options *op, struct termios *t
break;
case CTL('U'):
cp->kill = ascval; /* set kill character */
/* fallthrough */
case CTL('C'):
if (key == CTL('C') && !(op->flags & F_VCONSOLE))
/* Ignore CTRL+C on serial line */
break;
while (bp > logname) {
if ((tp->c_lflag & ECHO) == 0)
write_all(1, erase[cp->parity], 3);
Expand All @@ -2275,9 +2280,6 @@ static char *get_logname(struct issue *ie, struct options *op, struct termios *t
break;
case CTL('D'):
exit(EXIT_SUCCESS);
case CTL('C'):
/* Ignore */
break;
default:
if ((size_t)(bp - logname) >= sizeof(logname) - 1)
log_err(_("%s: input overrun"), op->tty);
Expand Down

4 comments on commit 6eb1c01

@hackerb9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I presume this is correct, I found it harder to follow the flow of control. Is there a cleaner way to do the same thing?

@karelzak
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the readability of the change? It would be possible to create a separate case for CTRL+C and duplicate the code that we use to delete the username, but I do not like duplicate code :-)

@hackerb9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the change makes the code harder for me to follow.

I also don't like duplication; it means having to be careful to keep them in sync in the future. There may be a way to arrange the flow without duplication, but I don't see a good one. Right now, it is not clear that ^C is supposed to be the same as ^U, with each running different code first.

If the three lines of shared code are too small to put in a function, but too large to duplicate, perhaps this is one of the rare times where goto is warranted?

@hackerb9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I am reluctant to suggest it, I think duplication of those three lines is the best solution. It is not ideal, but it will help future maintainers to understand what the code is trying to do.

Please sign in to comment.