mirror of
https://sourceware.org/git/binutils-gdb.git
synced 2024-12-09 04:21:49 +08:00
Forbid "set history size (INT_MAX..UINT_MAX)"
The whole readline interface is signed, and works with the 0..INT_MAX range. We don't allow setting the size to UINT_MAX directly. The documented user visible interface is "use 0 for unlimited". The UINT_MAX representation is an implementation detail we could change, e.g., by keeping a separate flag for "unlimited", which is actually what the readline interface does (stifled vs non stifled). Generically speaking, exposing this detail to clients of the interface may make our lives complicated when we find the need to extend the range of some command in the future, and it's better if users (frontends/scripts) aren't relying on anything but what we tell them to use for "unlimited". Making values other than 0 error out is the way to prevent users from using those ranges inappropriately. Quite related, note: (gdb) set history size 0xffffffff integer 4294967295 out of range But, (gdb) set history size 0xfffffffe (gdb) show history size The size of the command history is unlimited. (gdb) set history size 0x100000000 integer 4294967296 out of range If values over INT_MAX are accepted as unlimited, then there's no good argument for only accepting [INT_MAX..UINT_MAX) as valid "unlimited" magic numbers, while not accepting [UINT_MAX..inf). Making the setting's control variable of different type (unsigned int) of the rest of the related code (int) adds the need to recall that one variable among all these is unsigned, and that one need to think about whether these comparisons are signed or unsigned, along with the promotion/conversion rules. Since this is an easy to forget detail, this patch renames the variable to at least make it more obvious that this variable is not one of GNU history's public int variables, which are all signed. We don't actually need the only code that presently is affected by this, though, the code that is computing the current history's length. We can just use GNU history's history_length instead: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Variable: int history_length The number of entries currently stored in the history list. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /* Return the history entry which is logically at OFFSET in the history array. OFFSET is relative to history_base. */ HIST_ENTRY * history_get (offset) int offset; { int local_index; local_index = offset - history_base; return (local_index >= history_length || local_index < 0 || the_history == 0) ? (HIST_ENTRY *)NULL : the_history[local_index]; } ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ At the time this code was added (gdb 4.13 ~1994), 'history_length' was extern, but not documented in readline's GNU history documents, so I guess it wasn't considered public then and the loop was the workaround. One of the warts of GDB choosing 0 to mean unlimited is that "set history size 0" behaves differently from 'HISTSIZE=0 gdb'. The latter leaves GDB with no history, while the former means "unlimited"... $ HISTSIZE=0 ./gdb ... (gdb) show history size The size of the command history is 0. We shouldn't really change what HISTSIZE=0 means, as bash, etc. also handle 0 as real zero, and zero it's what really makes sense. gdb/ 2013-03-27 Pedro Alves <palves@redhat.com> * top.c (history_size): Rename to ... (history_size_setshow_var): ... this. Add comment. (show_commands): Use readline's 'history_length' instead of computing the history length by calling history_get in a loop. (set_history_size_command): Error out for sizes over INT_MAX. Restore previous history size on invalid size. (init_history): If HISTSIZE is negative, leave the history size as zero. Add comments. (init_main): Adjust.
This commit is contained in:
parent
7e93ea4b52
commit
840a9a1f86
@ -1,3 +1,15 @@
|
||||
2013-03-27 Pedro Alves <palves@redhat.com>
|
||||
|
||||
* top.c (history_size): Rename to ...
|
||||
(history_size_setshow_var): ... this. Add comment.
|
||||
(show_commands): Use readline's 'history_length' instead of
|
||||
computing the history length by calling history_get in a loop.
|
||||
(set_history_size_command): Error out for sizes over INT_MAX.
|
||||
Restore previous history size on invalid size.
|
||||
(init_history): If HISTSIZE is negative, leave the history size as
|
||||
zero. Add comments.
|
||||
(init_main): Adjust.
|
||||
|
||||
2013-03-27 Pedro Alves <palves@redhat.com>
|
||||
|
||||
* coff-pe-read.c (_initialize_coff_pe_read): Rename "set debug
|
||||
|
83
gdb/top.c
83
gdb/top.c
@ -711,7 +711,10 @@ show_write_history_p (struct ui_file *file, int from_tty,
|
||||
value);
|
||||
}
|
||||
|
||||
static unsigned int history_size;
|
||||
/* The variable associated with the "set/show history size"
|
||||
command. */
|
||||
static unsigned int history_size_setshow_var;
|
||||
|
||||
static void
|
||||
show_history_size (struct ui_file *file, int from_tty,
|
||||
struct cmd_list_element *c, const char *value)
|
||||
@ -1374,21 +1377,7 @@ show_commands (char *args, int from_tty)
|
||||
Relative to history_base. */
|
||||
static int num = 0;
|
||||
|
||||
/* The first command in the history which doesn't exist (i.e. one more
|
||||
than the number of the last command). Relative to history_base. */
|
||||
unsigned int hist_len;
|
||||
|
||||
/* Print out some of the commands from the command history. */
|
||||
/* First determine the length of the history list. */
|
||||
hist_len = history_size;
|
||||
for (offset = 0; offset < history_size; offset++)
|
||||
{
|
||||
if (!history_get (history_base + offset))
|
||||
{
|
||||
hist_len = offset;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (args)
|
||||
{
|
||||
@ -1402,7 +1391,7 @@ show_commands (char *args, int from_tty)
|
||||
/* "show commands" means print the last Hist_print commands. */
|
||||
else
|
||||
{
|
||||
num = hist_len - Hist_print;
|
||||
num = history_length - Hist_print;
|
||||
}
|
||||
|
||||
if (num < 0)
|
||||
@ -1410,14 +1399,16 @@ show_commands (char *args, int from_tty)
|
||||
|
||||
/* If there are at least Hist_print commands, we want to display the last
|
||||
Hist_print rather than, say, the last 6. */
|
||||
if (hist_len - num < Hist_print)
|
||||
if (history_length - num < Hist_print)
|
||||
{
|
||||
num = hist_len - Hist_print;
|
||||
num = history_length - Hist_print;
|
||||
if (num < 0)
|
||||
num = 0;
|
||||
}
|
||||
|
||||
for (offset = num; offset < num + Hist_print && offset < hist_len; offset++)
|
||||
for (offset = num;
|
||||
offset < num + Hist_print && offset < history_length;
|
||||
offset++)
|
||||
{
|
||||
printf_filtered ("%5d %s\n", history_base + offset,
|
||||
(history_get (history_base + offset))->line);
|
||||
@ -1441,16 +1432,30 @@ show_commands (char *args, int from_tty)
|
||||
static void
|
||||
set_history_size_command (char *args, int from_tty, struct cmd_list_element *c)
|
||||
{
|
||||
/* The type of parameter in stifle_history is int, so values from INT_MAX up
|
||||
mean 'unlimited'. */
|
||||
if (history_size >= INT_MAX)
|
||||
/* Readline's history interface works with 'int', so it can only
|
||||
handle history sizes up to INT_MAX. The command itself is
|
||||
uinteger, so UINT_MAX means "unlimited", but we only get that if
|
||||
the user does "set history size 0" -- "set history size <UINT_MAX>"
|
||||
throws out-of-range. */
|
||||
if (history_size_setshow_var > INT_MAX
|
||||
&& history_size_setshow_var != UINT_MAX)
|
||||
{
|
||||
/* Ensure that 'show history size' prints 'unlimited'. */
|
||||
history_size = UINT_MAX;
|
||||
unstifle_history ();
|
||||
unsigned int new_value = history_size_setshow_var;
|
||||
|
||||
/* Restore previous value before throwing. */
|
||||
if (history_is_stifled ())
|
||||
history_size_setshow_var = history_max_entries;
|
||||
else
|
||||
history_size_setshow_var = UINT_MAX;
|
||||
|
||||
error (_("integer %u out of range"), new_value);
|
||||
}
|
||||
|
||||
/* Commit the new value to readline's history. */
|
||||
if (history_size_setshow_var == UINT_MAX)
|
||||
unstifle_history ();
|
||||
else
|
||||
stifle_history (history_size);
|
||||
stifle_history (history_size_setshow_var);
|
||||
}
|
||||
|
||||
void
|
||||
@ -1503,11 +1508,27 @@ init_history (void)
|
||||
|
||||
tmpenv = getenv ("HISTSIZE");
|
||||
if (tmpenv)
|
||||
history_size = atoi (tmpenv);
|
||||
else if (!history_size)
|
||||
history_size = 256;
|
||||
{
|
||||
int var;
|
||||
|
||||
stifle_history (history_size);
|
||||
var = atoi (tmpenv);
|
||||
if (var < 0)
|
||||
{
|
||||
/* Prefer ending up with no history rather than overflowing
|
||||
readline's history interface, which uses signed 'int'
|
||||
everywhere. */
|
||||
var = 0;
|
||||
}
|
||||
|
||||
history_size_setshow_var = var;
|
||||
}
|
||||
/* If the init file hasn't set a size yet, pick the default. */
|
||||
else if (history_size_setshow_var == 0)
|
||||
history_size_setshow_var = 256;
|
||||
|
||||
/* Note that unlike "set history size 0", "HISTSIZE=0" really sets
|
||||
the history size to 0... */
|
||||
stifle_history (history_size_setshow_var);
|
||||
|
||||
tmpenv = getenv ("GDBHISTFILE");
|
||||
if (tmpenv)
|
||||
@ -1630,7 +1651,7 @@ Without an argument, saving is enabled."),
|
||||
show_write_history_p,
|
||||
&sethistlist, &showhistlist);
|
||||
|
||||
add_setshow_uinteger_cmd ("size", no_class, &history_size, _("\
|
||||
add_setshow_uinteger_cmd ("size", no_class, &history_size_setshow_var, _("\
|
||||
Set the size of the command history,"), _("\
|
||||
Show the size of the command history,"), _("\
|
||||
ie. the number of previous commands to keep a record of."),
|
||||
|
Loading…
Reference in New Issue
Block a user