[v2,5/7] Add early startup command file

Message ID 20200623132006.15863-6-tom@tromey.com
State New
Headers show
Series
  • Some user-friendliness changes
Related show

Commit Message

Tom Tromey June 23, 2020, 1:20 p.m.
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(+)

-- 
2.17.2

Comments

Tom Tromey July 5, 2020, 6:51 p.m. | #1
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:


Tom> This adds support for a gdb command file to be read early in startup.
Tom> Code that wants to save an early setting can register a callback to be
Tom> called to write to this file.  This code is not yet used, but will be
Tom> in subsequent patches.

Tom> +/* See cli-setshow.h.  */
Tom> +
Tom> +void
Tom> +write_startup_file ()
Tom> +{
[...]
Tom> +
Tom> +  for (auto &callback : write_startup_functions)
Tom> +    callback (&outfile);

I'm going to change this to add a comment to the file as well.  The
comment will say something about how the file is generated by gdb and
that its contents may be ovewritten.

Tom
Andrew Burgess Aug. 26, 2020, 3:47 p.m. | #2
* 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.


I think that this patch needs some documentation in, at least, the
'@node Startup' section of the manual.

Also I think that if this config file is going to be generated as
~/.config/gdb/... then we should allow the gdbinit file to be moved
into that directory too, though obviously this would be a follow up
patch.

Thanks,
Andrew



> ---

>  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);

> +}

> +

> +/* See cli-setshow.h.  */

> +

> +void

> +read_startup_file ()

> +{

> +  std::string config_dir = get_standard_config_dir ();

> +

> +  if (!config_dir.empty ())

> +    {

> +      std::string filename = config_dir + "/" + STARTUP_FILE;

> +      try

> +	{

> +	  source_script (filename.c_str (), 1);

> +	  /* If reading fails, we don't want to write the file -- it

> +	     might overwrite something.  So, we set this flag after

> +	     reading.  */

> +	  startup_file_read = true;

> +	}

> +      catch (const gdb_exception &)

> +	{

> +	  /* Ignore errors.  */

> +	}

> +    }

> +}

> +

> +

>  /* Return true if the change of command parameter should be notified.  */

>  

>  static int

> diff --git a/gdb/cli/cli-setshow.h b/gdb/cli/cli-setshow.h

> index 83e4984ed6c..35e229022ff 100644

> --- a/gdb/cli/cli-setshow.h

> +++ b/gdb/cli/cli-setshow.h

> @@ -62,4 +62,25 @@ extern std::string get_setshow_command_value_string (const cmd_list_element *c);

>  

>  extern void cmd_show_list (struct cmd_list_element *list, int from_tty);

>  

> +/* Write the file of gdb "set" commands that is read early in the

> +   startup sequence.  */

> +

> +extern void write_startup_file ();

> +

> +/* The type of a callback function that is used when writing the

> +   startup file.  */

> +

> +class ui_file;

> +typedef void write_startup_setting_ftype (ui_file *);

> +

> +/* Add a callback function that will be called when writing the

> +   startup sequence.  */

> +

> +extern void add_startup_writer (write_startup_setting_ftype *callback);

> +

> +/* Read the startup file.  This should only be called by the gdb

> +   startup sequence.  */

> +

> +extern void read_startup_file ();

> +

>  #endif /* CLI_CLI_SETSHOW_H */

> diff --git a/gdb/main.c b/gdb/main.c

> index 3649e4a2201..bd1f6d6b2c5 100644

> --- a/gdb/main.c

> +++ b/gdb/main.c

> @@ -35,6 +35,7 @@

>  #include "main.h"

>  #include "source.h"

>  #include "cli/cli-cmds.h"

> +#include "cli/cli-setshow.h"

>  #include "objfiles.h"

>  #include "auto-load.h"

>  #include "maint.h"

> @@ -933,6 +934,10 @@ captured_main_1 (struct captured_main_args *context)

>    /* Initialize all files.  */

>    gdb_init (gdb_program_name);

>  

> +  /* Set the startup style.  */

> +  if (!inhibit_gdbinit && !inhibit_home_gdbinit)

> +    read_startup_file ();

> +

>    /* Now that gdb_init has created the initial inferior, we're in

>       position to set args for that inferior.  */

>    if (set_args)

> -- 

> 2.17.2

>
Andrew Burgess Aug. 27, 2020, 4:32 p.m. | #3
* 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\

Patch

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);
+}
+
+/* See cli-setshow.h.  */
+
+void
+read_startup_file ()
+{
+  std::string config_dir = get_standard_config_dir ();
+
+  if (!config_dir.empty ())
+    {
+      std::string filename = config_dir + "/" + STARTUP_FILE;
+      try
+	{
+	  source_script (filename.c_str (), 1);
+	  /* If reading fails, we don't want to write the file -- it
+	     might overwrite something.  So, we set this flag after
+	     reading.  */
+	  startup_file_read = true;
+	}
+      catch (const gdb_exception &)
+	{
+	  /* Ignore errors.  */
+	}
+    }
+}
+
+
 /* Return true if the change of command parameter should be notified.  */
 
 static int
diff --git a/gdb/cli/cli-setshow.h b/gdb/cli/cli-setshow.h
index 83e4984ed6c..35e229022ff 100644
--- a/gdb/cli/cli-setshow.h
+++ b/gdb/cli/cli-setshow.h
@@ -62,4 +62,25 @@  extern std::string get_setshow_command_value_string (const cmd_list_element *c);
 
 extern void cmd_show_list (struct cmd_list_element *list, int from_tty);
 
+/* Write the file of gdb "set" commands that is read early in the
+   startup sequence.  */
+
+extern void write_startup_file ();
+
+/* The type of a callback function that is used when writing the
+   startup file.  */
+
+class ui_file;
+typedef void write_startup_setting_ftype (ui_file *);
+
+/* Add a callback function that will be called when writing the
+   startup sequence.  */
+
+extern void add_startup_writer (write_startup_setting_ftype *callback);
+
+/* Read the startup file.  This should only be called by the gdb
+   startup sequence.  */
+
+extern void read_startup_file ();
+
 #endif /* CLI_CLI_SETSHOW_H */
diff --git a/gdb/main.c b/gdb/main.c
index 3649e4a2201..bd1f6d6b2c5 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -35,6 +35,7 @@ 
 #include "main.h"
 #include "source.h"
 #include "cli/cli-cmds.h"
+#include "cli/cli-setshow.h"
 #include "objfiles.h"
 #include "auto-load.h"
 #include "maint.h"
@@ -933,6 +934,10 @@  captured_main_1 (struct captured_main_args *context)
   /* Initialize all files.  */
   gdb_init (gdb_program_name);
 
+  /* Set the startup style.  */
+  if (!inhibit_gdbinit && !inhibit_home_gdbinit)
+    read_startup_file ();
+
   /* Now that gdb_init has created the initial inferior, we're in
      position to set args for that inferior.  */
   if (set_args)