Re: Use of rl_echo_signal_char in _rl_handle_signal

[Andrew Burgess]

Chet Ramey <chet.ramey@case.edu> writes:

> On 6/7/23 8:52 AM, Andrew Burgess wrote:
>
>> I did some experimentation at my end and updated my sample program to
>> make use of rl_persistent_signal_handlers.  I agree with your first
>> suggestion, having the rl_echo_signal_char call conditional on
>> rl_persistent_signal_handlers (when in CALLBACK mode) seems to allow for
>> both options.
>
> OK, this is a good start. It's missing several cases, so let's see if we
> can tighten it up.
>
>
>>      In callback mode when rl_persistent_signal_handlers is on then the
>>      readline signal handler is always in place for the duration of the
>>      readline prompt being in use.  In this case it does make sense to
>>      always call rl_echo_signal_char.  By the time the application's signal
>>      handler is invoked we are on the way out of readline, and it is no
>>      longer the applications job to call rl_echo_signal_char.
>>      
>>      In non-callback mode, this patch changes nothing, rl_echo_signal_char
>>      is always called.
>
> These paragraphs are not correct. It's common practice to install a
> readline library that was compiled with READLINE_CALLBACKS defined as the
> standard readline library. I'm pretty sure Red Hat does it, for example.
> The idea is that callbacks won't be active unless the application calls
> rl_callback_handler_install().
>
> This patch makes the call to rl_echo_signal_char() dependent on the setting
> of rl_persistent_signal_handlers, which an application *not* using callback
> mode will never set.

Thanks for your patience.  I think I'm getting there slowly :)

>
> That test needs to be augmented with a condition that won't be true unless
> callback mode is actually active. You should be able to test
> RL_STATE_CALLBACK using RL_ISSTATE(), but you might also want to test
> whether rl_linefunc is non-null. Let me know what works for you.

Looking at the code I think that RL_ISSTATE (RL_STATE_CALLBACK) will be
true only when rl_linefunc != NULL and vice versa, so I think just
checking one of these will be fine.

The patch below calls rl_echo_signal_char when RL_STATE_CALLBACK is not
set, or when rl_persistent_signal_handlers is true, which I think covers
all the cases.

If READLINE_CALLBACKS is not even defined at compile time then
rl_echo_signal_char is always called, just like before.

This still seems to fix the problem GDB was seeing.

Thanks,
Andrew


---

commit 68fc0cf84b8a434675a22612d803d38e39c40591
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Jun 1 11:21:04 2023 +0100

    readline: don't always call rl_echo_signal_char in callback mode
    
    This patch was a result of some GDB work that is discussed in this
    thread:
    
      
https://inbox.sourceware.org/gdb-patches/20230518061046.17837-1-tdevries@suse.de/
    
    In callback mode, and if rl_persistent_signal_handlers is off, then
    most signals will be delivered directly to the applications signal
    handler.  If the application wants to see the signal char then the
    application will take care of calling rl_echo_signal_char.
    
    However, readline currently always calls rl_echo_signal_char for
    signals seen within readline, this means that if a signal happens to
    arrive at the right/wrong moment then rl_echo_signal_char will always
    be called, whether the application calls this itself or not.
    
    In callback mode when rl_persistent_signal_handlers is on then the
    readline signal handler is always in place for the duration of the
    readline prompt being in use.  In this case it does make sense to
    always call rl_echo_signal_char.  By the time the application's signal
    handler is invoked we are on the way out of readline, and it is no
    longer the applications job to call rl_echo_signal_char.
    
    When callback mode is off, this patch changes nothing,
    rl_echo_signal_char is always called.

diff --git a/signals.c b/signals.c
index ec835e5..16612c5 100644
--- a/signals.c
+++ b/signals.c
@@ -267,7 +267,11 @@ _rl_handle_signal (int sig)
        sigprocmask (SIG_BLOCK, &set, &oset);
 #endif
 
-      rl_echo_signal_char (sig);
+#if defined (READLINE_CALLBACKS)
+      if (!RL_ISSTATE (RL_STATE_CALLBACK) || rl_persistent_signal_handlers)
+#endif
+       rl_echo_signal_char (sig);
+
       rl_cleanup_after_signal ();
 
       /* At this point, the application's signal handler, if any, is the