Bash Version: 5.1
Patch Level: 4
Release Status: release
Git Commit: f3a35a2d (current master)
I don't really know the contribution rules for bash; my apologies in
advance if I'm missing anything or otherwise doing something wrong.
Description:
`history -d -#` can sometimes fail with the error `-bash: history: -#:
history position out of range` even when the history entry is present.
Repeat-By:
(Note that all of the `history` commands below are prefixed by a space
to prevent adding them to history.)
test:~$ HISTCONTROL=ignorespace
test:~$ HISTSIZE=4
test:~$ history -c
test:~$ echo 1 >/dev/null
test:~$ echo 2 >/dev/null
test:~$ echo 3 >/dev/null
test:~$ echo 4 >/dev/null
test:~$ echo 5 >/dev/null
test:~$ echo 6 >/dev/null
test:~$ history
3 echo 3 >/dev/null
4 echo 4 >/dev/null
5 echo 5 >/dev/null
6 echo 6 >/dev/null
test:~$ history -d -1
test:~$ history
3 echo 3 >/dev/null
4 echo 4 >/dev/null
5 echo 5 >/dev/null
test:~$ echo 6 >/dev/null
test:~$ echo 7 >/dev/null
test:~$ history
4 echo 4 >/dev/null
5 echo 5 >/dev/null
6 echo 6 >/dev/null
7 echo 7 >/dev/null
test:~$ history -d -1
-bash: history: -1: history position out of range
Fix:
Attached. In the code below from history.def, after `ind` is first
assigned to, it should be in the range [0, history_length) if it refers
to a valid entry. It's then compared to `history_base`, however it
should be compared to `0` instead, e.g. `if (ind < 0)`.
if (delete_arg[0] == '-' && delete_offset < 0)
{
/* since the_history[history_length] == 0x0, this calculation
means
that history -d -1 will delete the last history entry, which
at
this point is the history -d -1 we just added. */
ind = history_length + delete_offset;
if (ind < history_base)
{
sh_erange (delete_arg, _("history position"));
return (EXECUTION_FAILURE);
}
opt = ind + history_base; /* compensate for opt -
history_base below */
}
After applying the patch, everything seems to work as intended, however
I didn't look into how to write a test for this....
Regards,
Chris Gurnee
diff --git a/builtins/history.def b/builtins/history.def
index 5db44c2c..0ee14cb2 100644
--- a/builtins/history.def
+++ b/builtins/history.def
@@ -189,35 +189,27 @@ history_builtin (list)
return (EXECUTION_FAILURE);
}
if (delete_arg[0] == '-' && delete_start < 0)
- {
- /* the_history[history_length] == 0x0, so this is correct */
- delete_start += history_length;
- if (delete_start < history_base)
- {
-start_error:
- sh_erange (delete_arg, _("history position"));
- return (EXECUTION_FAILURE);
- }
- }
- /* numbers as displayed by display_history are offset by history_base */
+ /* the_history[history_length] == 0x0, so this is correct */
+ delete_start += history_length;
else if (delete_start > 0)
+ /* numbers as displayed by display_history are offset by history_base */
delete_start -= history_base;
if (delete_start < 0 || delete_start >= history_length)
- goto start_error;
+ {
+ sh_erange (delete_arg, _("history position"));
+ return (EXECUTION_FAILURE);
+ }
if (range[0] == '-' && delete_end < 0)
- {
- delete_end += history_length;
- if (delete_end < history_base)
- {
-range_error:
- sh_erange (range, _("history position"));
- return (EXECUTION_FAILURE);
- }
- }
+ {
+ delete_end += history_length;
+ }
else if (delete_end > 0)
delete_end -= history_base;
if (delete_end < 0 || delete_end >= history_length)
- goto range_error;
+ {
+ sh_erange (range, _("history position"));
+ return (EXECUTION_FAILURE);
+ }
result = bash_delete_history_range (delete_start, delete_end);
if (where_history () > history_length)
history_set_pos (history_length);
@@ -237,7 +229,7 @@ range_error:
that history -d -1 will delete the last history entry, which at
this point is the history -d -1 we just added. */
ind = history_length + delete_offset;
- if (ind < history_base)
+ if (ind < 0)
{
sh_erange (delete_arg, _("history position"));
return (EXECUTION_FAILURE);
