diff mbox series

[1/1] sandbox: support CTRL-C processing in U-Boot

Message ID 20200917180422.53595-1-xypron.glpk@gmx.de
State Rejected, archived
Delegated to: Simon Glass
Headers show
Series [1/1] sandbox: support CTRL-C processing in U-Boot | expand

Commit Message

Heinrich Schuchardt Sept. 17, 2020, 6:04 p.m. UTC 
Currently if SIGINT is received, it terminates U-Boot. This does not allow
testing the handling of CTRL-C in U-Boot and in UEFI applications.
It is especially annoying when working with the 'mm' command.

Let the serial console driver provide character 0x03 if SIGINT occurs. We
can still exit U-Boot using the reset or poweroff command.

Adjust the sandbox test_ctrl_c() Python test accordingly.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 arch/sandbox/cpu/os.c              | 15 ++++++++++++---
 drivers/serial/sandbox.c           | 11 ++++++++---
 include/os.h                       | 14 ++++++++++++++
 test/py/tests/test_sandbox_exit.py | 10 +++++++++-
 4 files changed, 43 insertions(+), 7 deletions(-)

--
2.28.0

Comments

Simon Glass Sept. 19, 2020, 2:59 a.m. UTC | #1 
Hi Heinrich,

On Thu, 17 Sep 2020 at 12:04, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Currently if SIGINT is received, it terminates U-Boot. This does not allow
> testing the handling of CTRL-C in U-Boot and in UEFI applications.
> It is especially annoying when working with the 'mm' command.
>
> Let the serial console driver provide character 0x03 if SIGINT occurs. We
> can still exit U-Boot using the reset or poweroff command.
>
> Adjust the sandbox test_ctrl_c() Python test accordingly.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  arch/sandbox/cpu/os.c              | 15 ++++++++++++---
>  drivers/serial/sandbox.c           | 11 ++++++++---
>  include/os.h                       | 14 ++++++++++++++
>  test/py/tests/test_sandbox_exit.py | 10 +++++++++-
>  4 files changed, 43 insertions(+), 7 deletions(-)

I'm sorry but I still do want Ctrl-C to work. How about adding a short
option to change the terminal mode?

Regards,
Simon
Heinrich Schuchardt Sept. 19, 2020, 4:19 a.m. UTC | #2 
Am 19. September 2020 04:59:46 MESZ schrieb Simon Glass <sjg@chromium.org>:
>Hi Heinrich,
>
>On Thu, 17 Sep 2020 at 12:04, Heinrich Schuchardt <xypron.glpk@gmx.de>
>wrote:
>>
>> Currently if SIGINT is received, it terminates U-Boot. This does not
>allow
>> testing the handling of CTRL-C in U-Boot and in UEFI applications.
>> It is especially annoying when working with the 'mm' command.
>>
>> Let the serial console driver provide character 0x03 if SIGINT
>occurs. We
>> can still exit U-Boot using the reset or poweroff command.
>>
>> Adjust the sandbox test_ctrl_c() Python test accordingly.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>  arch/sandbox/cpu/os.c              | 15 ++++++++++++---
>>  drivers/serial/sandbox.c           | 11 ++++++++---
>>  include/os.h                       | 14 ++++++++++++++
>>  test/py/tests/test_sandbox_exit.py | 10 +++++++++-
>>  4 files changed, 43 insertions(+), 7 deletions(-)
>
>I'm sorry but I still do want Ctrl-C to work. How about adding a short
>option to change the terminal mode?
>
>Regards,
>Simon


On QEMU you quit via <ctrl+A>X. Can we use the same for the sandbox?

Best regards

Heinrich
Simon Glass Sept. 19, 2020, 1:26 p.m. UTC | #3 
Hi Heinrich,

On Fri, 18 Sep 2020 at 22:19, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Am 19. September 2020 04:59:46 MESZ schrieb Simon Glass <sjg@chromium.org>:
> >Hi Heinrich,
> >
> >On Thu, 17 Sep 2020 at 12:04, Heinrich Schuchardt <xypron.glpk@gmx.de>
> >wrote:
> >>
> >> Currently if SIGINT is received, it terminates U-Boot. This does not
> >allow
> >> testing the handling of CTRL-C in U-Boot and in UEFI applications.
> >> It is especially annoying when working with the 'mm' command.
> >>
> >> Let the serial console driver provide character 0x03 if SIGINT
> >occurs. We
> >> can still exit U-Boot using the reset or poweroff command.
> >>
> >> Adjust the sandbox test_ctrl_c() Python test accordingly.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> ---
> >>  arch/sandbox/cpu/os.c              | 15 ++++++++++++---
> >>  drivers/serial/sandbox.c           | 11 ++++++++---
> >>  include/os.h                       | 14 ++++++++++++++
> >>  test/py/tests/test_sandbox_exit.py | 10 +++++++++-
> >>  4 files changed, 43 insertions(+), 7 deletions(-)
> >
> >I'm sorry but I still do want Ctrl-C to work. How about adding a short
> >option to change the terminal mode?
> >
> >Regards,
> >Simon
>
>
> On QEMU you quit via <ctrl+A>X. Can we use the same for the sandbox?

I just don't like programs that refused to quit when asked. I use
Ctrl-C all the time. Can you not use the -t options to get what you
want? Perhaps we could have an environment variable too?

Regards,
Simon
Heinrich Schuchardt Sept. 19, 2020, 6:07 p.m. UTC | #4 
On 9/19/20 3:26 PM, Simon Glass wrote:
> Hi Heinrich,
>
> On Fri, 18 Sep 2020 at 22:19, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> Am 19. September 2020 04:59:46 MESZ schrieb Simon Glass <sjg@chromium.org>:
>>> Hi Heinrich,
>>>
>>> On Thu, 17 Sep 2020 at 12:04, Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> wrote:
>>>>
>>>> Currently if SIGINT is received, it terminates U-Boot. This does not
>>> allow
>>>> testing the handling of CTRL-C in U-Boot and in UEFI applications.
>>>> It is especially annoying when working with the 'mm' command.
>>>>
>>>> Let the serial console driver provide character 0x03 if SIGINT
>>> occurs. We
>>>> can still exit U-Boot using the reset or poweroff command.
>>>>
>>>> Adjust the sandbox test_ctrl_c() Python test accordingly.
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>>> ---
>>>>  arch/sandbox/cpu/os.c              | 15 ++++++++++++---
>>>>  drivers/serial/sandbox.c           | 11 ++++++++---
>>>>  include/os.h                       | 14 ++++++++++++++
>>>>  test/py/tests/test_sandbox_exit.py | 10 +++++++++-
>>>>  4 files changed, 43 insertions(+), 7 deletions(-)
>>>
>>> I'm sorry but I still do want Ctrl-C to work. How about adding a short
>>> option to change the terminal mode?
>>>
>>> Regards,
>>> Simon
>>
>>
>> On QEMU you quit via <ctrl+A>X. Can we use the same for the sandbox?
>
> I just don't like programs that refused to quit when asked I use> Ctrl-C all the time. Can you not use the -t options to get what you
> want? Perhaps we could have an environment variable too?

Sorry, I was not aware of the -t option.

./u-boot -t raw -d arch/sandbox/dts/test.dtb

behaves according to my preferences.

Best regards

Heinrich
Simon Glass Sept. 20, 2020, 12:47 a.m. UTC | #5 
Hi Heinrich,

On Sat, 19 Sep 2020 at 12:07, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 9/19/20 3:26 PM, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Fri, 18 Sep 2020 at 22:19, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >> Am 19. September 2020 04:59:46 MESZ schrieb Simon Glass <sjg@chromium.org>:
> >>> Hi Heinrich,
> >>>
> >>> On Thu, 17 Sep 2020 at 12:04, Heinrich Schuchardt <xypron.glpk@gmx.de>
> >>> wrote:
> >>>>
> >>>> Currently if SIGINT is received, it terminates U-Boot. This does not
> >>> allow
> >>>> testing the handling of CTRL-C in U-Boot and in UEFI applications.
> >>>> It is especially annoying when working with the 'mm' command.
> >>>>
> >>>> Let the serial console driver provide character 0x03 if SIGINT
> >>> occurs. We
> >>>> can still exit U-Boot using the reset or poweroff command.
> >>>>
> >>>> Adjust the sandbox test_ctrl_c() Python test accordingly.
> >>>>
> >>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >>>> ---
> >>>>  arch/sandbox/cpu/os.c              | 15 ++++++++++++---
> >>>>  drivers/serial/sandbox.c           | 11 ++++++++---
> >>>>  include/os.h                       | 14 ++++++++++++++
> >>>>  test/py/tests/test_sandbox_exit.py | 10 +++++++++-
> >>>>  4 files changed, 43 insertions(+), 7 deletions(-)
> >>>
> >>> I'm sorry but I still do want Ctrl-C to work. How about adding a short
> >>> option to change the terminal mode?
> >>>
> >>> Regards,
> >>> Simon
> >>
> >>
> >> On QEMU you quit via <ctrl+A>X. Can we use the same for the sandbox?
> >
> > I just don't like programs that refused to quit when asked I use> Ctrl-C all the time. Can you not use the -t options to get what you
> > want? Perhaps we could have an environment variable too?
>
> Sorry, I was not aware of the -t option.
>
> ./u-boot -t raw -d arch/sandbox/dts/test.dtb
>
> behaves according to my preferences.

OK great! If you want an environment variable we could add that too.

Regards,
Simon
diff mbox series

Patch

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index e7ec892bdf..b5cfa38a32 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -161,6 +161,7 @@  err:
 static struct termios orig_term;
 static bool term_setup;
 static bool term_nonblock;
+static bool pending_ctrlc;

 void os_fd_restore(void)
 {
@@ -176,11 +177,19 @@  void os_fd_restore(void)
 	}
 }

+void os_clear_ctrlc(void)
+{
+	pending_ctrlc = false;
+}
+
+bool os_get_ctrlc(void)
+{
+	return pending_ctrlc;
+}
+
 static void os_sigint_handler(int sig)
 {
-	os_fd_restore();
-	signal(SIGINT, SIG_DFL);
-	raise(SIGINT);
+	pending_ctrlc = true;
 }

 /* Put tty into raw mode so <tab> and <ctrl+c> work */
diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c
index f09d291e04..e510d51e27 100644
--- a/drivers/serial/sandbox.c
+++ b/drivers/serial/sandbox.c
@@ -72,9 +72,6 @@  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;
 }

@@ -118,6 +115,9 @@  static int sandbox_serial_pending(struct udevice *dev, bool input)
 		increment_buffer_index(serial_buf_write);
 	ssize_t count;

+	if (os_get_ctrlc())
+		return true;
+
 	if (!input)
 		return 0;

@@ -139,6 +139,11 @@  static int sandbox_serial_getc(struct udevice *dev)
 {
 	int result;

+	if (os_get_ctrlc()) {
+		os_clear_ctrlc();
+		return 0x03;
+	}
+
 	if (!sandbox_serial_pending(dev, true))
 		return -EAGAIN;	/* buffer empty */

diff --git a/include/os.h b/include/os.h
index 1874ae674f..0cbf737d57 100644
--- a/include/os.h
+++ b/include/os.h
@@ -355,4 +355,18 @@  int os_read_file(const char *name, void **bufp, int *sizep);
  */
 void *os_find_text_base(void);

+/**
+ * os_get_ctrlc() - check if a CTRL+C has occurred
+ *
+ * Return:	true if CTRL+C has occurred
+ */
+bool os_get_ctrlc(void);
+
+/**
+ * os_clear_ctrlc() - clear CTRL+C
+ *
+ * Mark a previous CTRL+C event as handled.
+ */
+void os_clear_ctrlc(void);
+
 #endif
diff --git a/test/py/tests/test_sandbox_exit.py b/test/py/tests/test_sandbox_exit.py
index a301f4b559..4bc703ea56 100644
--- a/test/py/tests/test_sandbox_exit.py
+++ b/test/py/tests/test_sandbox_exit.py
@@ -15,7 +15,15 @@  def test_reset(u_boot_console):

 @pytest.mark.boardspec('sandbox')
 def test_ctrl_c(u_boot_console):
-    """Test that sending SIGINT to sandbox causes it to exit."""
+    """Test that SIGINT is handled as CTRL-C."""

     u_boot_console.kill(signal.SIGINT)
+    u_boot_console.wait_for('<INTERRUPT>')
+    u_boot_console.run_command('echo')
+
+    u_boot_console.run_command('\x03', wait_for_echo=False, wait_for_prompt=False)
+    u_boot_console.wait_for('<INTERRUPT>')
+    u_boot_console.run_command('echo')
+
+    u_boot_console.kill(signal.SIGKILL)
     assert(u_boot_console.validate_exited())