gdb: make string-like set show commands use std::string variable

String-like settings (var_string, var_filename, var_optional_filename,
var_string_noescape) currently take a pointer to a `char *` storage
variable (typically global) that holds the setting's value.  I'd like to
"mordernize" this by changing them to use an std::string for storage.

An obvious reason is that string operations on std::string are often
easier to write than with C strings.  And they avoid having to do any
manual memory management.

Another interesting reason is that, with `char *`, nullptr and an empty
string often both have the same meaning of "no value".  String settings
are initially nullptr (unless initialized otherwise).  But when doing
"set foo" (where `foo` is a string setting), the setting now points to
an empty string.  For example, solib_search_path is nullptr at startup,
but points to an empty string after doing "set solib-search-path".  This
leads to some code that needs to check for both to check for "no value".
Or some code that converts back and forth between NULL and "" when
getting or setting the value.  I find this very error-prone, because it
is very easy to forget one or the other.  With std::string, we at least
know that the variable is not "NULL".  There is only one way of
representing an empty string setting, that is with an empty string.

I was wondering whether the distinction between NULL and "" would be
important for some setting, but it doesn't seem so.  If that ever
happens, it would be more C++-y and self-descriptive to use
optional<string> anyway.

Actually, there's one spot where this distinction mattered, it's in
init_history, for the test gdb.base/gdbinit-history.exp.  init_history
sets the history filename to the default ".gdb_history" if it sees that
the setting was never set - if history_filename is nullptr.  If
history_filename is an empty string, it means the setting was explicitly
cleared, so it leaves it as-is.  With the change to std::string, this
distinction doesn't exist anymore.  This can be fixed by moving the code
that chooses a good default value for history_filename to
_initialize_top.  This is ran before -ex commands are processed, so an
-ex command can then clear that value if needed (what
gdb.base/gdbinit-history.exp tests).

Another small improvement, in my opinion is that we can now easily
give string parameters initial values, by simply initializing the global
variables, instead of xstrdup-ing it in the _initialize function.

In Python and Guile, when registering a string-like parameter, we
allocate (with new) an std::string that is owned by the param_smob (in
Guile) and the parmpy_object (in Python) objects.

This patch started by changing all relevant add_setshow_* commands to
take an `std::string *` instead of a `char **` and fixing everything
that failed to build.  That includes of course all string setting
variable and their uses.

string_option_def now uses an std::string also, because there's a
connection between options and settings (see
add_setshow_cmds_for_options).

The add_path function in source.c is really complex and twisted, I'd
rather not try to change it to work on an std::string right now.
Instead, I added an overload that copies the std:string to a `char *`
and back.  This means more copying, but this is not used in a hot path
at all, so I think it is acceptable.

Change-Id: I92c50a1bdd8307141cdbacb388248e4e4fc08c93
Co-authored-by: Lancelot SIX <lsix@lancelotsix.com>

1 files changed, 53 insertions, 59 deletions

diff --git a/gdb/top.c b/gdb/top.c
index bca007e..75692d3 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -84,8 +84,6 @@
 
 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
@@ -921,12 +919,16 @@ static bool command_editing_p;
    variable must be set to something sensible.  */
 static bool write_history_p;
 
+/* 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 std::string history_filename;
+
 /* 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)
 {
-  if (!write_history_p || !history_filename_empty ())
+  if (!write_history_p || !history_filename.empty ())
     fprintf_filtered (file, _("Saving of the history record on exit is %s.\n"),
 		      value);
   else
@@ -960,24 +962,12 @@ 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)
 {
-  if (!history_filename_empty ())
+  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));
@@ -1244,14 +1234,15 @@ gdb_safe_append_history (void)
   int ret, saved_errno;
 
   std::string local_history_filename
-    = string_printf ("%s-gdb%ld~", history_filename, (long) getpid ());
+    = string_printf ("%s-gdb%ld~", history_filename.c_str (), (long) getpid ());
 
-  ret = rename (history_filename, local_history_filename.c_str ());
+  ret = rename (history_filename.c_str (), local_history_filename.c_str ());
   saved_errno = errno;
   if (ret < 0 && saved_errno != ENOENT)
     {
       warning (_("Could not rename %ps to %ps: %s"),
-	       styled_string (file_name_style.style (), history_filename),
+	       styled_string (file_name_style.style (),
+			      history_filename.c_str ()),
 	       styled_string (file_name_style.style (),
 			      local_history_filename.c_str ()),
 	       safe_strerror (saved_errno));
@@ -1279,11 +1270,11 @@ gdb_safe_append_history (void)
 				   history_max_entries);
 	}
 
-      ret = rename (local_history_filename.c_str (), history_filename);
+      ret = rename (local_history_filename.c_str (), history_filename.c_str ());
       saved_errno = errno;
       if (ret < 0 && saved_errno != EEXIST)
 	warning (_("Could not rename %s to %s: %s"),
-		 local_history_filename.c_str (), history_filename,
+		 local_history_filename.c_str (), history_filename.c_str (),
 		 safe_strerror (saved_errno));
     }
 }
@@ -1656,12 +1647,12 @@ tree, and GDB will still find it.)\n\
 
 /* The current top level prompt, settable with "set prompt", and/or
    with the python `gdb.prompt_hook' hook.  */
-static char *top_prompt;
+static std::string top_prompt;
 
 /* Access method for the GDB prompt string.  */
 
-char *
-get_prompt (void)
+const std::string &
+get_prompt ()
 {
   return top_prompt;
 }
@@ -1671,10 +1662,7 @@ get_prompt (void)
 void
 set_prompt (const char *s)
 {
-  char *p = xstrdup (s);
-
-  xfree (top_prompt);
-  top_prompt = p;
+  top_prompt = s;
 }
 
 
@@ -1814,7 +1802,7 @@ quit_force (int *exit_arg, int from_tty)
   /* Save the history information if it is appropriate to do so.  */
   try
     {
-      if (write_history_p && history_filename)
+      if (write_history_p && !history_filename.empty ())
 	{
 	  int save = 0;
 
@@ -2062,27 +2050,8 @@ init_history (void)
 
   set_readline_history_size (history_size_setshow_var);
 
-  tmpenv = getenv ("GDBHISTFILE");
-  if (tmpenv != nullptr)
-    history_filename = xstrdup (tmpenv);
-  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
-	 that was read.  */
-#ifdef __MSDOS__
-      /* No leading dots in file names are allowed on MSDOS.  */
-      const char *fname = "_gdb_history";
-#else
-      const char *fname = ".gdb_history";
-#endif
-
-      gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (fname));
-      history_filename = temp.release ();
-    }
-
-  if (!history_filename_empty ())
-    read_history (history_filename);
+  if (!history_filename.empty ())
+    read_history (history_filename.c_str ());
 }
 
 static void
@@ -2132,21 +2101,20 @@ show_exec_done_display_p (struct ui_file *file, int from_tty,
    Extension languages, for example Python's gdb.parameter API, will read
    the value directory from this variable, so we must ensure that this
    always contains the correct value.  */
-static char *staged_gdb_datadir;
+static std::string staged_gdb_datadir;
 
 /* "set" command for the gdb_datadir configuration variable.  */
 
 static void
 set_gdb_datadir (const char *args, int from_tty, struct cmd_list_element *c)
 {
-  set_gdb_data_directory (staged_gdb_datadir);
+  set_gdb_data_directory (staged_gdb_datadir.c_str ());
 
   /* SET_GDB_DATA_DIRECTORY will resolve relative paths in
      STAGED_GDB_DATADIR, so we now copy the value from GDB_DATADIR
      back into STAGED_GDB_DATADIR so the extension languages can read the
      correct value.  */
-  free (staged_gdb_datadir);
-  staged_gdb_datadir = strdup (gdb_datadir.c_str ());
+  staged_gdb_datadir = gdb_datadir;
 
   gdb::observers::gdb_datadir_changed.notify ();
 }
@@ -2171,12 +2139,13 @@ 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 (!history_filename_empty () && !IS_ABSOLUTE_PATH (history_filename))
+  if (!history_filename.empty ()
+      && !IS_ABSOLUTE_PATH (history_filename.c_str ()))
     {
-      gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (history_filename));
+      gdb::unique_xmalloc_ptr<char> temp
+	(gdb_abspath (history_filename.c_str ()));
 
-      xfree (history_filename);
-      history_filename = temp.release ();
+      history_filename = temp.get ();
     }
 }
 
@@ -2342,7 +2311,7 @@ When set, GDB uses the specified path to search for data files."),
 			   &setlist,
 			    &showlist);
   /* Prime the initial value for data-directory.  */
-  staged_gdb_datadir = strdup (gdb_datadir.c_str ());
+  staged_gdb_datadir = gdb_datadir;
 
   add_setshow_auto_boolean_cmd ("interactive-mode", class_support,
 				&interactive_mode, _("\
@@ -2428,3 +2397,28 @@ gdb_init ()
   /* Create $_gdb_major and $_gdb_minor convenience variables.  */
   init_gdb_version_vars ();
 }
+
+void _initialize_top ();
+void
+_initialize_top ()
+{
+  /* Determine a default value for the history filename.  */
+  const char *tmpenv = getenv ("GDBHISTFILE");
+  if (tmpenv != nullptr)
+    history_filename = tmpenv;
+  else
+    {
+      /* 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.  */
+#ifdef __MSDOS__
+      /* No leading dots in file names are allowed on MSDOS.  */
+      const char *fname = "_gdb_history";
+#else
+      const char *fname = ".gdb_history";
+#endif
+
+      gdb::unique_xmalloc_ptr<char> temp (gdb_abspath (fname));
+      history_filename = temp.get ();
+    }
+}