Greetings:

After sending the original email concerning this issue, I delved into
the code again for one last review.  I discovered that, even after the
original fix a segmentation fault is still possible, because histbeg
could be higher than histend!  The idea from "fc" would be to print the
list in reverse.

However, histbeg and histend are swapped after histend is checked in the
original patch.  So, I have attached an updated patch that avoids the
following additional segfault:

  $ bash --norc
  $ fc -0 100
  Segmentation fault (core dumped)

This patch works by moving the reversal of histbeg and histend to before
when histend is checked.  This handles the other case when a segfault
could occur (where histbeg is real_last).

Thanks again for reviewing!

-- 
Jason Franklin
diff --git a/builtins/fc.def b/builtins/fc.def
index 6951a687..04361b92 100644
--- a/builtins/fc.def
+++ b/builtins/fc.def
@@ -353,9 +353,20 @@ fc_builtin (list)
 	histbeg = histend = last_hist;
     }
 
+  if (histend < histbeg)
+    {
+      i = histend;
+      histend = histbeg;
+      histbeg = i;
+
+      reverse = 1;
+    }
+
   /* "When not listing, the fc command that caused the editing shall not be
-     entered into the history list." */
-  if (listing == 0 && hist_last_line_added)
+     entered into the history list."  However, if the user passed "-0", then
+     histend will have been set to real_last above.  This means the user wants
+     to include the current command, so we do not remove it here. */
+  if (listing == 0 && hist_last_line_added && histend < real_last)
     {
       bash_delete_last_history ();
       /* If we're editing a single command -- the last command in the
@@ -382,15 +393,6 @@ fc_builtin (list)
       return (EXECUTION_FAILURE);
     }
 
-  if (histend < histbeg)
-    {
-      i = histend;
-      histend = histbeg;
-      histbeg = i;
-
-      reverse = 1;
-    }
-
   if (listing)
     stream = stdout;
   else

Reply via email to