Re: read builtin breaks autocompletion

[Philip Rowlands]

In builtins/read.def (from bash-3.0.tar.gz):

  677   old_attempted_completion_function = rl_attempted_completion_function;
  678   rl_attempted_completion_function = (rl_completion_func_t *)NULL;
  679   ret = readline (p);
  680   rl_attempted_completion_function = old_attempted_completion_function;


I suspect that, in the case of timeout, the old function is never 
restored, and remains NULL. (I'm puzzled that this isn't repeatable, 
unless readline is completely disabled.)


I hope this makes sense to somebody, because the signal handling code 
and add_unwind_protect looks... scary, and out of my depth to patch.



Phil


___
Bug-bash mailing list
Bug-bash@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-bash

[patch] unsafe signal handler

[Tim Waugh]

Hi,

Here is a patch which aims to address the problem reported earlier:

  http://lists.gnu.org/archive/html/bug-bash/2005-07/msg00129.html

by making the signal handler set a flag which is then checked at
strategic points in the main flow of code.

What do you think?

Tim.
*/

--- bash-3.0/sig.c.sighandler   2005-12-06 16:55:26.0 +
+++ bash-3.0/sig.c  2005-12-06 17:14:29.0 +
@@ -58,6 +58,8 @@
 extern int loop_level, continuing, breaking;
 extern int parse_and_execute_level, shell_initialized;
 
+int need_termination_unwind_protect = 0;
+
 /* Non-zero after SIGINT. */
 int interrupt_state;
 
@@ -408,6 +410,13 @@
 termination_unwind_protect (sig)
  int sig;
 {
+  need_termination_unwind_protect = sig;
+  SIGRETURN (0);
+}
+
+void
+do_termination_unwind_protect (int sig)
+{
   if (sig == SIGINT && signal_is_trapped (SIGINT))
 run_interrupt_trap ();
 
@@ -429,8 +438,6 @@
   run_exit_trap ();
   set_signal_handler (sig, SIG_DFL);
   kill (getpid (), sig);
-
-  SIGRETURN (0);
 }
 
 /* What we really do when SIGINT occurs. */
--- bash-3.0/sig.h.sighandler   2005-12-06 17:00:22.0 +
+++ bash-3.0/sig.h  2005-12-06 17:17:32.0 +
@@ -109,8 +109,12 @@
 
 #endif /* JOB_CONTROL */
 
+/* Global variables from sig.c */
+extern int need_termination_unwind_protect;
+
 /* Functions from sig.c. */
 extern sighandler termination_unwind_protect __P((int));
+extern void do_termination_unwind_protect __P((int));
 extern sighandler sigint_sighandler __P((int));
 extern void initialize_signals __P((int));
 extern void initialize_terminating_signals __P((void));
@@ -123,4 +127,11 @@
 extern SigHandler *trap_to_sighandler __P((int));
 extern sighandler trap_handler __P((int));
 
+#define CATCH_SIGNALS()
\
+  do   \
+{  \
+  if (need_termination_unwind_protect) \
+   do_termination_unwind_protect (need_termination_unwind_protect); \
+} while (0)
+   
 #endif /* _SIG_H_ */
--- bash-3.0/input.c.sighandler 2005-12-06 17:19:59.0 +
+++ bash-3.0/input.c2005-12-06 17:27:04.0 +
@@ -41,6 +41,7 @@
 #include "input.h"
 #include "error.h"
 #include "externs.h"
+#include "sig.h"
 
 #if !defined (errno)
 extern int errno;
@@ -66,6 +67,7 @@
 {
   while (1)
{
+ CATCH_SIGNALS ();
  local_bufused = read (fileno (stream), localbuf, sizeof(localbuf));
  if (local_bufused > 0)
break;
--- bash-3.0/jobs.c.sighandler  2005-12-06 17:22:46.0 +
+++ bash-3.0/jobs.c 2005-12-06 17:26:06.0 +
@@ -2513,6 +2513,7 @@
 retry:
   if (wcontinued_not_supported)
waitpid_flags &= ~WCONTINUED;
+  CATCH_SIGNALS ();
   pid = WAITPID (-1, &status, waitpid_flags);
   if (pid == -1 && errno == EINVAL)
{
@@ -2537,6 +2538,7 @@
 
   /* If waitpid returns 0, there are running children.  If it returns -1,
 the only other error POSIX says it can return is EINTR. */
+  CATCH_SIGNALS ();
   if (pid <= 0)
continue;   /* jumps right to the test */
 
--- bash-3.0/execute_cmd.c.sighandler   2005-12-07 11:26:31.0 +
+++ bash-3.0/execute_cmd.c  2005-12-07 11:27:28.0 +
@@ -360,6 +360,7 @@
 unlink_fifo_list ();
 #endif /* PROCESS_SUBSTITUTION */
 
+  CATCH_SIGNALS ();
   return (result);
 }
 


___
Bug-bash mailing list
Bug-bash@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-bash

[patch] memory leak in read builtin

[Tim Waugh]

There is at least one memory leak in the read builtin in bash-3.0.  To
demonstrate it, try this test case:

  https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=173283#c10

There is a link to a file, nonblock.c, which you should compile like
'make nonblock'.  Then run the short script to see the problem.

The problem comes about when stdin has O_NONBLOCK set.  Here is the
fix:

--- bash-3.0/builtins/read.def.read-memleak 2005-12-07 17:45:38.0 
+
+++ bash-3.0/builtins/read.def  2005-12-07 17:45:39.0 +
@@ -461,6 +461,7 @@
   if (retval < 0)
 {
   builtin_error (_("read error: %d: %s"), fd, strerror (errno));
+  run_unwind_frame ("read_builtin");
   return (EXECUTION_FAILURE);
 }
 #endif

There is another suspicious place in that function where an xfree is
notable by its absence:

--- bash-3.0/builtins/read.def.read-memleak 2005-12-07 17:45:38.0 
+
+++ bash-3.0/builtins/read.def  2005-12-07 18:00:26.0 +
@@ -508,7 +508,10 @@
 
   var = find_or_make_array_variable (arrayname, 1);
   if (var == 0)
-   return EXECUTION_FAILURE;   /* readonly or noassign */
+   {
+ xfree (input_string);
+ return EXECUTION_FAILURE; /* readonly or noassign */
+   }
   array_flush (array_cell (var));
 
   alist = list_string (input_string, ifs_chars, 0);

..but I couldn't work out how to demonstrate this particular leak, if
that's what it is.

Tim.
*/


___
Bug-bash mailing list
Bug-bash@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-bash