[U-Boot] [PATCH 1/2] console: Remember if ctrlc is disabled in console_tstc()

We don't necessarily want to re-enable ctrl-c if it was already disabled when calling tstc().
Signed-off-by: Joe Hershberger joe.hershberger@ni.com ---
common/console.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/common/console.c b/common/console.c index 2ba33dc574..36c0568dbf 100644 --- a/common/console.c +++ b/common/console.c @@ -196,20 +196,21 @@ static int console_tstc(int file) { int i, ret; struct stdio_dev *dev; + int prev;
- disable_ctrlc(1); + prev = disable_ctrlc(1); for (i = 0; i < cd_count[file]; i++) { dev = console_devices[file][i]; if (dev->tstc != NULL) { ret = dev->tstc(dev); if (ret > 0) { tstcdev = dev; - disable_ctrlc(0); + disable_ctrlc(prev); return ret; } } } - disable_ctrlc(0); + disable_ctrlc(prev);
return 0; }

In raw mode, handle ctrl-c as normal. This allows normal ctrl-c behavior such as aborting a command that is timing out without completely terminating the sandbox executable.
In [1], Simon disabled this. His reason for it was that it interferes with piping test scripts. Piping should be done in cooked mode, so this change should still not interfere.
[1] commit 8969ea3e9f2db04a6b3675 ("sandbox: Disable Ctrl-C")
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
---
common/console.c | 2 -- drivers/serial/sandbox.c | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/common/console.c b/common/console.c index 36c0568dbf..7aa58d0a63 100644 --- a/common/console.c +++ b/common/console.c @@ -604,7 +604,6 @@ static int ctrlc_disabled = 0; /* see disable_ctrl() */ static int ctrlc_was_pressed = 0; int ctrlc(void) { -#ifndef CONFIG_SANDBOX if (!ctrlc_disabled && gd->have_console) { if (tstc()) { switch (getc()) { @@ -616,7 +615,6 @@ int ctrlc(void) } } } -#endif
return 0; } diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c index d2e007284c..771f8f0a44 100644 --- a/drivers/serial/sandbox.c +++ b/drivers/serial/sandbox.c @@ -69,6 +69,9 @@ static int sandbox_serial_probe(struct udevice *dev) os_tty_raw(0, state->term_raw == STATE_TERM_RAW_WITH_SIGS); priv->start_of_line = 0;
+ if (state->term_raw != STATE_TERM_RAW) + disable_ctrlc(1); + return 0; }

Hi Joe,
On 2 July 2018 at 18:06, Joe Hershberger joe.hershberger@ni.com wrote:
In raw mode, handle ctrl-c as normal. This allows normal ctrl-c behavior such as aborting a command that is timing out without completely terminating the sandbox executable.
In [1], Simon disabled this. His reason for it was that it interferes with piping test scripts. Piping should be done in cooked mode, so this change should still not interfere.
[1] commit 8969ea3e9f2db04a6b3675 ("sandbox: Disable Ctrl-C")
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
common/console.c | 2 -- drivers/serial/sandbox.c | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-)
It is designed so that ctrl-C exits in raw mode. That is the normal behaviour for an application and I don't think sandbox should be any different.
How about using cooked mode instead?
Regards, Simon

On Sun, Jul 8, 2018 at 9:39 PM, Simon Glass sjg@chromium.org wrote:
Hi Joe,
On 2 July 2018 at 18:06, Joe Hershberger joe.hershberger@ni.com wrote:
In raw mode, handle ctrl-c as normal. This allows normal ctrl-c behavior such as aborting a command that is timing out without completely terminating the sandbox executable.
In [1], Simon disabled this. His reason for it was that it interferes with piping test scripts. Piping should be done in cooked mode, so this change should still not interfere.
[1] commit 8969ea3e9f2db04a6b3675 ("sandbox: Disable Ctrl-C")
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
common/console.c | 2 -- drivers/serial/sandbox.c | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-)
It is designed so that ctrl-C exits in raw mode. That is the normal behaviour for an application and I don't think sandbox should be any different.
How about using cooked mode instead?
That seems backward. Only in raw mode (STATE_TERM_RAW) is the console not interfering with keyboard inputs and changing behaviors based on interpreting control codes. In raw mode, U-Boot sandbox can handle things the way it does in real hardware.
To be clear, this is not the default case (STATE_TERM_RAW_WITH_SIGS) where ctrl-C exits. In fully raw mode (STATE_TERM_RAW), ctrl-C does absolutely nothing without this patch.
Also, if cooked mode were used, it would once again break the test script piping that your previous commit talked about. I'm not sure exactly what that applies to (would it affect Travis such that I can validate interference?) so I'm not sure I've tested that use-case effectively.
Thanks, -Joe

Hi Joe,
On 9 July 2018 at 13:26, Joe Hershberger joe.hershberger@ni.com wrote:
On Sun, Jul 8, 2018 at 9:39 PM, Simon Glass sjg@chromium.org wrote:
Hi Joe,
On 2 July 2018 at 18:06, Joe Hershberger joe.hershberger@ni.com wrote:
In raw mode, handle ctrl-c as normal. This allows normal ctrl-c behavior such as aborting a command that is timing out without completely terminating the sandbox executable.
In [1], Simon disabled this. His reason for it was that it interferes with piping test scripts. Piping should be done in cooked mode, so this change should still not interfere.
[1] commit 8969ea3e9f2db04a6b3675 ("sandbox: Disable Ctrl-C")
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
common/console.c | 2 -- drivers/serial/sandbox.c | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-)
It is designed so that ctrl-C exits in raw mode. That is the normal behaviour for an application and I don't think sandbox should be any different.
How about using cooked mode instead?
That seems backward. Only in raw mode (STATE_TERM_RAW) is the console not interfering with keyboard inputs and changing behaviors based on interpreting control codes. In raw mode, U-Boot sandbox can handle things the way it does in real hardware.
To be clear, this is not the default case (STATE_TERM_RAW_WITH_SIGS) where ctrl-C exits. In fully raw mode (STATE_TERM_RAW), ctrl-C does absolutely nothing without this patch.
Also, if cooked mode were used, it would once again break the test script piping that your previous commit talked about. I'm not sure exactly what that applies to (would it affect Travis such that I can validate interference?) so I'm not sure I've tested that use-case effectively.
Er yes, sorry, I mean raw mode. I misunderstood your commit message I think.
Reviewed-by: Simon Glass sjg@chromium.org
Regards, Simon

On Tue, Jul 10, 2018 at 3:49 PM, Simon Glass sjg@chromium.org wrote:
Hi Joe,
On 9 July 2018 at 13:26, Joe Hershberger joe.hershberger@ni.com wrote:
On Sun, Jul 8, 2018 at 9:39 PM, Simon Glass sjg@chromium.org wrote:
Hi Joe,
On 2 July 2018 at 18:06, Joe Hershberger joe.hershberger@ni.com wrote:
In raw mode, handle ctrl-c as normal. This allows normal ctrl-c behavior such as aborting a command that is timing out without completely terminating the sandbox executable.
In [1], Simon disabled this. His reason for it was that it interferes with piping test scripts. Piping should be done in cooked mode, so this change should still not interfere.
[1] commit 8969ea3e9f2db04a6b3675 ("sandbox: Disable Ctrl-C")
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
common/console.c | 2 -- drivers/serial/sandbox.c | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-)
It is designed so that ctrl-C exits in raw mode. That is the normal behaviour for an application and I don't think sandbox should be any different.
How about using cooked mode instead?
That seems backward. Only in raw mode (STATE_TERM_RAW) is the console not interfering with keyboard inputs and changing behaviors based on interpreting control codes. In raw mode, U-Boot sandbox can handle things the way it does in real hardware.
To be clear, this is not the default case (STATE_TERM_RAW_WITH_SIGS) where ctrl-C exits. In fully raw mode (STATE_TERM_RAW), ctrl-C does absolutely nothing without this patch.
Also, if cooked mode were used, it would once again break the test script piping that your previous commit talked about. I'm not sure exactly what that applies to (would it affect Travis such that I can validate interference?) so I'm not sure I've tested that use-case effectively.
Er yes, sorry, I mean raw mode. I misunderstood your commit message I think.
Reviewed-by: Simon Glass sjg@chromium.org
You want to take this series in or should I?
Thanks, -Joe

Hi Joe,
On 11 July 2018 at 13:32, Joe Hershberger joe.hershberger@ni.com wrote:
On Tue, Jul 10, 2018 at 3:49 PM, Simon Glass sjg@chromium.org wrote:
Hi Joe,
On 9 July 2018 at 13:26, Joe Hershberger joe.hershberger@ni.com wrote:
On Sun, Jul 8, 2018 at 9:39 PM, Simon Glass sjg@chromium.org wrote:
Hi Joe,
On 2 July 2018 at 18:06, Joe Hershberger joe.hershberger@ni.com wrote:
In raw mode, handle ctrl-c as normal. This allows normal ctrl-c behavior such as aborting a command that is timing out without completely terminating the sandbox executable.
In [1], Simon disabled this. His reason for it was that it interferes with piping test scripts. Piping should be done in cooked mode, so this change should still not interfere.
[1] commit 8969ea3e9f2db04a6b3675 ("sandbox: Disable Ctrl-C")
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
common/console.c | 2 -- drivers/serial/sandbox.c | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-)
It is designed so that ctrl-C exits in raw mode. That is the normal behaviour for an application and I don't think sandbox should be any different.
How about using cooked mode instead?
That seems backward. Only in raw mode (STATE_TERM_RAW) is the console not interfering with keyboard inputs and changing behaviors based on interpreting control codes. In raw mode, U-Boot sandbox can handle things the way it does in real hardware.
To be clear, this is not the default case (STATE_TERM_RAW_WITH_SIGS) where ctrl-C exits. In fully raw mode (STATE_TERM_RAW), ctrl-C does absolutely nothing without this patch.
Also, if cooked mode were used, it would once again break the test script piping that your previous commit talked about. I'm not sure exactly what that applies to (would it affect Travis such that I can validate interference?) so I'm not sure I've tested that use-case effectively.
Er yes, sorry, I mean raw mode. I misunderstood your commit message I think.
Reviewed-by: Simon Glass sjg@chromium.org
You want to take this series in or should I?
Please go ahead.
Regards, Simon

Hi Joe,
https://patchwork.ozlabs.org/patch/938299/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git
Thanks! -Joe

On 2 July 2018 at 18:06, Joe Hershberger joe.hershberger@ni.com wrote:
We don't necessarily want to re-enable ctrl-c if it was already disabled when calling tstc().
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
common/console.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Hi Joe,
https://patchwork.ozlabs.org/patch/938298/ was applied to http://git.denx.de/?p=u-boot/u-boot-net.git
Thanks! -Joe
participants (2)
-
Joe Hershberger
-
Simon Glass