diff mbox

Test the interaction between GDBHISTSIZE and .gdbinit

Message ID 1434572241-16019-1-git-send-email-patrick@parcs.ath.cx
State New, archived
Headers

Commit Message

Patrick Palka June 17, 2015, 8:17 p.m. UTC 
  The value inside the GDBHISTSIZE environment variable, only if valid,
should override setting the history size through one's .gdbinit file.

gdb/testsuite/ChangeLog:

	* gdb.base/gdbinit-history.exp: Test the interaction between
	setting GDBHISTSIZE and setting the history size via .gdbinit.
---
 gdb/testsuite/gdb.base/gdbinit-history.exp | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

Comments

Pedro Alves June 18, 2015, 9:06 a.m. UTC | #1 
On 06/17/2015 09:17 PM, Patrick Palka wrote:
> The value inside the GDBHISTSIZE environment variable, only if valid,
> should override setting the history size through one's .gdbinit file.

Thanks, looks good.

> +    unset -nocomplain env(GDBHISTSIZE)
>      array set env [array get old_env]

Though this unset looks unnecessary, given that the following line
restores the whole array.

Thanks,
Pedro Alves
Patrick Palka June 18, 2015, 12:44 p.m. UTC | #2 
On Thu, Jun 18, 2015 at 5:06 AM, Pedro Alves <palves@redhat.com> wrote:
> On 06/17/2015 09:17 PM, Patrick Palka wrote:
>> The value inside the GDBHISTSIZE environment variable, only if valid,
>> should override setting the history size through one's .gdbinit file.
>
> Thanks, looks good.
>
>> +    unset -nocomplain env(GDBHISTSIZE)
>>      array set env [array get old_env]
>
> Though this unset looks unnecessary, given that the following line
> restores the whole array.

It turns out that

    array set env [array get old_env]

does not completely restore the env array to its original state.  What
it seems to do is to reset each pre-existing environment variable
(existing in the saved env array) to its original value.  New
environment variables that were set inside the env array in the
meantime do not get unset after restoring.  So e.g. after doing

    array set old_env [array get env]
    set env(SOME_NEW_VAR) foo
    array set env [array get old_env]

the environment variable SOME_NEW_VAR=foo will still be in the env
array.  So this "array set env" trick is insufficient.  That is why
the unset of GDBHISTSIZE is necessary there.

To make the pattern of "temporarily altering global variables,
restoring their original value afterwards" more convenient and less
error-prone, I've been thinking about introducing a new tcl proc that
acts as a wrapper for saving/restoring a specified list of variables.
Its use would look something like:

    save_vars { INTERNAL_GDBFLAGS env(GDBHISTSIZE) env(HOME) } {
        append INTERNAL_GDBFLAGS " -nx"
        unset -nocomplain env(GDBHISTSIZE)
        unset -nocomplain env(HOME)
        gdb_test ....
        more_gdb_test ...
    }

which guarantees that after the body has finished executing, the given
list of variables will have their contents restored to their original
values.  What do you think about this?
Doug Evans June 22, 2015, 1:21 p.m. UTC | #3 
On Thu, Jun 18, 2015 at 7:44 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Thu, Jun 18, 2015 at 5:06 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 06/17/2015 09:17 PM, Patrick Palka wrote:
>>> The value inside the GDBHISTSIZE environment variable, only if valid,
>>> should override setting the history size through one's .gdbinit file.
>>
>> Thanks, looks good.
>>
>>> +    unset -nocomplain env(GDBHISTSIZE)
>>>      array set env [array get old_env]
>>
>> Though this unset looks unnecessary, given that the following line
>> restores the whole array.
>
> It turns out that
>
>     array set env [array get old_env]
>
> does not completely restore the env array to its original state.  What
> it seems to do is to reset each pre-existing environment variable
> (existing in the saved env array) to its original value.  New
> environment variables that were set inside the env array in the
> meantime do not get unset after restoring.

http://tcl.tk/man/tcl8.5/TclCmd/array.htm

> So e.g. after doing
>
>     array set old_env [array get env]
>     set env(SOME_NEW_VAR) foo
>     array set env [array get old_env]
>
> the environment variable SOME_NEW_VAR=foo will still be in the env
> array.  So this "array set env" trick is insufficient.  That is why
> the unset of GDBHISTSIZE is necessary there.

I haven't read the save_vars patch yet, but how about:

array set old_env [array get env]
...
array unset env ;# <<<<<<<<<<<<<<<
array set env [array get old_env]
array unset old_env

It might be a teensy bit simpler to do:

set old_env [array get env]
...
array set env $old_env
unset old_env

Dunno.
Patrick Palka June 22, 2015, 1:46 p.m. UTC | #4 
On Mon, Jun 22, 2015 at 9:21 AM, Doug Evans <dje@google.com> wrote:
> On Thu, Jun 18, 2015 at 7:44 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
>> On Thu, Jun 18, 2015 at 5:06 AM, Pedro Alves <palves@redhat.com> wrote:
>>> On 06/17/2015 09:17 PM, Patrick Palka wrote:
>>>> The value inside the GDBHISTSIZE environment variable, only if valid,
>>>> should override setting the history size through one's .gdbinit file.
>>>
>>> Thanks, looks good.
>>>
>>>> +    unset -nocomplain env(GDBHISTSIZE)
>>>>      array set env [array get old_env]
>>>
>>> Though this unset looks unnecessary, given that the following line
>>> restores the whole array.
>>
>> It turns out that
>>
>>     array set env [array get old_env]
>>
>> does not completely restore the env array to its original state.  What
>> it seems to do is to reset each pre-existing environment variable
>> (existing in the saved env array) to its original value.  New
>> environment variables that were set inside the env array in the
>> meantime do not get unset after restoring.
>
> http://tcl.tk/man/tcl8.5/TclCmd/array.htm
>
>> So e.g. after doing
>>
>>     array set old_env [array get env]
>>     set env(SOME_NEW_VAR) foo
>>     array set env [array get old_env]
>>
>> the environment variable SOME_NEW_VAR=foo will still be in the env
>> array.  So this "array set env" trick is insufficient.  That is why
>> the unset of GDBHISTSIZE is necessary there.
>
> I haven't read the save_vars patch yet, but how about:
>
> array set old_env [array get env]
> ...
> array unset env ;# <<<<<<<<<<<<<<<
> array set env [array get old_env]
> array unset old_env
>
> It might be a teensy bit simpler to do:
>
> set old_env [array get env]
> ...
> array set env $old_env
> unset old_env
>
> Dunno.

The env array is "magical" so I'm not sure if these techniques may work on it.
Doug Evans June 22, 2015, 1:59 p.m. UTC | #5 
On Mon, Jun 22, 2015 at 8:46 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Mon, Jun 22, 2015 at 9:21 AM, Doug Evans <dje@google.com> wrote:
>> On Thu, Jun 18, 2015 at 7:44 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
>>> On Thu, Jun 18, 2015 at 5:06 AM, Pedro Alves <palves@redhat.com> wrote:
>>>> On 06/17/2015 09:17 PM, Patrick Palka wrote:
>>>>> The value inside the GDBHISTSIZE environment variable, only if valid,
>>>>> should override setting the history size through one's .gdbinit file.
>>>>
>>>> Thanks, looks good.
>>>>
>>>>> +    unset -nocomplain env(GDBHISTSIZE)
>>>>>      array set env [array get old_env]
>>>>
>>>> Though this unset looks unnecessary, given that the following line
>>>> restores the whole array.
>>>
>>> It turns out that
>>>
>>>     array set env [array get old_env]
>>>
>>> does not completely restore the env array to its original state.  What
>>> it seems to do is to reset each pre-existing environment variable
>>> (existing in the saved env array) to its original value.  New
>>> environment variables that were set inside the env array in the
>>> meantime do not get unset after restoring.
>>
>> http://tcl.tk/man/tcl8.5/TclCmd/array.htm
>>
>>> So e.g. after doing
>>>
>>>     array set old_env [array get env]
>>>     set env(SOME_NEW_VAR) foo
>>>     array set env [array get old_env]
>>>
>>> the environment variable SOME_NEW_VAR=foo will still be in the env
>>> array.  So this "array set env" trick is insufficient.  That is why
>>> the unset of GDBHISTSIZE is necessary there.
>>
>> I haven't read the save_vars patch yet, but how about:
>>
>> array set old_env [array get env]
>> ...
>> array unset env ;# <<<<<<<<<<<<<<<
>> array set env [array get old_env]
>> array unset old_env
>>
>> It might be a teensy bit simpler to do:
>>
>> set old_env [array get env]
>> ...
>> array set env $old_env
>> unset old_env
>>
>> Dunno.
>
> The env array is "magical" so I'm not sure if these techniques may work on it.

Well, that's unfortunate.

https://www.tcl.tk/man/tcl8.5/TclCmd/tclvars.htm

"If the entire env array is unset then Tcl will stop monitoring env
accesses and will not update environment variables."

Still, it should be possible to write the equivalent.
I'll look at the save_vars patch.
diff mbox

Patch

diff --git a/gdb/testsuite/gdb.base/gdbinit-history.exp b/gdb/testsuite/gdb.base/gdbinit-history.exp
index 7bdce5f..85177f3 100644
--- a/gdb/testsuite/gdb.base/gdbinit-history.exp
+++ b/gdb/testsuite/gdb.base/gdbinit-history.exp
@@ -19,9 +19,10 @@ 
 
 
 # Check that the history size is properly set to SIZE when reading the .gdbinit
-# file located in HOME.
+# file located in HOME with the environment variable GDBHISTSIZE optionally
+# set to GDBHISTSIZE_VAL.
 
-proc test_gdbinit_history_setting { home size } {
+proc test_gdbinit_history_setting { home size { gdbhistsize_val "-" } } {
     global env
     global INTERNAL_GDBFLAGS
     global srcdir
@@ -36,10 +37,19 @@  proc test_gdbinit_history_setting { home size } {
     # set.
     unset -nocomplain env(GDBHISTSIZE)
 
+    if { $gdbhistsize_val != "-" } {
+	set env(GDBHISTSIZE) $gdbhistsize_val
+    }
+
     set saved_internal_gdbflags $INTERNAL_GDBFLAGS
     set INTERNAL_GDBFLAGS [string map {"-nx" ""} $INTERNAL_GDBFLAGS]
 
-    with_test_prefix "home=$home" {
+    set prefix "home=$home"
+    if { $gdbhistsize_val != "-" } {
+	append prefix " gdbhistsize=$gdbhistsize_val"
+    }
+
+    with_test_prefix $prefix {
 	gdb_exit
 	gdb_start
 
@@ -54,6 +64,7 @@  proc test_gdbinit_history_setting { home size } {
 
     set INTERNAL_GDBFLAGS $saved_internal_gdbflags
 
+    unset -nocomplain env(GDBHISTSIZE)
     array set env [array get old_env]
 }
 
@@ -117,3 +128,9 @@  test_gdbinit_history_setting "gdbinit-history/unlimited" "unlimited"
 test_gdbinit_history_setting "gdbinit-history/zero" "0"
 
 test_no_truncation_of_unlimited_history_file
+
+# A valid GDBHISTSIZE value overrides the setting inside the .gdbinit file; an
+# invalid GDBHISTSIZE value is ignored, falling back on the setting inside the
+# .gdbinit file.
+test_gdbinit_history_setting "gdbinit-history/unlimited" "1000" "1000"
+test_gdbinit_history_setting "gdbinit-history/unlimited" "unlimited" "foo"