[binutils-gdb] gdb: Allow GDB to _not_ load a previous command history

[binutils-gdb] gdb: Allow GDB to _not_ load a previous command history

[Andrew Burgess]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=63e163f24fe80fe1509527e6ccfcfb9622f5e99e

commit 63e163f24fe80fe1509527e6ccfcfb9622f5e99e
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Wed Jan 22 18:21:58 2020 +0000

    gdb: Allow GDB to _not_ load a previous command history
    
    This commit aims to give a cleaner mechanism by which the user can
    prevent GDB from trying to load any previous command history.
    
    Currently the user can change the path to the history file, either
    using a command line flag, or by setting the GDBHISTFILE environment
    variable, and if the path is set to a non-existent file, then
    obviously GDB wont load any command history.  However, this feels like
    a bit of a bodge, I'd like to add an official mechanism by which we
    can disable command history loading.
    
    Why would we want to prevent command history loading?  The specific
    use case I have is GDB starting with a CWD that is a network mounted
    directory, and there is no command history present.  Still GDB will
    access the network in order to check for the file.  In my particular
    use case I'm actually starting a large number of GDB instances in
    parallel, all in the same network mounted directory, the large number
    of network accesses looking for this file introduces a noticeable
    delay at GDB startup.
    
    The approach I'm proposing here is a slight adjustment to the current
    rules for setting up the history filename.  Currently, if a user does
    this, they see an error:
    
      (gdb) set history filename
      Argument required (filename to set it to.).
    
    However, if a user does this:
    
      $ GDBHISTFILE= gdb --quiet
      (gdb) set history save on
      (gdb) q
      warning: Could not rename -gdb18416~ to : No such file or directory
    
    So, we already have a bug in this area.  My plan is to allow the empty
    filename to be accepted, and for this to mean, neither load, nor save
    the command history.
    
    This does mean that we now have two mechanisms to prevent saving the
    command history:
    
      (gdb) set history filename
    
    or
    
      (gdb) set history save off
    
    But the only way to prevent loading the command history is to set the
    filename to the empty string _before_ you get to a GDB prompt, either
    using a command line option, or the environment variable.
    
    I've updated some of the show commands, for example this session:
    
      (gdb) set history filename
      (gdb) show history filename
      There is no filename currently set for recording the command history in.
      (gdb) show history save
      Saving of the history record on exit is off.
      (gdb) set history save on
      (gdb) show history save
      Saving of the history is disabled due to the value of 'history filename'.
      (gdb) set history filename /tmp/hist
      (gdb) show history save
      Saving of the history record on exit is on.
    
    I've updated the manual, and added some tests.
    
    gdb/ChangeLog:
    
    	* NEWS: Mention new behaviour of the history filename.
    	* top.c (write_history_p): Add comment.
    	(show_write_history_p): Add header comment, give a different
    	message when history writing is on, but the history filename is
    	empty.
    	(history_filename): Add comment.
    	(history_filename_empty): New function.
    	(show_history_filename): Add header comment, give a different
    	message when the filename is empty.
    	(init_history): Compare history_filename against nullptr, and only
    	read history if the filename is not empty.
    	(set_history_filename): Add header comment, and only make
    	non-empty filenames absolute.
    	(init_main): Make the filename argument to 'set history filename'
    	optional.
    
    gdb/doc/ChangeLog:
    
    	* gdb.texinfo (Command History): Extend description for
    	GDBHISTFILE and GDBHISTSIZE, add detail about the filename for
    	'set history filename' being optional.  Describe the effect of an
    	empty history filename on 'set history save on'.
    
    gdb/testsuite/ChangeLog:
    
    	* gdb.base/default.exp: Remove test of 'set history filename'.
    	* gdb.base/gdbinit-history.exp: Add tests for setting the history
    	filename to the empty string.
    	* lib/gdb.exp (gdb_init): Unset environment variables GDBHISTFILE
    	and GDBHISTSIZE.
    
    Change-Id: Ia586e4311182fac99113b60f11ef8a11fbd5450b

Diff:
---
 gdb/ChangeLog                              |  18 ++++
 gdb/NEWS                                   |   6 ++
 gdb/doc/ChangeLog                          |   7 ++
 gdb/doc/gdb.texinfo                        |  25 ++++-
 gdb/testsuite/ChangeLog                    |   8 ++
 gdb/testsuite/gdb.base/default.exp         |   2 -
 gdb/testsuite/gdb.base/gdbinit-history.exp | 151 +++++++++++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp                  |   6 ++
 gdb/top.c                                  |  51 ++++++++--
 9 files changed, 259 insertions(+), 15 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c997c35..e303a81 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,21 @@
+2020-03-02  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* NEWS: Mention new behaviour of the history filename.
+	* top.c (write_history_p): Add comment.
+	(show_write_history_p): Add header comment, give a different
+	message when history writing is on, but the history filename is
+	empty.
+	(history_filename): Add comment.
+	(history_filename_empty): New function.
+	(show_history_filename): Add header comment, give a different
+	message when the filename is empty.
+	(init_history): Compare history_filename against nullptr, and only
+	read history if the filename is not empty.
+	(set_history_filename): Add header comment, and only make
+	non-empty filenames absolute.
+	(init_main): Make the filename argument to 'set history filename'
+	optional.
+
 2020-03-02  Christian Biesinger  <cbiesinger@google.com>
 
 	* arm-nbsd-nat.c (arm_supply_fparegset): Rename to...
diff --git a/gdb/NEWS b/gdb/NEWS
index d3a3605..cbdfcad 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -33,6 +33,12 @@
 
 * TUI windows can now be arranged horizontally.
 
+* The command history filename can now be set to the empty string
+  either using 'set history filename' or by setting 'GDBHISTFILE=' in
+  the environment.  The effect of setting this filename to the empty
+  string is that GDB will not try to load any previous command
+  history.
+
 * New commands
 
 set exec-file-mismatch -- Set exec-file-mismatch handling (ask|warn|off).
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index c7f0243..95b6843 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,10 @@
+2020-03-02  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.texinfo (Command History): Extend description for
+	GDBHISTFILE and GDBHISTSIZE, add detail about the filename for
+	'set history filename' being optional.  Describe the effect of an
+	empty history filename on 'set history save on'.
+
 2020-02-24  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* gdb.texinfo (Threads): Fix alignment in 'info threads' example
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b947c5d..32e419e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -25306,7 +25306,7 @@ history.
 @cindex history file
 @kindex set history filename
 @cindex @env{GDBHISTFILE}, environment variable
-@item set history filename @var{fname}
+@item set history filename @r{[}@var{fname}@r{]}
 Set the name of the @value{GDBN} command history file to @var{fname}.
 This is the file where @value{GDBN} reads an initial command history
 list, and where it writes the command history from this session when it
@@ -25316,15 +25316,29 @@ to the value of the environment variable @code{GDBHISTFILE}, or to
 @file{./.gdb_history} (@file{./_gdb_history} on MS-DOS) if this variable
 is not set.
 
+The @code{GDBHISTFILE} environment variable is read after processing
+any @value{GDBN} initialization files (@pxref{Startup}) and after
+processing any commands passed using command line options (for
+example, @code{-ex}).
+
+If the @var{fname} argument is not given, or if the @code{GDBHISTFILE}
+is the empty string then @value{GDBN} will neither try to load an
+existing history file, nor will it try to save the history on exit.
+
 @cindex save command history
 @kindex set history save
 @item set history save
 @itemx set history save on
 Record command history in a file, whose name may be specified with the
-@code{set history filename} command.  By default, this option is disabled.
+@code{set history filename} command.  By default, this option is
+disabled.  The command history will be recorded when @value{GDBN}
+exits.  If @code{set history filename} is set to the empty string then
+history saving is disabled, even when @code{set history save} is
+@code{on}.
 
 @item set history save off
-Stop recording command history in a file.
+Don't record the command history into the file specified by @code{set
+history filename} when @value{GDBN} exits.
 
 @cindex history size
 @kindex set history size
@@ -25338,6 +25352,11 @@ are ignored.  If @var{size} is @code{unlimited} or if @env{GDBHISTSIZE} is
 either a negative number or the empty string, then the number of commands
 @value{GDBN} keeps in the history list is unlimited.
 
+The @code{GDBHISTSIZE} environment variable is read after processing
+any @value{GDBN} initialization files (@pxref{Startup}) and after
+processing any commands passed using command line options (for
+example, @code{-ex}).
+
 @cindex remove duplicate history
 @kindex set history remove-duplicates
 @item set history remove-duplicates @var{count}
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 488a328..55c0a3e 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,13 @@
 2020-03-02  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* gdb.base/default.exp: Remove test of 'set history filename'.
+	* gdb.base/gdbinit-history.exp: Add tests for setting the history
+	filename to the empty string.
+	* lib/gdb.exp (gdb_init): Unset environment variables GDBHISTFILE
+	and GDBHISTSIZE.
+
+2020-03-02  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* gdb.server/stop-reply-no-thread.exp: Add test where T packet is
 	disabled.
 
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index 097619d..c51ec63 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -497,8 +497,6 @@ gdb_test "set environment" "Argument required .environment variable and value.*"
 gdb_test "set height" "Argument required .integer to set it to.*"
 #test set history expansion
 gdb_test_no_output "set history expansion" "set history expansion"
-#test set history filename
-gdb_test "set history filename" "Argument required .filename to set it to.*"
 # Make sure the history ends up in the right place.
 gdb_test_no_output "set history filename [standard_output_file .gdb_history]" \
     "set the history filename"
diff --git a/gdb/testsuite/gdb.base/gdbinit-history.exp b/gdb/testsuite/gdb.base/gdbinit-history.exp
index 528c07d..baa1b49 100644
--- a/gdb/testsuite/gdb.base/gdbinit-history.exp
+++ b/gdb/testsuite/gdb.base/gdbinit-history.exp
@@ -128,6 +128,154 @@ proc test_no_truncation_of_unlimited_history_file { } {
     }
 }
 
+# Check that the current command history matches HIST, which is a list
+# of commands, oldest fist.
+proc check_history { hist } {
+
+    # The show commands we issue here always appears last in the
+    # commands list.
+    lappend hist "show commands"
+
+    # Number all of the entries in the HIST list and convert the list
+    # into a pattern to match against GDB.
+    set hist_lines [list]
+    set idx 1
+    foreach h $hist {
+	lappend hist_lines "    $idx  $h"
+	incr idx
+    }
+    set pattern [eval multi_line $hist_lines]
+
+    # Check the history.
+    gdb_test "show commands" "$pattern.*"
+}
+
+# Run 'show history filename' and check the output contains the
+# filename matching PATTERN, unless, PATTERN is the empty string, in
+# which case match a different output that GDB will give if the
+# history filename is the empty string.
+#
+# TESTNAME is the name for the test, which defaults to the command run
+# in the test.
+proc check_history_filename { pattern {testname ""} } {
+
+    set cmd "show history filename"
+    if { $testname == "" } {
+	set testname $cmd
+    }
+
+    if { $pattern == "" } {
+	gdb_test $cmd \
+	    "There is no filename currently set for recording the command history in." \
+	    $testname
+    } else {
+	gdb_test $cmd \
+	    "The filename in which to record the command history is \"$pattern\"\." \
+	    $testname
+    }
+}
+
+# Tests for how GDB handles setting the history filename to the empty
+# string.
+proc test_empty_history_filename { } {
+    global env
+    global gdb_prompt
+
+    set common_history [list "set height 0" "set width 0"]
+
+    set test_dir [standard_output_file history_test]
+    remote_exec host "mkdir -p $test_dir"
+    foreach entry { { ".gdb_history" "xxxxx" } \
+			{ "_gdb_history" "xxxxx" } \
+			{ "alt_history" "yyyyy" } } {
+	set fn [lindex $entry 0]
+	set content [lindex $entry 1]
+	set fd [open [standard_output_file "$test_dir/$fn"] w]
+	puts $fd "$content"
+	close $fd
+    }
+
+    with_cwd "$test_dir" {
+	with_test_prefix "load default history file" {
+	    # Start GDB and see that the history file was loaded
+	    # correctly.
+	    gdb_exit
+	    gdb_start
+	    check_history [concat "xxxxx" $common_history]
+	    check_history_filename ".*/.gdb_history"
+	}
+
+	with_test_prefix "load GDBHISTFILE history file" {
+	    # Now restart GDB with GDBHISTFILE set to see that the
+	    # "other" history file is loaded.
+	    save_vars { env(GDBHISTFILE) } {
+		setenv GDBHISTFILE \
+		    "$test_dir/alt_history"
+		gdb_exit
+		gdb_start
+		check_history [concat "yyyyy" $common_history]
+		check_history_filename ".*/alt_history"
+	    }
+	}
+
+	with_test_prefix "GDBHISTFILE is empty" {
+	    # Now restart GDB with GDBHISTFILE set to indicate don't
+	    # load any history file, check none was loaded.
+	    save_vars { env(GDBHISTFILE) } {
+	    setenv GDBHISTFILE ""
+		gdb_exit
+		gdb_start
+		check_history $common_history
+		check_history_filename ""
+	    }
+
+	    # Check that 'show history save' does the right thing when
+	    # the history filename is the empty string.
+	    gdb_test_no_output "set history save off" \
+		"ensure history save is off initially"
+	    gdb_test "show history save" \
+		"Saving of the history record on exit is off." \
+		"Check history save is off"
+	    gdb_test_no_output "set history save on"
+	    gdb_test "show history save" \
+		"Saving of the history is disabled due to the value of 'history filename'." \
+		"Check history save is off due to filename"
+	    gdb_test_no_output \
+		"set history filename $test_dir/alt_history" \
+		"set history filename at the command line"
+	    check_history_filename ".*/alt_history" \
+		"check filename after setting at the command line"
+	    gdb_test "show history save" \
+		"Saving of the history record on exit is on." \
+		"Check history save is on"
+	    gdb_test_no_output "set history filename"
+	    gdb_test "show history save" \
+		"Saving of the history is disabled due to the value of 'history filename'." \
+		"Check history save is off due to filename again"
+	    gdb_test_no_output "set history save off"
+	}
+
+	with_test_prefix "Use -ex to clear history file" {
+	    # Now restart GDB with the command line '-ex' to indicate
+	    # no history file should be loaded.
+	    gdb_exit
+	    if {[gdb_spawn_with_cmdline_opts \
+		     "-ex \"set history filename\""] != 0} {
+		fail "spawn"
+		return
+	    }
+	    set test "initial prompt"
+	    gdb_test_multiple "" $test {
+		-re ".*$gdb_prompt $" {
+		    pass "$test"
+		}
+	    }
+	    check_history [list]
+	    check_history_filename ""
+	}
+    }
+}
+
 test_gdbinit_history_setting "gdbinit-history/unlimited" "unlimited"
 test_gdbinit_history_setting "gdbinit-history/zero" "0"
 
@@ -138,3 +286,6 @@ test_no_truncation_of_unlimited_history_file
 # .gdbinit file.
 test_gdbinit_history_setting "gdbinit-history/unlimited" "1000" "1000"
 test_gdbinit_history_setting "gdbinit-history/unlimited" "unlimited" "foo"
+
+# Check handling of empty history filename.
+test_empty_history_filename
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index ab22f24..f8f404f 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -5097,6 +5097,12 @@ proc gdb_init { test_file_name } {
     # tests.
     setenv TERM "dumb"
 
+    # Ensure that GDBHISTFILE and GDBHISTSIZE are removed from the
+    # environment, we don't want these modifications to the history
+    # settings.
+    unset -nocomplain ::env(GDBHISTFILE)
+    unset -nocomplain ::env(GDBHISTSIZE)
+
     # Initialize GDB's pty with a fixed size, to make sure we avoid pagination
     # during startup.  See "man expect" for details about stty_init.
     global stty_init
diff --git a/gdb/top.c b/gdb/top.c
index 1a98ae1..e243248 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -84,6 +84,8 @@
 
 extern void initialize_all_files (void);
 
+static bool history_filename_empty (void);
+
 #define PROMPT(X) the_prompts.prompt_stack[the_prompts.top + X].prompt
 #define PREFIX(X) the_prompts.prompt_stack[the_prompts.top + X].prefix
 #define SUFFIX(X) the_prompts.prompt_stack[the_prompts.top + X].suffix
@@ -884,13 +886,22 @@ static bool command_editing_p;
 
 /* static */ bool history_expansion_p;
 
+/* Should we write out the command history on exit?  In order to write out
+   the history both this flag must be true, and the history_filename
+   variable must be set to something sensible.  */
 static bool write_history_p;
+
+/* Implement 'show history save'.  */
 static void
 show_write_history_p (struct ui_file *file, int from_tty,
 		      struct cmd_list_element *c, const char *value)
 {
-  fprintf_filtered (file, _("Saving of the history record on exit is %s.\n"),
-		    value);
+  if (!write_history_p || !history_filename_empty ())
+    fprintf_filtered (file, _("Saving of the history record on exit is %s.\n"),
+		      value);
+  else
+    fprintf_filtered (file, _("Saving of the history is disabled due to "
+			      "the value of 'history filename'.\n"));
 }
 
 /* The variable associated with the "set/show history size"
@@ -919,14 +930,30 @@ show_history_remove_duplicates (struct ui_file *file, int from_tty,
 		    value);
 }
 
+/* The name of the file in which GDB history will be written.  If this is
+   set to NULL, of the empty string then history will not be written.  */
 static char *history_filename;
+
+/* Return true if the history_filename is either NULL or the empty string,
+   indicating that we should not try to read, nor write out the history.  */
+static bool
+history_filename_empty (void)
+{
+  return (history_filename == nullptr || *history_filename == '\0');
+}
+
+/* Implement 'show history filename'.  */
 static void
 show_history_filename (struct ui_file *file, int from_tty,
 		       struct cmd_list_element *c, const char *value)
 {
-  fprintf_filtered (file, _("The filename in which to record "
-			    "the command history is \"%ps\".\n"),
-		    styled_string (file_name_style.style (), value));
+  if (!history_filename_empty ())
+    fprintf_filtered (file, _("The filename in which to record "
+			      "the command history is \"%ps\".\n"),
+		      styled_string (file_name_style.style (), value));
+  else
+    fprintf_filtered (file, _("There is no filename currently set for "
+			      "recording the command history in.\n"));
 }
 
 /* This is like readline(), but it has some gdb-specific behavior.
@@ -2017,9 +2044,9 @@ init_history (void)
   set_readline_history_size (history_size_setshow_var);
 
   tmpenv = getenv ("GDBHISTFILE");
-  if (tmpenv)
+  if (tmpenv != nullptr)
     history_filename = xstrdup (tmpenv);
-  else if (!history_filename)
+  else if (history_filename == nullptr)
     {
       /* We include the current directory so that if the user changes
          directories the file written will be the same as the one
@@ -2034,7 +2061,9 @@ init_history (void)
       gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (fname));
       history_filename = temp.release ();
     }
-  read_history (history_filename);
+
+  if (!history_filename_empty ())
+    read_history (history_filename);
 }
 
 static void
@@ -2103,6 +2132,8 @@ show_gdb_datadir (struct ui_file *file, int from_tty,
 				   gdb_datadir.c_str ()));
 }
 
+/* Implement 'set history filename'.  */
+
 static void
 set_history_filename (const char *args,
 		      int from_tty, struct cmd_list_element *c)
@@ -2110,7 +2141,7 @@ set_history_filename (const char *args,
   /* We include the current directory so that if the user changes
      directories the file written will be the same as the one
      that was read.  */
-  if (!IS_ABSOLUTE_PATH (history_filename))
+  if (!history_filename_empty () && !IS_ABSOLUTE_PATH (history_filename))
     {
       gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (history_filename));
 
@@ -2217,7 +2248,7 @@ By default this option is set to 0."),
 			   show_history_remove_duplicates,
 			   &sethistlist, &showhistlist);
 
-  add_setshow_filename_cmd ("filename", no_class, &history_filename, _("\
+  add_setshow_optional_filename_cmd ("filename", no_class, &history_filename, _("\
 Set the filename in which to record the command history."), _("\
 Show the filename in which to record the command history."), _("\
 (the list of previous commands of which a record is kept)."),