This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix PR gdb/16999
- From: Pedro Alves <palves at redhat dot com>
- To: Patrick Palka <patrick at parcs dot ath dot cx>, gdb-patches at sourceware dot org
- Date: Thu, 14 May 2015 11:16:17 +0100
- Subject: Re: [PATCH] Fix PR gdb/16999
- Authentication-results: sourceware.org; auth=none
- References: <1431555450-15493-1-git-send-email-patrick at parcs dot ath dot cx>
On 05/13/2015 11:17 PM, Patrick Palka wrote:
> When GDB reads a nonsensical value for the HISTSIZE environment variable
> variable, i.e. one that is non-numeric or negative, GDB then sets its
> history size to 0. This behavior is contrary to that of bash, which
> defaults the history size to unlimited in such cases.
>
> This patch makes the behavior of invalid HISTSIZE match that of bash.
> When we encounter an invalid HISTSIZE we now set the history size to
> unlimited instead of 0.
>
> gdb/ChangeLog:
>
> PR gdb/16999
> * top.c (init_history): For invalid HISTSIZE, set history size
> to unlimited.
>
> gdb/testsuite/ChangeLog:
>
> PR gdb/16999
> * gdb.base/histsize-history.exp: New test.
> ---
> gdb/testsuite/gdb.base/histsize-history.exp | 54 +++++++++++++++++++++++++++++
> gdb/top.c | 16 ++++-----
> 2 files changed, 62 insertions(+), 8 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/histsize-history.exp
>
> diff --git a/gdb/testsuite/gdb.base/histsize-history.exp b/gdb/testsuite/gdb.base/histsize-history.exp
> new file mode 100644
> index 0000000..b7b13cf
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/histsize-history.exp
> @@ -0,0 +1,54 @@
> +# Copyright 2015 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +# This file is part of the gdb testsuite.
> +
> +# Test the setting of "history size" via the HISTSIZE environment variable
> +
> +
> +# Check that the history size is properly set to SIZE when env(HISTSIZE) is set
> +# to HISTSIZE.
> +
> +proc test_histsize_history_setting { histsize size } {
> + global env
> +
> + if [info exists env(HISTSIZE)] {
> + set old_histsize $env(HISTSIZE)
> + }
> + set env(HISTSIZE) $histsize
> +
> + gdb_exit
> + gdb_start
> +
> + gdb_test "show history size" "The size of the command history is $size."
> +
> + if { $size == "0" } {
> + gdb_test_no_output "show commands"
> + } elseif { $size != "1" } {
> + gdb_test "show commands" " . show history size\r\n . show commands"
> + }
> +
> + if [info exists old_histsize] {
> + set env(HISTSIZE) $old_histsize
> + } else {
> + unset env(HISTSIZE)
> + }
> +}
> +
> +test_histsize_history_setting "0" "0"
> +test_histsize_history_setting "20" "20"
> +test_histsize_history_setting "-5" "unlimited"
> +test_histsize_history_setting "not_an_integer" "unlimited"
> +test_histsize_history_setting "10zab" "unlimited"
> diff --git a/gdb/top.c b/gdb/top.c
> index 74e1e07..00fee8d 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -1684,15 +1684,15 @@ init_history (void)
> if (tmpenv)
> {
> int var;
> + char *endptr;
>
> - 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;
> - }
> + var = strtol (tmpenv, &endptr, 10);
> +
> + /* For the sake of consistency with bash, if HISTSIZE is
> + non-numeric or if HISTSIZE is negative then set our history
> + size to unlimited. */
> + if (*endptr != '\0' || var < 0)
> + var = -1;
Hmm, looking at the master branch (Bash-4.3 patch 33) in:
http://git.savannah.gnu.org/cgit/bash.git
This seems to be where bash implements this:
#if defined (HISTORY)
/* What to do after the HISTSIZE or HISTFILESIZE variables change.
If there is a value for this HISTSIZE (and it is numeric), then stifle
the history. Otherwise, if there is NO value for this variable,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
unstifle the history. If name is HISTFILESIZE, and its value is
^^^^^^^^^^^^^^^^^^^^
numeric, truncate the history file to hold no more than that many
lines. */
void
sv_histsize (name)
char *name;
{
char *temp;
intmax_t num;
int hmax;
temp = get_string_value (name);
if (temp && *temp)
{
if (legal_number (temp, &num))
{
hmax = num;
if (hmax < 0 && name[4] == 'S')
unstifle_history (); /* unstifle history if HISTSIZE < 0 */
else if (name[4] == 'S')
{
stifle_history (hmax);
hmax = where_history ();
if (history_lines_this_session > hmax)
history_lines_this_session = hmax;
}
else if (hmax >= 0) /* truncate HISTFILE if HISTFILESIZE >= 0 */
{
history_truncate_file (get_string_value ("HISTFILE"), hmax);
if (hmax <= history_lines_in_file)
history_lines_in_file = hmax;
}
}
}
else if (name[4] == 'S')
unstifle_history ();
}
Note the comment I underlined above.
If I'm reading right, stripping out all the HISTFILESIZE handling
and then removing dead code, we get:
void
sv_histsize (char * name)
{
char *temp;
intmax_t num;
int hmax;
temp = get_string_value (name);
if (temp && *temp)
{
if (legal_number (temp, &num))
{
hmax = num;
if (hmax < 0)
unstifle_history (); /* unstifle history if HISTSIZE < 0 */
else
stifle_history (hmax);
}
}
else
unstifle_history ();
}
Note:
#1 - if HISTSIZE is non-numeric nothing happens. I think that means
bash ends up with the default history size.
#2 - if HISTSIZE is set to the empty string, bash ends up
with unlimited history size.
It seems to me that your patch handles both of these differently.
#2 appears consistent with what is suggested here:
http://stackoverflow.com/questions/9457233/unlimited-bash-history
"Set HISTSIZE and HISTFILESIZE to an empty string:
HISTSIZE= HISTFILESIZE=
In bash 4.3 and later you can also use HISTSIZE=-1 HISTFILESIZE=-1:
"
Thanks,
Pedro Alves