public inbox for dash@vger.kernel.org
 help / color / mirror / Atom feed
* Incorrect detection of EOF with interactive editline
@ 2025-09-18  0:33 Abdullah
  2025-09-18  0:33 ` [PATCH] histedit: fix dash exiting when receiving a signal interactively Abdullah
  0 siblings, 1 reply; 4+ messages in thread
From: Abdullah @ 2025-09-18  0:33 UTC (permalink / raw)
  To: dash

When an interactive shell (with editline) has a trap signal set
(e.g., [1]) and the signal is caught AND there's nothing on the input
line, the shell (or at least editline), after executing the trap
handler, treats the empty line as EOF.

Try [1] with interactive editline, have a clean line input line,
and change the size of your term, then your shell would die.

Also, we should call el_resize on WINCH, because we have not set
the EL_SIGNAL option (and we shouldn't set EL_SIGNAL, that would mixup
with trap handlers).


[1] $ trap 'echo "WINDOW CHANGED! New size:"; stty size' WINCH

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] histedit: fix dash exiting when receiving a signal interactively
  2025-09-18  0:33 Incorrect detection of EOF with interactive editline Abdullah
@ 2025-09-18  0:33 ` Abdullah
  2025-09-18  3:20   ` Herbert Xu
  0 siblings, 1 reply; 4+ messages in thread
From: Abdullah @ 2025-09-18  0:33 UTC (permalink / raw)
  To: dash; +Cc: Abdullah

---
 src/input.c | 32 ++++++++++++++++++++++++++++----
 src/trap.c  | 16 +++++++++++++++-
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/src/input.c b/src/input.c
index c36d120..f775c9f 100644
--- a/src/input.c
+++ b/src/input.c
@@ -39,6 +39,7 @@
 #include <string.h>
 #include <termios.h>
 #include <unistd.h>
+#include <wchar.h>
 
 /*
  * This file implements the input routines used by the parser.
@@ -276,6 +277,11 @@ preadfd(void)
 	int unget;
 	int pnr;
 	int nr;
+#ifndef SMALL
+	const LineInfoW *liw;
+	wchar_t wbuf[2];
+	const wchar_t *wc;
+#endif
 
 	nr = input_get_lleft(parsefile);
 
@@ -306,9 +312,8 @@ retry:
 			rl_cp = el_gets(el, &el_len);
 			popstackmark(&smark);
 		}
-		if (rl_cp == NULL)
-			nr = 0;
-		else {
+
+		if (rl_cp) {
 			if (nr > el_len)
 				nr = el_len;
 			memcpy(buf, rl_cp, nr);
@@ -317,9 +322,28 @@ retry:
 				rl_cp += nr;
 			} else
 				rl_cp = 0;
+			return nr;
 		}
 
-		return nr;
+		/* Got a signal while reading. */
+		if (el_len < 0 && errno == EINTR && pending_sig) {
+			out2c('\r');
+			liw = el_wline(el);
+			/* Don't lose the current line.
+			 * el_wpush() takes a L'\0' terminated string.
+			 * el_wline() gives us a string with N elements.
+			 * Simple solution: el_wpush() it wchar-by-wchar.
+			 */
+			wbuf[1] = L'\0';
+			wc = liw->buffer;
+			while (wc < liw->lastchar) {
+				wbuf[0] = *wc++;
+				el_wpush(el, wbuf);
+			}
+			dotrap();
+			goto retry;
+		}
+		return 0;
 	}
 #endif
 
diff --git a/src/trap.c b/src/trap.c
index 23829a5..c045257 100644
--- a/src/trap.c
+++ b/src/trap.c
@@ -53,6 +53,7 @@
 #include "error.h"
 #include "trap.h"
 #include "mystring.h"
+#include "myhistedit.h"
 
 /*
  * Sigmode records the current value of the signal handlers for the various
@@ -243,7 +244,12 @@ setsignal(int signo)
 		}
 	}
 
-	if (signo == SIGCHLD)
+	if (
+		signo == SIGCHLD
+#if defined(WINCH) && !defined(SMALL)
+		|| signo == WINCH
+#endif
+	)
 		action = S_CATCH;
 
 	t = &sigmode[signo - 1];
@@ -321,6 +327,11 @@ onsig(int signo)
 			return;
 	}
 
+#if defined(SIGWINCH) && !defined(SMALL)
+	if (signo == SIGWINCH && el)
+		el_resize(el);
+#endif
+
 	gotsig[signo - 1] = 1;
 	pending_sig = signo;
 
@@ -397,6 +408,9 @@ setinteractive(int on)
 	setsignal(SIGINT);
 	setsignal(SIGQUIT);
 	setsignal(SIGTERM);
+#if defined(WINCH) && !defined(SMALL)
+	setsignal(WINCH);
+#endif
 }
 
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] histedit: fix dash exiting when receiving a signal interactively
  2025-09-18  0:33 ` [PATCH] histedit: fix dash exiting when receiving a signal interactively Abdullah
@ 2025-09-18  3:20   ` Herbert Xu
       [not found]     ` <DCW6NI4KVK3I.1ALXRKL5V6UCM@gmail.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Herbert Xu @ 2025-09-18  3:20 UTC (permalink / raw)
  To: Abdullah; +Cc: dash

Abdullah <3bd3bdr@gmail.com> wrote:
>
> +               /* Got a signal while reading. */
> +               if (el_len < 0 && errno == EINTR && pending_sig) {
> +                       out2c('\r');
> +                       liw = el_wline(el);
> +                       /* Don't lose the current line.
> +                        * el_wpush() takes a L'\0' terminated string.
> +                        * el_wline() gives us a string with N elements.
> +                        * Simple solution: el_wpush() it wchar-by-wchar.
> +                        */
> +                       wbuf[1] = L'\0';
> +                       wc = liw->buffer;
> +                       while (wc < liw->lastchar) {
> +                               wbuf[0] = *wc++;
> +                               el_wpush(el, wbuf);
> +                       }
> +                       dotrap();
> +                       goto retry;
> +               }
> +               return 0;

Can you please explain why this is necessary and why el_gets can't
handle it?
 
> -       if (signo == SIGCHLD)
> +       if (
> +               signo == SIGCHLD
> +#if defined(WINCH) && !defined(SMALL)
> +               || signo == WINCH
> +#endif

Please use SIGWINCH consistently.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] histedit: fix dash exiting when receiving a signal interactively
       [not found]     ` <DCW6NI4KVK3I.1ALXRKL5V6UCM@gmail.com>
@ 2025-09-22  3:43       ` Herbert Xu
  0 siblings, 0 replies; 4+ messages in thread
From: Herbert Xu @ 2025-09-22  3:43 UTC (permalink / raw)
  To: 3bd; +Cc: DASH Mailing List

On Thu, Sep 18, 2025 at 10:45:38PM +0300, 3bd wrote:
>
> EditLine has no "restore the current line to XYZ".
> No el_restore_line() or something similar.
> 
> We could avoid this if we set the SA_RESTART flag in sa_flags,
> then EditLine's internal call to read(2) won't fail.
> But I'm not sure about this. This could play with dash's internal
> handling of signals.

I think that's a defiency in libedit and should be addressed there.

> >> -       if (signo == SIGCHLD)
> >> +       if (
> >> +               signo == SIGCHLD
> >> +#if defined(WINCH) && !defined(SMALL)
> >> +               || signo == WINCH
> >> +#endif
> >
> > Please use SIGWINCH consistently.
> 
> Oh, sorry, I just noticed the mistake.

I'm happy to take the SIGWINCH change though.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-09-22  3:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-18  0:33 Incorrect detection of EOF with interactive editline Abdullah
2025-09-18  0:33 ` [PATCH] histedit: fix dash exiting when receiving a signal interactively Abdullah
2025-09-18  3:20   ` Herbert Xu
     [not found]     ` <DCW6NI4KVK3I.1ALXRKL5V6UCM@gmail.com>
2025-09-22  3:43       ` Herbert Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox