Message ID | 1431555450-15493-1-git-send-email-patrick@parcs.ath.cx |
---|---|
State | New, archived |
Headers |
Received: (qmail 71973 invoked by alias); 13 May 2015 22:17:40 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 71963 invoked by uid 89); 13 May 2015 22:17:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: mail-qk0-f179.google.com Received: from mail-qk0-f179.google.com (HELO mail-qk0-f179.google.com) (209.85.220.179) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 13 May 2015 22:17:38 +0000 Received: by qkgy4 with SMTP id y4so38523024qkg.2 for <gdb-patches@sourceware.org>; Wed, 13 May 2015 15:17:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=d4xXWIgWWSS5bVPTih9h0J4V1euLZkmCXYMMSiIA4jk=; b=NhByJcJkYD/vfM98MhNQOy2/MCHLGzufUykSlG08hGvelM2GmdqiFZD5dFehBetSXH wjn724bV+9o0R+sAfryN1IFVkVzZmeNGkko3YpZias8LU7GLkeUG9mauNq/FaeMtX6D3 GQuIQ8rEwdqe5mWvBi9XyufBJMwaE+LdP30VanchACcXDgDh7M1UIu4N+1GUcA85F+v4 rmYtKY/6g3B7MoJUEgsTZbZezcnlpeL8ZmhUUa5qedFPr8aGllSMdXklROGuHq0r5h3z W5RT0P6swVz6X2gtZ2T/uUPF85hW4GNmkPYtO2wl1PgUCNzTZWapjoujs/Hb2xtCeIjs VbMA== X-Gm-Message-State: ALoCoQlrVohBtKY0X3wgPwJw0rnlIhW9AZScXxBC4CAt3du7QY/0BxGdoLZF5JQ3yIRG5dnmIFxq X-Received: by 10.140.144.76 with SMTP id 73mr1604065qhq.28.1431555455943; Wed, 13 May 2015 15:17:35 -0700 (PDT) Received: from localhost.localdomain (ool-4353acd8.dyn.optonline.net. [67.83.172.216]) by mx.google.com with ESMTPSA id c8sm16703530qgf.6.2015.05.13.15.17.35 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 13 May 2015 15:17:35 -0700 (PDT) From: Patrick Palka <patrick@parcs.ath.cx> To: gdb-patches@sourceware.org Cc: Patrick Palka <patrick@parcs.ath.cx> Subject: [PATCH] Fix PR gdb/16999 Date: Wed, 13 May 2015 18:17:30 -0400 Message-Id: <1431555450-15493-1-git-send-email-patrick@parcs.ath.cx> |
Commit Message
Patrick Palka
May 13, 2015, 10:17 p.m. UTC
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
Comments
> > 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. The GDB behaviour makes more sense to me especially in light of: > - /* Prefer ending up with no history rather than overflowing > - readline's history interface, which uses signed 'int' > - everywhere. */
On Wed, May 13, 2015 at 6:24 PM, Mark Kettenis <mark.kettenis@xs4all.nl> 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. > > The GDB behaviour makes more sense to me especially in light of: > >> - /* Prefer ending up with no history rather than overflowing >> - readline's history interface, which uses signed 'int' >> - everywhere. */ We won't overflow readline's history interface with or without the patch. The setting of our history size to -1 instructs set_readline_history_size() to call unstifle_history() instead of stifle_history(size). I personally don't like GDB's behavior because 1) it's not consistent with bash and 2) it's unforgiving: a mere typo when setting HISTSIZE will truncate the entire history file at exit. Another possibility is to not touch the history size at all when HISTSIZE is invalid. That makes the most sense to me but then the inconsistency with bash still remains. Dunno what's better..
On 05/13/2015 11:38 PM, Patrick Palka wrote: > On Wed, May 13, 2015 at 6:24 PM, Mark Kettenis <mark.kettenis@xs4all.nl> 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. ... > I personally don't like GDB's behavior because 1) it's not consistent > with bash and 2) it's unforgiving: a mere typo when setting HISTSIZE > will truncate the entire history file at exit. I agree. Users can well set HISTSIZE to -1 in their ~/.bashrc to affect bash's history, and then it's very annoying that GDB ends up with no history... As we're reusing this environment variable from bash, I think the right thing to do is follow whatever bash does. (BTW, like we have GDBHISTFILE vs bash's HISTFILE, it may be good to have a GDBHISTSIZE to override HISTSIZE without affecting bash.) Thanks, Pedro Alves
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
On Thu, May 14, 2015 at 6:16 AM, Pedro Alves <palves@redhat.com> wrote: > 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: > " When HISTSIZE is the empty string, I think we invoke UB. I'm not sure atoi() I was basing this patch off of the behavior of HISTFILESIZE, not HISTSIZE. This is because the behavior of HISTFILESIZE is more specified than that of HISTSIZE. According to the documentation for HISTFILESIZE: Non-numeric values and numeric values less than zero inhibit truncation. The shell sets the default value to the value of HISTSIZE after reading any startup files. ... If HISTFILESIZE is unset, or set to null, a non-numeric value, or a numeric value less than zero, the history file is not truncated. Whereas for HISTSIZE, the documentation just says: If the value is 0, commands are not saved in the history list. Numeric values less than zero result in every command being saved on the history list (there is no limit). bash does not explicitly document what happens if HISTSIZE is null or non-numeric. And because HISTSIZE determines both GDB's in-memory and in-file history size (we don't respond to HISTFILESIZE) I think it's not so bad to base our behavior of HISTSIZE handling off of bash's HISTFILESIZE handling. (Though I should've mentioned this in the patch.) But unless GDB responds to both HISTSIZE and HISTFILESIZE I suppose we will never be truly consistent with bash since apparently both of these variables have different behaviors under edge cases. So should we anyway match the behavior of bash's HISTSIZE?
On 05/14/2015 01:05 PM, Patrick Palka wrote: >> #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: >> " > > When HISTSIZE is the empty string, I think we invoke UB. I'm not sure atoi() I'd be very surprised if any implementation returned anything other than zero. > > I was basing this patch off of the behavior of HISTFILESIZE, not > HISTSIZE. This is because the behavior of HISTFILESIZE is more > specified than that of HISTSIZE. According to the documentation for > HISTFILESIZE: > > Non-numeric values and numeric values less than zero inhibit > truncation. The shell sets the default value to the value of HISTSIZE > after reading any startup files. ... If HISTFILESIZE is unset, or set > to null, a non-numeric value, or a numeric value less than zero, the > history file is not truncated. But this says that if set to null the history file is not truncated. AFAICS, your patch treats HISTSIZE= as leaving the size to the default, which truncates. Seems to me that the way to implement "not truncate" would be to set the history size to unlimited, like bash does (even if undocumented). The non-numeric handling of HISTSIZE and HISTFILESIZE seems to be different indeed. I think that's less of an issue than "HISTSIZE=", as people shouldn't be using non-numeric values on purpose, though I'm fine with not truncating (thus, unlimited) to avoid data loss on mistakes/typos. > But unless GDB responds to both HISTSIZE and HISTFILESIZE I suppose we > will never be truly consistent with bash since apparently both of > these variables have different behaviors under edge cases. > > So should we anyway match the behavior of bash's HISTSIZE? Yes, I think so, at least for the "HISTSIZE=" parts, given that lots of people will be following the stack-exchange advice of doing that to get unlimited history in all versions of bash. Thanks, Pedro Alves
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; history_size_setshow_var = var; }