This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix PR gdb/16999


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]