2007-12-22 14:20:14

by Joe Peterson

[permalink] [raw]
Subject: [PATCH] Fix IXANY and restart after signal (e.g. ctrl-C) in n_tty line discipline

Fix two N_TTY line discipline issues related to resuming a stopped TTY
(typically done with ctrl-S):

1) Fix handling of character that resumes a stopped TTY (with IXANY)

With "stty ixany", the TTY line discipline would lose the first character
after the stop, so typing, for example, "hi^Sthere" resulted in "hihere"
(the 't' would cause the resume after ^S, but it would then be thrown away
rather than processed as an input character). This was inconsistent with
the behavior of other Unix systems.

2) Fix interrupt signal (e.g. ctrl-C) behavior in stopped TTYs

With "stty -ixany" (often the default), interrupt signals were ignored
in a stopped TTY until the TTY was resumed with the start char (typically
ctrl-Q), which was inconsistent with the behavior of other Unix systems.

Signed-off-by: Joe Peterson <[email protected]>
---

diff -puN a/drivers/char/n_tty.c~tty-fix-ixany-and-restart-after-signal-in-the-n_tty-discipline a/drivers/char/n_tty.c
--- a/drivers/char/n_tty.c~tty-fix-ixany-and-restart-after-signal-in-the-n_tty-discipline 2007-12-20 11:35:05.000000000 -0700
+++ a/drivers/char/n_tty.c 2007-12-20 11:35:36.000000000 -0700
@@ -695,11 +695,8 @@ static inline void n_tty_receive_char(st
return;
}

- if (tty->stopped && !tty->flow_stopped &&
- I_IXON(tty) && I_IXANY(tty)) {
+ if (tty->stopped && !tty->flow_stopped && I_IXON(tty) && I_IXANY(tty))
start_tty(tty);
- return;
- }

if (I_ISTRIP(tty))
c &= 0x7f;
@@ -769,6 +766,8 @@ static inline void n_tty_receive_char(st
signal = SIGTSTP;
if (c == SUSP_CHAR(tty)) {
send_signal:
+ if (tty->stopped && !tty->flow_stopped && I_IXON(tty))
+ start_tty(tty);
/*
* Echo character, and then send the signal.
* Note that we do not use isig() here because we want


Attachments:
ixany-restart.patch (1.73 kB)

2008-01-17 23:43:27

by Joe Peterson

[permalink] [raw]
Subject: [PATCH] (*revised*) Fix IXANY and restart after signal (e.g. ctrl-C) in n_tty line discipline

Fix two N_TTY line discipline issues related to resuming a stopped TTY
(typically done with ctrl-S):

1) Fix handling of character that resumes a stopped TTY (with IXANY)

With "stty ixany", the TTY line discipline would lose the first character
after the stop, so typing, for example, "hi^Sthere" resulted in "hihere"
(the 't' would cause the resume after ^S, but it would then be thrown away
rather than processed as an input character). This was inconsistent with
the behavior of other Unix systems.

2) Fix interrupt signal (e.g. ctrl-C) behavior in stopped TTYs

With "stty -ixany" (often the default), interrupt signals were ignored
in a stopped TTY until the TTY was resumed with the start char (typically
ctrl-Q), which was inconsistent with the behavior of other Unix systems.

Signed-off-by: Joe Peterson <[email protected]>
---

diff -puN drivers/char/n_tty.c~fix-ixany-and-restart-after-signal-eg-ctrl-c-in-n_tty-line-discipline drivers/char/n_tty.c
--- a/drivers/char/n_tty.c~fix-ixany-and-restart-after-signal-eg-ctrl-c-in-n_tty-line-discipline 2007-12-20 11:35:05.000000000 -0700
+++ a/drivers/char/n_tty.c 2007-12-20 11:35:36.000000000 -0700
@@ -695,17 +695,16 @@
return;
}

- if (tty->stopped && !tty->flow_stopped &&
- I_IXON(tty) && I_IXANY(tty)) {
- start_tty(tty);
- return;
- }
-
if (I_ISTRIP(tty))
c &= 0x7f;
if (I_IUCLC(tty) && L_IEXTEN(tty))
c=tolower(c);

+ if (tty->stopped && !tty->flow_stopped && I_IXON(tty) &&
+ ((I_IXANY(tty) && c != START_CHAR(tty) && c != STOP_CHAR(tty)) ||
+ c == INTR_CHAR(tty) || c == QUIT_CHAR(tty)))
+ start_tty(tty);
+
if (tty->closing) {
if (I_IXON(tty)) {
if (c == START_CHAR(tty))


Attachments:
fix-ixany-and-restart-after-signal-eg-ctrl-c-in-n_tty-line-discipline.patch (1.64 kB)

2008-01-18 00:12:20

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] (*revised*) Fix IXANY and restart after signal (e.g. ctrl-C) in n_tty line discipline

On Thu, 17 Jan 2008 16:44:33 -0700
Joe Peterson <[email protected]> wrote:

> Here is a revised version of my patch, currently in the -mm kernel as
> "fix-ixany-and-restart-after-signal-eg-ctrl-c-in-n_tty-line-discipline".
> It fixes a couple of issues with ttys in the stopped state. See patch
> below for more details.

What were the "couple of issues"?

> This patch should *replace* the old version of the patch.

I don't like replacements a lot. It forces one to re-review the whole
thing from scratch. Which is why I almost always turn the replacement
into an incremental patch, as below.

> It is
> cleaner, and its behavior better matches that of other Unix platforms.
> It also avoids needless/redundant calls to start_tty() when ctrl-S or
> ctrl-Q are hit.
>

--- a/drivers/char/n_tty.c~fix-ixany-and-restart-after-signal-eg-ctrl-c-in-n_tty-line-discipline-update
+++ a/drivers/char/n_tty.c
@@ -695,14 +695,16 @@ static inline void n_tty_receive_char(st
return;
}

- if (tty->stopped && !tty->flow_stopped && I_IXON(tty) && I_IXANY(tty))
- start_tty(tty);
-
if (I_ISTRIP(tty))
c &= 0x7f;
if (I_IUCLC(tty) && L_IEXTEN(tty))
c=tolower(c);

+ if (tty->stopped && !tty->flow_stopped && I_IXON(tty) &&
+ ((I_IXANY(tty) && c != START_CHAR(tty) && c != STOP_CHAR(tty)) ||
+ c == INTR_CHAR(tty) || c == QUIT_CHAR(tty)))
+ start_tty(tty);
+
if (tty->closing) {
if (I_IXON(tty)) {
if (c == START_CHAR(tty))
@@ -766,8 +768,6 @@ static inline void n_tty_receive_char(st
signal = SIGTSTP;
if (c == SUSP_CHAR(tty)) {
send_signal:
- if (tty->stopped && !tty->flow_stopped && I_IXON(tty))
- start_tty(tty);
/*
* Echo character, and then send the signal.
* Note that we do not use isig() here because we want
_

2008-01-18 00:38:49

by Joe Peterson

[permalink] [raw]
Subject: Re: [PATCH] (*revised*) Fix IXANY and restart after signal (e.g. ctrl-C) in n_tty line discipline

Andrew Morton wrote:
> What were the "couple of issues"?

They are the same 2 issues described in the original version of the
patch and included in the text of this one too (i.e. the text at the
start of this latest patch version is the same as before).

>> This patch should *replace* the old version of the patch.
>
> I don't like replacements a lot. It forces one to re-review the whole
> thing from scratch. Which is why I almost always turn the replacement
> into an incremental patch, as below.

I see your point. Cool, thanks for making the update patch, and I'll
submit them like that in the future.

-Thanks, Joe