ÂHola Peter!
El 2013-08-19 a las 08:25 -0400, Peter Hurley escribiÃ:My primary concern is canonical readers not become stuck with a full
read buffer, even with bogus input data (IOW, that an error condition will
not prevent a reader from making forward progress). I believe that won't
happen with this change, but what I really need in this case is a detailed
analysis from you of why that won't happen. That analysis should be in
the patch changelog. (Feel free to send me private mail if you need help
preparing a patch.)
I'm not sure what level of analysis you are looking for.
The driver will block
when there are no readers but as soon as there is a read call it unblocks.
I've added this information to the patch description that I'm including below.
And the patch above has a bug that allows a negative 'left' to be
assigned to tty->receive_room which will be interpreted as a very large
positive value.
Ok, fixed with an else clause. It could also use an extra &&, but it looks a
bit confusing.
This approach still has several drawbacks.
1) Since additional state is reset when the termios is changed by
readline(), the canonical line buffer state will be bogus.
This renders the termios change by readline() pointless; the
caller will not be able to retrieve expected input properly.
2) Since the input data is interpreted with the current termios when
data is received, any embedded control characters will not be
interpreted properly; again, the caller will not be able to retrieve
expected input properly.
Indeed this is correct, however this is not an issue of this patch but of the
current interaction between the kernel and readline.
In order to fix this, the
reading buffer should always be in raw and only when responding to a read call
for canonical mode should it be interpreted. This is a very big change, and
I'm not sure if anybody will be interested in implementing it.
What do you think? Is the proposed solution, or something along those lines,
acceptable?
I'm wondering if this problem might be best addressed on the paste side
instead of the read side. Although this wouldn't be a magic bullet, it
would be easier to control when more paste data is added.
I don't see how this could work, could you elaborate?
This is the patch proposal, for comments:
From 81afd3b666cbf94bb9923ebf87fb2017a7cd645e Mon Sep 17 00:00:00 2001
From: Maximiliano Curia <maxy@xxxxxxxxxxxxxxxxx>
Date: Tue, 3 Sep 2013 22:48:34 +0200
Subject: [PATCH] Only let characters through when there are active readers.
If there is an active reader, previous behavior is in place. When there is no
active reader, input is blocked until the next read call unblocks it.
This fixes a long standing issue with readline when pasting more than 4096
bytes.
---
drivers/tty/n_tty.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 4bf0fc0..cdc3b19 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -147,9 +147,16 @@ static int set_room(struct tty_struct *tty)
* pending newlines, let characters through without limit, so
* that erase characters will be handled. Other excess
* characters will be beeped.
+ * If there is no reader waiting for the input, block instead of
+ * letting the characters through.
*/
if (left <= 0)
- left = ldata->icanon && !ldata->canon_data;
+ if (waitqueue_active(&tty->read_wait)) {
+ left = ldata->icanon && !ldata->canon_data;
+ } else {
+ left = 0;
+ }
+
old_left = tty->receive_room;
tty->receive_room = left;