Re: [PATCH v2 5/7] Add early startup command file

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Tom Tromey <tom@tromey.com> [2020-06-23 07:20:04 -0600]:

> This adds support for a gdb command file to be read early in startup.
> Code that wants to save an early setting can register a callback to be
> called to write to this file.  This code is not yet used, but will be
> in subsequent patches.
> 
> 2020-06-22  Tom Tromey  <tom@tromey.com>
> 
> 	* main.c (captured_main_1): Call read_startup_file.
> 	* cli/cli-setshow.h (write_startup_functions, add_startup_writer)
> 	(read_startup_file): Declare.
> 	(write_startup_setting_ftype): New typedef.
> 	* cli/cli-setshow.c (STARTUP_FILE): New define.
> 	(write_startup_functions, startup_file_read): New globals.
> 	(write_startup_file, add_startup_writer, read_startup_file): New
> 	functions.
> ---
>  gdb/ChangeLog         | 11 +++++++
>  gdb/cli/cli-setshow.c | 76 +++++++++++++++++++++++++++++++++++++++++++
>  gdb/cli/cli-setshow.h | 21 ++++++++++++
>  gdb/main.c            |  5 +++
>  4 files changed, 113 insertions(+)
> 
> diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
> index 19a5565bfe0..32e867160cb 100644
> --- a/gdb/cli/cli-setshow.c
> +++ b/gdb/cli/cli-setshow.c
> @@ -21,6 +21,8 @@
>  #include <ctype.h>
>  #include "arch-utils.h"
>  #include "observable.h"
> +#include "gdbsupport/pathstuff.h"
> +#include <vector>
>  
>  #include "ui-out.h"
>  
> @@ -29,6 +31,80 @@
>  #include "cli/cli-setshow.h"
>  #include "cli/cli-utils.h"
>  
> +/* File name for startup style.  */
> +
> +#define STARTUP_FILE "startup-commands"
> +
> +/* Callbacks that will be called to write out startup settings.  */
> +static std::vector<write_startup_setting_ftype *> write_startup_functions;
> +
> +/* True after gdb has read the startup file.  */
> +static bool startup_file_read;
> +
> +/* See cli-setshow.h.  */
> +
> +void
> +write_startup_file ()
> +{
> +  std::string config_dir = get_standard_config_dir ();
> +
> +  if (config_dir.empty ())
> +    {
> +      warning (_("Couldn't determine a path for the startup settings."));
> +      return;
> +    }
> +
> +  if (!mkdir_recursive (config_dir.c_str ()))
> +    {
> +      warning (_("Could not make config directory: %s"),
> +	       safe_strerror (errno));
> +      return;
> +    }
> +
> +  std::string fullname = config_dir + "/" + STARTUP_FILE;
> +  stdio_file outfile;
> +
> +  if (!outfile.open (fullname.c_str (), FOPEN_WT))
> +    perror_with_name (fullname.c_str ());
> +
> +  for (auto &callback : write_startup_functions)
> +    callback (&outfile);
> +}
> +
> +/* See cli-setshow.h.  */
> +
> +void
> +add_startup_writer (write_startup_setting_ftype *callback)
> +{
> +  write_startup_functions.push_back (callback);
> +}

I wonder if, as more options are added to this system, we will find
lots of similar code where we add a set/show option then add a writer
callback, where the callback just prints 'set blah value' to the file.

I'd like to propose the patch below.  This applies on top of this
entire series.  What the patch does is first, carry the
cmd_list_element pointer around along with the callback function.
Then a new default callback is added that uses the cmd_list_element to
build the string 'set blah value'.  In some cases this will remove the
need to write a custom callback function.

What do you think?  Feel free to integrate this with your series, or
not, as you see fit.

Thanks,
Andrew

---

commit 65768ef778a8f1203e8075acab139a523728c586
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Thu Aug 27 17:25:20 2020 +0100

    gdb: introduce default_startup_writer_callback
    
    A default function that can be used as the writer callback for options
    that can be added to the startup file.  This default builds the
    command string from the cmd_list_element and should reduce the amount
    of boilerplate code as more options are added to the startup config
    system.
    
    gdb/ChangeLog:
    
            * cli/cli-setshow.c (write_startup_functions): Change type.
            (write_startup_file): Update walk of write_startup_functions.
            (default_startup_writer_callback): New function.
            (add_default_startup_writer): New function.
            * cli/cli-setshow.h (write_startup_setting_ftype): Update typedef.
            (add_startup_writer): Add new parameter with default value.
            (add_default_startup_writer): Declare new function.
            * cli/cli-style.c (_initialize_cli_style): Update function
            signature for lambda function.
            * top.c (write_startup_quietly): Delete.
            (init_main): Use add_default_startup_writer.

diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index 52838bb9aed..bcb7b19fdca 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -36,7 +36,8 @@
 #define STARTUP_FILE "startup-commands"
 
 /* Callbacks that will be called to write out startup settings.  */
-static std::vector<write_startup_setting_ftype *> write_startup_functions;
+static std::vector<std::pair<write_startup_setting_ftype *,
+			     const cmd_list_element *>> write_startup_functions;
 
 /* True after gdb has read the startup file.  */
 static bool startup_file_read;
@@ -71,16 +72,45 @@ write_startup_file ()
 # This file is written by gdb whenever the relevant settings are changed.\n\
 # Any edits you make here may be overwritten by gdb.\n");
 
-  for (auto &callback : write_startup_functions)
-    callback (&outfile);
+  for (auto &callback_and_cmd : write_startup_functions)
+    callback_and_cmd.first (&outfile, callback_and_cmd.second);
+}
+
+/* A default callback that can be used to write the value of CMD into the
+   startup configuration file.  All the required information is extracted
+   from CMD and the result written to OUTFILE.  */
+
+static void
+default_startup_writer_callback (ui_file *outfile,
+				 const cmd_list_element *cmd)
+{
+  std::string str = "set ";
+  if (cmd->prefixname != nullptr)
+    str += cmd->prefixname;
+  gdb_assert (cmd->name != nullptr);
+  str += cmd->name;
+  str += " ";
+  str += get_setshow_command_value_string (cmd);
+  str += "\n";
+  fputs_unfiltered (str.c_str (), outfile);
+}
+
+/* See cli-setshow.h.  */
+
+void
+add_startup_writer (write_startup_setting_ftype *callback,
+		    const cmd_list_element *cmd)
+{
+  write_startup_functions.emplace_back (callback, cmd);
 }
 
 /* See cli-setshow.h.  */
 
 void
-add_startup_writer (write_startup_setting_ftype *callback)
+add_default_startup_writer (const cmd_list_element *cmd)
 {
-  write_startup_functions.push_back (callback);
+  gdb_assert (cmd != nullptr);
+  add_startup_writer (default_startup_writer_callback, cmd);
 }
 
 /* See cli-setshow.h.  */
diff --git a/gdb/cli/cli-setshow.h b/gdb/cli/cli-setshow.h
index 35e229022ff..6c828897aeb 100644
--- a/gdb/cli/cli-setshow.h
+++ b/gdb/cli/cli-setshow.h
@@ -71,12 +71,19 @@ extern void write_startup_file ();
    startup file.  */
 
 class ui_file;
-typedef void write_startup_setting_ftype (ui_file *);
+typedef void write_startup_setting_ftype (ui_file *, const cmd_list_element *);
 
 /* Add a callback function that will be called when writing the
    startup sequence.  */
 
-extern void add_startup_writer (write_startup_setting_ftype *callback);
+extern void add_startup_writer (write_startup_setting_ftype *callback,
+				const cmd_list_element *cmd = nullptr);
+
+/* Add the default callback function that will be called when writing the
+   startup sequence.  The default callback builds a string to set the
+   option from CMD.  */
+
+extern void add_default_startup_writer (const cmd_list_element *cmd);
 
 /* Read the startup file.  This should only be called by the gdb
    startup sequence.  */
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index 8486d22ecd9..c5842023223 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -407,7 +407,7 @@ Configure colors used in some startup text."),
 				      &style_set_list, &style_show_list,
 				      false);
   /* Ensure that the startup style is written to the startup file.  */
-  add_startup_writer ([] (ui_file *outfile)
+  add_startup_writer ([] (ui_file *outfile, const cmd_list_element *cmd)
     {
       startup_style.write (outfile);
     });
diff --git a/gdb/top.c b/gdb/top.c
index a135c585e09..eb2af926eee 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -2217,15 +2217,6 @@ check_quiet_mode ()
   return startup_quiet;
 }
 
-/* Write "set startup-quietly" to the file.  */
-
-static void
-write_startup_quietly (ui_file *outfile)
-{
-  fprintf_unfiltered (outfile, "set startup-quietly %s\n",
-		      startup_quiet ? "on" : "off");
-}
-
 /* Set the startup-quiet flag.  */
 
 static void
@@ -2396,16 +2387,16 @@ input settings."),
                         show_interactive_mode,
                         &setlist, &showlist);
 
-  add_setshow_boolean_cmd ("startup-quietly", class_support,
-			   &startup_quiet, _("\
+  c = add_setshow_boolean_cmd ("startup-quietly", class_support,
+			       &startup_quiet, _("\
 Set whether GDB should start up quietly."), _("\
 Show whether GDB should start up quietly."), NULL,
-			   set_startup_quiet,
-			   show_startup_quiet,
-			   &setlist, &showlist);
+			       set_startup_quiet,
+			       show_startup_quiet,
+			       &setlist, &showlist);
   /* Arrange to write "set startup-quietly" to the early startup
      file.  */
-  add_startup_writer (write_startup_quietly);
+  add_default_startup_writer (c);
 
   c = add_cmd ("new-ui", class_support, new_ui_command, _("\
 Create a new UI.\n\