[PATCH v2 0/7] Some user-friendliness changes

[PATCH v2 0/7] Some user-friendliness changes

[Tom Tromey]

Here is v2 of my "user-friendliness" patch series.  I've added docs
and some minimal tests.

v1 is here:
https://sourceware.org/pipermail/gdb-patches/2020-April/167350.html

In this version, I reimplemented "set style startup" and
"set startup-quietly" to both use a generic facility to store
some parameters in an early startup file.  This makes it simpler
to add more such settings, should we want to.

Let me know what you think.

Tom

[PATCH v2 1/7] Introduce read_entire_file

[Tom Tromey]

This adds a new function, read_entire_file, and changes the source
cache to use it.  This will be used in a subsequent patch as well.

gdb/ChangeLog
2020-06-22  Tom Tromey  <tom@tromey.com>

	* utils.h (myread): Don't declare.
	* utils.c (myread): Remove.
	* source-cache.c (source_cache::get_plain_source_lines): Use
	read_entire_file.

gdbsupport/ChangeLog
2020-06-22  Tom Tromey  <tom@tromey.com>

	* filestuff.h (read_entire_file): Declare.
	* filestuff.cc (read_entire_file): New function.
---
 gdb/ChangeLog           |  7 +++++++
 gdb/source-cache.c      | 14 +++++---------
 gdb/utils.c             | 22 ----------------------
 gdb/utils.h             |  2 --
 gdbsupport/ChangeLog    |  5 +++++
 gdbsupport/filestuff.cc | 33 +++++++++++++++++++++++++++++++++
 gdbsupport/filestuff.h  |  9 +++++++++
 7 files changed, 59 insertions(+), 33 deletions(-)

diff --git a/gdb/source-cache.c b/gdb/source-cache.c
index 9196e3a19e3..63e08c3aeb0 100644
--- a/gdb/source-cache.c
+++ b/gdb/source-cache.c
@@ -18,6 +18,7 @@
 
 #include "defs.h"
 #include "source-cache.h"
+#include "gdbsupport/filestuff.h"
 #include "gdbsupport/scoped_fd.h"
 #include "source.h"
 #include "cli/cli-style.h"
@@ -56,14 +57,9 @@ source_cache::get_plain_source_lines (struct symtab *s,
   if (desc.get () < 0)
     perror_with_name (symtab_to_filename_for_display (s));
 
-  struct stat st;
-  if (fstat (desc.get (), &st) < 0)
-    perror_with_name (symtab_to_filename_for_display (s));
-
-  std::string lines;
-  lines.resize (st.st_size);
-  if (myread (desc.get (), &lines[0], lines.size ()) < 0)
-    perror_with_name (symtab_to_filename_for_display (s));
+  time_t file_mtime;
+  std::string lines = read_entire_file (symtab_to_filename_for_display (s),
+					desc.get (), &file_mtime);
 
   time_t mtime = 0;
   if (SYMTAB_OBJFILE (s) != NULL && SYMTAB_OBJFILE (s)->obfd != NULL)
@@ -71,7 +67,7 @@ source_cache::get_plain_source_lines (struct symtab *s,
   else if (exec_bfd)
     mtime = exec_bfd_mtime;
 
-  if (mtime && mtime < st.st_mtime)
+  if (mtime && mtime < file_mtime)
     warning (_("Source file is more recent than executable."));
 
   std::vector<off_t> offsets;
diff --git a/gdb/utils.c b/gdb/utils.c
index 102db28787f..023adfae066 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -687,28 +687,6 @@ flush_streams ()
   gdb_stderr->flush ();
 }
 
-/* My replacement for the read system call.
-   Used like `read' but keeps going if `read' returns too soon.  */
-
-int
-myread (int desc, char *addr, int len)
-{
-  int val;
-  int orglen = len;
-
-  while (len > 0)
-    {
-      val = read (desc, addr, len);
-      if (val < 0)
-	return val;
-      if (val == 0)
-	return orglen - len;
-      len -= val;
-      addr += val;
-    }
-  return orglen;
-}
-
 void
 print_spaces (int n, struct ui_file *file)
 {
diff --git a/gdb/utils.h b/gdb/utils.h
index 3434ff1caa2..d0989204128 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -540,8 +540,6 @@ void dummy_obstack_deallocate (void *object, void *data);
 extern pid_t wait_to_die_with_timeout (pid_t pid, int *status, int timeout);
 #endif
 
-extern int myread (int, char *, int);
-
 /* Resource limits used by getrlimit and setrlimit.  */
 
 enum resource_limit_kind
diff --git a/gdbsupport/filestuff.cc b/gdbsupport/filestuff.cc
index 179c4258918..0f0a238beb0 100644
--- a/gdbsupport/filestuff.cc
+++ b/gdbsupport/filestuff.cc
@@ -501,3 +501,36 @@ mkdir_recursive (const char *dir)
       component_start = component_end;
     }
 }
+
+/* See filestuff.h.  */
+
+std::string
+read_entire_file (const char *name, int fd, time_t *mtime)
+{
+  struct stat st;
+  if (fstat (fd, &st) < 0)
+    perror_with_name (name);
+  size_t len = st.st_size;
+
+  std::string lines;
+  lines.resize (len);
+  char *addr = &lines[0];
+
+  while (len > 0)
+    {
+      int val = read (fd, addr, len);
+      if (val < 0)
+	perror_with_name (name);
+      if (val == 0)
+	break;
+      len -= val;
+      addr += val;
+    }
+
+  if (mtime != nullptr)
+    *mtime = st.st_mtime;
+
+  /* Just in case we really did end up short.  */
+  lines.resize (st.st_size - len);
+  return lines;
+}
diff --git a/gdbsupport/filestuff.h b/gdbsupport/filestuff.h
index e36936a84ce..84924f462e2 100644
--- a/gdbsupport/filestuff.h
+++ b/gdbsupport/filestuff.h
@@ -139,4 +139,13 @@ extern bool is_regular_file (const char *name, int *errno_ptr);
 
 extern bool mkdir_recursive (const char *dir);
 
+/* Read and return the entire contents of a file.  The file must
+   already be open on FD.  NAME is the file name it is used when
+   reporting errors, which is done by throwing an exception.  The
+   mtime of the file is optionally stored in *MTIME; if MTIME is
+   nullptr, this is ignored.  */
+
+extern std::string read_entire_file (const char *name, int fd,
+				     time_t *mtime = nullptr);
+
 #endif /* COMMON_FILESTUFF_H */
-- 
2.17.2

Re: [PATCH v2 1/7] Introduce read_entire_file

[Simon Marchi]

On 2020-06-23 9:20 a.m., Tom Tromey wrote:
> +std::string
> +read_entire_file (const char *name, int fd, time_t *mtime)
> +{
> +  struct stat st;
> +  if (fstat (fd, &st) < 0)
> +    perror_with_name (name);
> +  size_t len = st.st_size;
> +
> +  std::string lines;
> +  lines.resize (len);
> +  char *addr = &lines[0];
> +
> +  while (len > 0)
> +    {
> +      int val = read (fd, addr, len);

Should we handle EINTR here?

> +      if (val < 0)
> +	perror_with_name (name);
> +      if (val == 0)
> +	break;
> +      len -= val;
> +      addr += val;
> +    }

Would you mind adding newlines around the `if` statements?  Maybe it's
just me, but I find it really more difficult to follow the code when it's
all packed.  I would mean something like:

      int val = read (fd, addr, len);

      if (val < 0)
	perror_with_name (name);

      if (val == 0)
	break;

      len -= val;
      addr += val;

Like this, my mind is more easily capable of seeing the logical steps.

Simon

Re: [PATCH v2 1/7] Introduce read_entire_file

[Andrew Burgess]

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

> This adds a new function, read_entire_file, and changes the source
> cache to use it.  This will be used in a subsequent patch as well.
> 
> gdb/ChangeLog
> 2020-06-22  Tom Tromey  <tom@tromey.com>
> 
> 	* utils.h (myread): Don't declare.
> 	* utils.c (myread): Remove.
> 	* source-cache.c (source_cache::get_plain_source_lines): Use
> 	read_entire_file.
> 
> gdbsupport/ChangeLog
> 2020-06-22  Tom Tromey  <tom@tromey.com>
> 
> 	* filestuff.h (read_entire_file): Declare.
> 	* filestuff.cc (read_entire_file): New function.

LGTM.

Thanks,
Andrew


> ---
>  gdb/ChangeLog           |  7 +++++++
>  gdb/source-cache.c      | 14 +++++---------
>  gdb/utils.c             | 22 ----------------------
>  gdb/utils.h             |  2 --
>  gdbsupport/ChangeLog    |  5 +++++
>  gdbsupport/filestuff.cc | 33 +++++++++++++++++++++++++++++++++
>  gdbsupport/filestuff.h  |  9 +++++++++
>  7 files changed, 59 insertions(+), 33 deletions(-)
> 
> diff --git a/gdb/source-cache.c b/gdb/source-cache.c
> index 9196e3a19e3..63e08c3aeb0 100644
> --- a/gdb/source-cache.c
> +++ b/gdb/source-cache.c
> @@ -18,6 +18,7 @@
>  
>  #include "defs.h"
>  #include "source-cache.h"
> +#include "gdbsupport/filestuff.h"
>  #include "gdbsupport/scoped_fd.h"
>  #include "source.h"
>  #include "cli/cli-style.h"
> @@ -56,14 +57,9 @@ source_cache::get_plain_source_lines (struct symtab *s,
>    if (desc.get () < 0)
>      perror_with_name (symtab_to_filename_for_display (s));
>  
> -  struct stat st;
> -  if (fstat (desc.get (), &st) < 0)
> -    perror_with_name (symtab_to_filename_for_display (s));
> -
> -  std::string lines;
> -  lines.resize (st.st_size);
> -  if (myread (desc.get (), &lines[0], lines.size ()) < 0)
> -    perror_with_name (symtab_to_filename_for_display (s));
> +  time_t file_mtime;
> +  std::string lines = read_entire_file (symtab_to_filename_for_display (s),
> +					desc.get (), &file_mtime);
>  
>    time_t mtime = 0;
>    if (SYMTAB_OBJFILE (s) != NULL && SYMTAB_OBJFILE (s)->obfd != NULL)
> @@ -71,7 +67,7 @@ source_cache::get_plain_source_lines (struct symtab *s,
>    else if (exec_bfd)
>      mtime = exec_bfd_mtime;
>  
> -  if (mtime && mtime < st.st_mtime)
> +  if (mtime && mtime < file_mtime)
>      warning (_("Source file is more recent than executable."));
>  
>    std::vector<off_t> offsets;
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 102db28787f..023adfae066 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -687,28 +687,6 @@ flush_streams ()
>    gdb_stderr->flush ();
>  }
>  
> -/* My replacement for the read system call.
> -   Used like `read' but keeps going if `read' returns too soon.  */
> -
> -int
> -myread (int desc, char *addr, int len)
> -{
> -  int val;
> -  int orglen = len;
> -
> -  while (len > 0)
> -    {
> -      val = read (desc, addr, len);
> -      if (val < 0)
> -	return val;
> -      if (val == 0)
> -	return orglen - len;
> -      len -= val;
> -      addr += val;
> -    }
> -  return orglen;
> -}
> -
>  void
>  print_spaces (int n, struct ui_file *file)
>  {
> diff --git a/gdb/utils.h b/gdb/utils.h
> index 3434ff1caa2..d0989204128 100644
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -540,8 +540,6 @@ void dummy_obstack_deallocate (void *object, void *data);
>  extern pid_t wait_to_die_with_timeout (pid_t pid, int *status, int timeout);
>  #endif
>  
> -extern int myread (int, char *, int);
> -
>  /* Resource limits used by getrlimit and setrlimit.  */
>  
>  enum resource_limit_kind
> diff --git a/gdbsupport/filestuff.cc b/gdbsupport/filestuff.cc
> index 179c4258918..0f0a238beb0 100644
> --- a/gdbsupport/filestuff.cc
> +++ b/gdbsupport/filestuff.cc
> @@ -501,3 +501,36 @@ mkdir_recursive (const char *dir)
>        component_start = component_end;
>      }
>  }
> +
> +/* See filestuff.h.  */
> +
> +std::string
> +read_entire_file (const char *name, int fd, time_t *mtime)
> +{
> +  struct stat st;
> +  if (fstat (fd, &st) < 0)
> +    perror_with_name (name);
> +  size_t len = st.st_size;
> +
> +  std::string lines;
> +  lines.resize (len);
> +  char *addr = &lines[0];
> +
> +  while (len > 0)
> +    {
> +      int val = read (fd, addr, len);
> +      if (val < 0)
> +	perror_with_name (name);
> +      if (val == 0)
> +	break;
> +      len -= val;
> +      addr += val;
> +    }
> +
> +  if (mtime != nullptr)
> +    *mtime = st.st_mtime;
> +
> +  /* Just in case we really did end up short.  */
> +  lines.resize (st.st_size - len);
> +  return lines;
> +}
> diff --git a/gdbsupport/filestuff.h b/gdbsupport/filestuff.h
> index e36936a84ce..84924f462e2 100644
> --- a/gdbsupport/filestuff.h
> +++ b/gdbsupport/filestuff.h
> @@ -139,4 +139,13 @@ extern bool is_regular_file (const char *name, int *errno_ptr);
>  
>  extern bool mkdir_recursive (const char *dir);
>  
> +/* Read and return the entire contents of a file.  The file must
> +   already be open on FD.  NAME is the file name it is used when
> +   reporting errors, which is done by throwing an exception.  The
> +   mtime of the file is optionally stored in *MTIME; if MTIME is
> +   nullptr, this is ignored.  */
> +
> +extern std::string read_entire_file (const char *name, int fd,
> +				     time_t *mtime = nullptr);
> +
>  #endif /* COMMON_FILESTUFF_H */
> -- 
> 2.17.2
> 

[PATCH v2 2/7] Add "help news"

[Tom Tromey]

This adds a "help news" subcommand, which simply dumps the NEWS file.
The NEWS file is now installed.

gdb/ChangeLog
2020-06-22  Tom Tromey  <tom@tromey.com>

	* NEWS: Add entry.
	* data-directory/Makefile.in (GDB_FILES): New variable.
	(all): Add gdb-files.
	(gdb-files, install-gdb-files, uninstall-gdb-files): New targets.
	(install-only, uninstall): Update.
	* cli/cli-decode.c (help_news): New file.
	(help_cmd): Handle "news".
	(help_list): Mention "help news".

gdb/doc/ChangeLog
2020-06-22  Tom Tromey  <tom@tromey.com>

	* gdb.texinfo (Help): Mention help news.
---
 gdb/ChangeLog                  | 11 +++++++++
 gdb/NEWS                       |  3 +++
 gdb/cli/cli-decode.c           | 41 ++++++++++++++++++++++++++++++++++
 gdb/data-directory/Makefile.in | 28 ++++++++++++++++++++---
 gdb/doc/ChangeLog              |  4 ++++
 gdb/doc/gdb.texinfo            |  4 ++++
 6 files changed, 88 insertions(+), 3 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index f7585133c51..0fd39857326 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -65,6 +65,9 @@
 
 * New commands
 
+help news
+  Show this NEWS file.
+
 set exec-file-mismatch -- Set exec-file-mismatch handling (ask|warn|off).
 show exec-file-mismatch -- Show exec-file-mismatch handling (ask|warn|off).
   Set or show the option 'exec-file-mismatch'.  When GDB attaches to a
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 85f50aa8e48..c986a1c45fb 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1197,6 +1197,38 @@ apropos_cmd (struct ui_file *stream,
     }
 }
 
+/* Implement "help news".  */
+
+static void
+help_news (struct ui_file *stream)
+{
+  std::string news_name = std::string (gdb_datadir) + SLASH_STRING + "NEWS";
+  gdb_file_up news_file = gdb_fopen_cloexec (news_name.c_str (), "r");
+  if (news_file == nullptr)
+    perror_with_name (_("could not open the NEWS file"));
+
+  char buffer[1024];
+  size_t offset = 0;
+  while (true)
+    {
+      size_t nbytes = fread (&buffer[offset], 1, sizeof (buffer) - offset,
+			     news_file.get ());
+      if (nbytes == 0)
+	break;
+      size_t n_valid = offset + nbytes;
+      size_t newline;
+      for (newline = n_valid; newline > 0 && buffer[newline] != '\n'; --newline)
+	;
+      if (newline == 0)
+	error (_("NEWS file is malformed"));
+      buffer[newline] = '\0';
+      fputs_filtered (buffer, stream);
+      fputs_filtered ("\n", stream);
+      offset = n_valid - (newline + 1);
+      memmove (buffer, &buffer[newline + 1], offset);
+    }
+}
+
 /* This command really has to deal with two things:
    1) I want documentation on *this string* (usually called by
       "help commandname").
@@ -1225,6 +1257,12 @@ help_cmd (const char *command, struct ui_file *stream)
       return;
     }
 
+  if (strcmp (command, "news") == 0)
+    {
+      help_news (stream);
+      return;
+    }
+
   const char *orig_command = command;
   c = lookup_cmd (&command, cmdlist, "", NULL, 0, 0);
 
@@ -1330,6 +1368,9 @@ Type \"help%s\" followed by a class name for a list of commands in ",
 
       fprintf_filtered (stream, "\n\
 Type \"help all\" for the list of all commands.");
+
+      fprintf_filtered (stream, "\n\
+Type \"help news\" to see what is new in GDB.");
     }
 
   fprintf_filtered (stream, "\nType \"help%s\" followed by %scommand name ",
diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index 3f0c729404b..85a2f41c351 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -139,6 +139,8 @@ SYSTEM_GDBINIT_FILES = \
 	elinos.py \
 	wrs-linux.py
 
+GDB_FILES = NEWS
+
 FLAGS_TO_PASS = \
 	"prefix=$(prefix)" \
 	"exec_prefix=$(exec_prefix)" \
@@ -172,7 +174,7 @@ FLAGS_TO_PASS = \
 	"RUNTESTFLAGS=$(RUNTESTFLAGS)"
 
 .PHONY: all
-all: stamp-syscalls stamp-python stamp-guile stamp-system-gdbinit
+all: stamp-syscalls stamp-python stamp-guile stamp-system-gdbinit gdb-files
 
 %.xml: @MAINTAINER_MODE_TRUE@ %.xml.in apply-defaults.xsl linux-defaults.xml.in
 	$(XSLTPROC) -o $(SYSCALLS_SRCDIR)/$@ $(SYSCALLS_SRCDIR)/apply-defaults.xsl\
@@ -234,6 +236,26 @@ uninstall-syscalls:
 	  done \
 	done
 
+gdb-files: $(addprefix $(srcdir)/../,$(GDB_FILES))
+	files='$(GDB_FILES)'; \
+	for file in $$files; do \
+	  cp $(srcdir)/../$$file .; \
+	done
+
+.PHONY: install-gdb-files
+install-gdb-files:
+	files='$(GDB_FILES)'; \
+	for file in $$files; do \
+	  $(INSTALL_DATA) $(srcdir)/../$$file  $(DESTDIR)$(GDB_DATADIR); \
+	done
+
+.PHONY: uninstall-gdb-files
+uninstall-gdb-files:
+	files='$(GDB_FILES)'; \
+	for file in $$files; do \
+	  rm -f $(DESTDIR)$(GDB_DATADIR)/$$file; \
+	done
+
 stamp-python: Makefile $(PYTHON_FILES)
 	rm -rf ./$(PYTHON_DIR)
 	files='$(PYTHON_FILES)' ; \
@@ -379,11 +401,11 @@ install: all
 
 .PHONY: install-only
 install-only: install-syscalls install-python install-guile \
-	install-system-gdbinit
+	install-system-gdbinit install-news
 
 .PHONY: uninstall
 uninstall: uninstall-syscalls uninstall-python uninstall-guile \
-	uninstall-system-gdbinit
+	uninstall-system-gdbinit uninstall-news
 
 .PHONY: clean
 clean: clean-syscalls clean-python clean-guile clean-system-gdbinit
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7f572c37c5c..37d5fe1e4cd 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2163,6 +2163,10 @@ the command name and all its aliases separated by commas.
 This first line will be followed by the full definition of all aliases
 having default arguments.
 
+@item help news
+Show the news file.  Notable features in each release are documented
+in this file.
+
 @kindex apropos
 @item apropos [-v] @var{regexp}
 The @code{apropos} command searches through all of the @value{GDBN}
-- 
2.17.2

Re: [PATCH v2 2/7] Add "help news"

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Date: Tue, 23 Jun 2020 07:20:01 -0600
> Cc: Tom Tromey <tom@tromey.com>
> 
> This adds a "help news" subcommand, which simply dumps the NEWS file.
> The NEWS file is now installed.
> 
> gdb/ChangeLog
> 2020-06-22  Tom Tromey  <tom@tromey.com>
> 
> 	* NEWS: Add entry.
> 	* data-directory/Makefile.in (GDB_FILES): New variable.
> 	(all): Add gdb-files.
> 	(gdb-files, install-gdb-files, uninstall-gdb-files): New targets.
> 	(install-only, uninstall): Update.
> 	* cli/cli-decode.c (help_news): New file.
> 	(help_cmd): Handle "news".
> 	(help_list): Mention "help news".
> 
> gdb/doc/ChangeLog
> 2020-06-22  Tom Tromey  <tom@tromey.com>
> 
> 	* gdb.texinfo (Help): Mention help news.
> ---
>  gdb/ChangeLog                  | 11 +++++++++
>  gdb/NEWS                       |  3 +++
>  gdb/cli/cli-decode.c           | 41 ++++++++++++++++++++++++++++++++++
>  gdb/data-directory/Makefile.in | 28 ++++++++++++++++++++---
>  gdb/doc/ChangeLog              |  4 ++++
>  gdb/doc/gdb.texinfo            |  4 ++++
>  6 files changed, 88 insertions(+), 3 deletions(-)

OK for the documentation parts.

Thanks.

Re: [PATCH v2 2/7] Add "help news"

[Christian Biesinger]

On Tue, Jun 23, 2020 at 8:20 AM Tom Tromey <tom@tromey.com> wrote:
> +static void
> +help_news (struct ui_file *stream)
> +{
> +  std::string news_name = std::string (gdb_datadir) + SLASH_STRING + "NEWS";
> +  gdb_file_up news_file = gdb_fopen_cloexec (news_name.c_str (), "r");
> +  if (news_file == nullptr)
> +    perror_with_name (_("could not open the NEWS file"));
> +
> +  char buffer[1024];
> +  size_t offset = 0;
> +  while (true)
> +    {
> +      size_t nbytes = fread (&buffer[offset], 1, sizeof (buffer) - offset,
> +                            news_file.get ());


Why not use read_entire_file from your other patch?

Christian

Re: [PATCH v2 2/7] Add "help news"

[Tom Tromey]

>>>>> "Christian" == Christian Biesinger via Gdb-patches <gdb-patches@sourceware.org> writes:

Christian> On Tue, Jun 23, 2020 at 8:20 AM Tom Tromey <tom@tromey.com> wrote:
>> +static void
>> +help_news (struct ui_file *stream)
>> +{
>> +  std::string news_name = std::string (gdb_datadir) + SLASH_STRING + "NEWS";
>> +  gdb_file_up news_file = gdb_fopen_cloexec (news_name.c_str (), "r");
>> +  if (news_file == nullptr)
>> +    perror_with_name (_("could not open the NEWS file"));
>> +
>> +  char buffer[1024];
>> +  size_t offset = 0;
>> +  while (true)
>> +    {
>> +      size_t nbytes = fread (&buffer[offset], 1, sizeof (buffer) - offset,
>> +                            news_file.get ());


Christian> Why not use read_entire_file from your other patch?

The NEWS file seems large-ish, and I figured the normal thing for users
would be to use the pager to view the top, then stop reading.

Tom

Re: [PATCH v2 2/7] Add "help news"

[Simon Marchi]

On 2020-07-05 11:59 a.m., Tom Tromey wrote:
>>>>>> "Christian" == Christian Biesinger via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Christian> On Tue, Jun 23, 2020 at 8:20 AM Tom Tromey <tom@tromey.com> wrote:
>>> +static void
>>> +help_news (struct ui_file *stream)
>>> +{
>>> +  std::string news_name = std::string (gdb_datadir) + SLASH_STRING + "NEWS";
>>> +  gdb_file_up news_file = gdb_fopen_cloexec (news_name.c_str (), "r");
>>> +  if (news_file == nullptr)
>>> +    perror_with_name (_("could not open the NEWS file"));
>>> +
>>> +  char buffer[1024];
>>> +  size_t offset = 0;
>>> +  while (true)
>>> +    {
>>> +      size_t nbytes = fread (&buffer[offset], 1, sizeof (buffer) - offset,
>>> +                            news_file.get ());
> 
> 
> Christian> Why not use read_entire_file from your other patch?
> 
> The NEWS file seems large-ish, and I figured the normal thing for users
> would be to use the pager to view the top, then stop reading.

The NEWS file is 281k and contains all NEWS since GDB 4.0 (pre year 2000).  So
I think it's safe to say that it grows much slower than the available memory
on the average computer grows.  That amount in memory is minimal compared to
what is typically used for reading DWARF information of large-ish programs,
and it's just transient use anyway.

So I think it's safe to read it all, and preferable if it makes the code
simpler.  I don't think reading 281k will cause any delay noticeable to the
user of "help news".

Simon

Re: [PATCH v2 2/7] Add "help news"

[Tom Tromey]

[...]
Christian> Why not use read_entire_file from your other patch?

Since Simon also mentioned this, I went ahead and made the change.

Tom

Re: [PATCH v2 2/7] Add "help news"

[Simon Marchi]

On 2020-06-23 9:20 a.m., Tom Tromey wrote:
> This adds a "help news" subcommand, which simply dumps the NEWS file.
> The NEWS file is now installed.

(1) I first tried "help news" with GDB in its build directory without passing
--data-directory, expecting it to fail, just to see how it would fail.  It
says:

  (gdb) help news
  could not open the NEWS file: No such file or directory.

Maybe this message should say which path it's trying to open.  If this
happens, the cause would probably be a broken GDB installation, and it
would be useful to know where is GDB looking for that file.

(2) When trying to "make install", I get this error:

  $ make install DESTDIR=/tmp/install
  ...
  make[4]: *** No rule to make target 'install-news', needed by 'install-only'.  Stop.

Indeed, the install-news target is used but does not exist.

(3) Since we look for the NEWS file at <data-directory>/NEWS, I presume it won't
work when running GDB in its build directory, either with

  ./gdb --data-directory=data-directory

or

  make run

since the NEWS file is not there.  It's not a big deal, but if you think of an easy
way to make it work, it would be nice.  Can we put a symlink from data-directory/NEWS
to NEWS?

(4) I tried what I mentioned just above and I get some sanitizer error:

  $ ./gdb -q -nx --data-directory=data-directory
  (gdb) help news
  /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1214:60: runtime error: index 1024 out of bounds for type 'char [1024]'

Not sure if it's ASan or UBSan.

Simon

Re: [PATCH v2 2/7] Add "help news"

[Simon Marchi]

On 2020-07-06 10:06 a.m., Simon Marchi wrote:
> On 2020-06-23 9:20 a.m., Tom Tromey wrote:
>> This adds a "help news" subcommand, which simply dumps the NEWS file.
>> The NEWS file is now installed.
> 
> (1) I first tried "help news" with GDB in its build directory without passing
> --data-directory, expecting it to fail, just to see how it would fail.  It
> says:
> 
>   (gdb) help news
>   could not open the NEWS file: No such file or directory.
> 
> Maybe this message should say which path it's trying to open.  If this
> happens, the cause would probably be a broken GDB installation, and it
> would be useful to know where is GDB looking for that file.
> 
> (2) When trying to "make install", I get this error:
> 
>   $ make install DESTDIR=/tmp/install
>   ...
>   make[4]: *** No rule to make target 'install-news', needed by 'install-only'.  Stop.
> 
> Indeed, the install-news target is used but does not exist.
> 
> (3) Since we look for the NEWS file at <data-directory>/NEWS, I presume it won't
> work when running GDB in its build directory, either with
> 
>   ./gdb --data-directory=data-directory
> 
> or
> 
>   make run
> 
> since the NEWS file is not there.  It's not a big deal, but if you think of an easy
> way to make it work, it would be nice.  Can we put a symlink from data-directory/NEWS
> to NEWS?
> 
> (4) I tried what I mentioned just above and I get some sanitizer error:
> 
>   $ ./gdb -q -nx --data-directory=data-directory
>   (gdb) help news
>   /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1214:60: runtime error: index 1024 out of bounds for type 'char [1024]'
> 
> Not sure if it's ASan or UBSan.
> 
> Simon
> 

Oh and:

(5) the way "help news" is implemented means the "news" part is not discoverable
through tab-completion.  I'm not super familiar with how commands work, but would
it work if you registered the "help news" command as a separate command?

That's not a deal-breaker, but I thought I would mention it just in case it's easy
to fix.

Simon

Re: [PATCH v2 2/7] Add "help news"

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> (5) the way "help news" is implemented means the "news" part is not discoverable
Simon> through tab-completion.  I'm not super familiar with how commands work, but would
Simon> it work if you registered the "help news" command as a separate command?

I wasn't sure if that would work, and gdb already fails to list "help all"
as a possible completion.  So, in v3 of the series I've added a new
completion function for "help".

Tom

Re: [PATCH v2 2/7] Add "help news"

[Simon Marchi]

On 2020-07-06 10:06 a.m., Simon Marchi wrote:
> (3) Since we look for the NEWS file at <data-directory>/NEWS, I presume it won't
> work when running GDB in its build directory, either with
> 
>   ./gdb --data-directory=data-directory
> 
> or
> 
>   make run
> 
> since the NEWS file is not there.  It's not a big deal, but if you think of an easy
> way to make it work, it would be nice.  Can we put a symlink from data-directory/NEWS
> to NEWS?

Ah scratch that, I see that your Makefile change copies NEWS to the data-directory in
the build directory,  so you already thought about that.

Simon

Re: [PATCH v2 2/7] Add "help news"

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon>   $ make install DESTDIR=/tmp/install
Simon>   ...
Simon>   make[4]: *** No rule to make target 'install-news', needed by 'install-only'.  Stop.

Oops, that's a leftover from an earlier iteration.
I fixed this.

Simon> (4) I tried what I mentioned just above and I get some sanitizer error:

Simon>   $ ./gdb -q -nx --data-directory=data-directory
Simon>   (gdb) help news
Simon>   /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1214:60: runtime error: index 1024 out of bounds for type 'char [1024]'

Simon> Not sure if it's ASan or UBSan.

Probably fixed by using read_entire_file, but I'll check.

Tom

[PATCH v2 3/7] Add "tips" file to gdb

[Tom Tromey]

This adds a "tips" file to gdb.  This file holds handy tips -- right
now there are just a few, but it's easy to add more.  A random tip is
displayed during interactive startup.

gdb/ChangeLog
2020-06-22  Tom Tromey  <tom@tromey.com>

	* NEWS: Add entry.
	* top.c (startup_style): New global.
	(print_tip): New function.
	(print_gdb_version): Update.  Don't print "show configuration"
	or "apropos" info.  Call print_tip.
	* tips: New file.
	* data-directory/Makefile.in (GDB_FILES): Add "tips".

gdb/doc/ChangeLog
2020-06-22  Tom Tromey  <tom@tromey.com>

	* gdb.texinfo (Help): Mention tips.
---
 gdb/ChangeLog                  | 10 +++++
 gdb/NEWS                       |  2 +
 gdb/data-directory/Makefile.in |  2 +-
 gdb/doc/ChangeLog              |  4 ++
 gdb/doc/gdb.texinfo            |  3 ++
 gdb/tips                       |  9 ++++
 gdb/top.c                      | 78 +++++++++++++++++++++++++++++-----
 7 files changed, 96 insertions(+), 12 deletions(-)
 create mode 100644 gdb/tips

diff --git a/gdb/NEWS b/gdb/NEWS
index 0fd39857326..e977630c445 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,8 @@
 
 *** Changes since GDB 9
 
+* GDB will now display a helpful tip when starting up.
+
 * Help and apropos commands will now show the documentation of a
   command only once, even if that command has one or more aliases.
   These commands now show the command name, then all of its aliases,
diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index 85a2f41c351..b3c1243e832 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -139,7 +139,7 @@ SYSTEM_GDBINIT_FILES = \
 	elinos.py \
 	wrs-linux.py
 
-GDB_FILES = NEWS
+GDB_FILES = NEWS tips
 
 FLAGS_TO_PASS = \
 	"prefix=$(prefix)" \
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 37d5fe1e4cd..0a74992fbac 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2324,6 +2324,9 @@ automatically by @command{configure}.  When reporting a @value{GDBN}
 bug (@pxref{GDB Bugs}), it is important to include this information in
 your report.
 
+Finally, unless @code{--quiet} is used, @value{GDBN} will print a
+helpful tip at startup.
+
 @end table
 
 @node Running
diff --git a/gdb/tips b/gdb/tips
new file mode 100644
index 00000000000..75957fb853d
--- /dev/null
+++ b/gdb/tips
@@ -0,0 +1,9 @@
+This file is in the old "fortune" format.  Each entry is separated by
+a percent character on a line of its own.  Anything before the first
+percent will never be shown.
+%
+You can use "help news" to see what has changed in GDB.
+%
+Type "apropos word" to search for commands related to "word".
+%
+Type "show configuration" for configuration details.
diff --git a/gdb/top.c b/gdb/top.c
index da9b805b479..62a2c706031 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -56,6 +56,8 @@
 #include "gdbarch.h"
 #include "gdbsupport/pathstuff.h"
 #include "cli/cli-style.h"
+#include "gdbsupport/scoped_fd.h"
+#include <random>
 
 /* readline include files.  */
 #include "readline/readline.h"
@@ -128,6 +130,14 @@ current_ui_current_uiout_ptr ()
 
 int inhibit_gdbinit = 0;
 
+/* The style used for informational messages at startup.  */
+static ui_file_style startup_style =
+{
+  ui_file_style::MAGENTA,
+  ui_file_style::NONE,
+  ui_file_style::BOLD
+};
+
 /* Flag for whether we want to confirm potentially dangerous
    operations.  Default is yes.  */
 
@@ -1389,6 +1399,59 @@ command_line_input (const char *prompt_arg, const char *annotation_suffix)
   return cmd;
 }
 \f
+
+/* Print a useful tip to STREAM.  */
+
+static void
+print_tip (struct ui_file *stream)
+{
+  std::string tip_name = std::string (gdb_datadir) + SLASH_STRING + "tips";
+  scoped_fd fd (gdb_open_cloexec (tip_name.c_str (), O_RDONLY | O_BINARY, 0));
+  if (fd.get () < 0)
+    {
+      perror_warning_with_name ((std::string (_("Could not open tip file "))
+				 + tip_name).c_str ());
+      return;
+    }
+
+  std::string lines;
+  try
+    {
+      lines = read_entire_file (tip_name.c_str (), fd.get ());
+    }
+  catch (const gdb_exception &exc)
+    {
+      exception_print (stream, exc);
+      return;
+    }
+
+  std::vector<gdb::string_view> strings;
+  size_t offset = lines.find ("%\n");
+  if (offset == std::string::npos)
+    return;
+  while (offset != std::string::npos)
+    {
+      offset += 2;
+      size_t next_offset = lines.find ("%\n", offset);
+
+      if (next_offset == std::string::npos)
+	strings.emplace_back (lines.c_str () + offset);
+      else
+	strings.emplace_back (lines.c_str () + offset, next_offset - offset);
+
+      offset = next_offset;
+    }
+
+  std::random_device rd;
+  std::mt19937 mt (rd ());
+  std::uniform_int_distribution<> distr (0, strings.size () - 1);
+  int index = distr (mt);
+  /* Note that the string will include a newline.  */
+  fprintf_styled (stream, startup_style, "\n%.*s",
+		  (int) strings[index].size (),
+		  strings[index].data ());
+}
+
 /* See top.h.  */
 void
 print_gdb_version (struct ui_file *stream, bool interactive)
@@ -1399,11 +1462,7 @@ print_gdb_version (struct ui_file *stream, bool interactive)
 
   ui_file_style style;
   if (interactive)
-    {
-      ui_file_style nstyle = { ui_file_style::MAGENTA, ui_file_style::NONE,
-			       ui_file_style::BOLD };
-      style = nstyle;
-    }
+    style = startup_style;
   fprintf_styled (stream, style, "GNU gdb %s%s\n", PKGVERSION, version);
 
   /* Second line is a copyright notice.  */
@@ -1441,9 +1500,6 @@ There is NO WARRANTY, to the extent permitted by law.");
     }
   fprintf_filtered (stream, "\".\n");
 
-  fprintf_filtered (stream, _("Type \"show configuration\" "
-			      "for configuration details.\n"));
-
   if (REPORT_BUGS_TO[0])
     {
       fprintf_filtered (stream,
@@ -1455,9 +1511,9 @@ There is NO WARRANTY, to the extent permitted by law.");
 resources online at:\n    <http://www.gnu.org/software/gdb/documentation/>."));
   fprintf_filtered (stream, "\n\n");
   fprintf_filtered (stream, _("For help, type \"help\".\n"));
-  fprintf_filtered (stream,
-		    _("Type \"apropos word\" to search for commands \
-related to \"word\"."));
+
+  if (interactive)
+    print_tip (stream);
 }
 
 /* Print the details of GDB build-time configuration.  */
-- 
2.17.2

Re: [PATCH v2 3/7] Add "tips" file to gdb

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Date: Tue, 23 Jun 2020 07:20:02 -0600
> Cc: Tom Tromey <tom@tromey.com>
> 
> This adds a "tips" file to gdb.  This file holds handy tips -- right
> now there are just a few, but it's easy to add more.  A random tip is
> displayed during interactive startup.
> 
> gdb/ChangeLog
> 2020-06-22  Tom Tromey  <tom@tromey.com>
> 
> 	* NEWS: Add entry.
> 	* top.c (startup_style): New global.
> 	(print_tip): New function.
> 	(print_gdb_version): Update.  Don't print "show configuration"
> 	or "apropos" info.  Call print_tip.
> 	* tips: New file.
> 	* data-directory/Makefile.in (GDB_FILES): Add "tips".
> 
> gdb/doc/ChangeLog
> 2020-06-22  Tom Tromey  <tom@tromey.com>
> 
> 	* gdb.texinfo (Help): Mention tips.

Thanks, this is OK for the documentation parts.

Re: [PATCH v2 3/7] Add "tips" file to gdb

[Simon Marchi]

On 2020-06-23 9:20 a.m., Tom Tromey wrote:
> This adds a "tips" file to gdb.  This file holds handy tips -- right
> now there are just a few, but it's easy to add more.  A random tip is
> displayed during interactive startup.

Awesome!  I propose we merge this after the GDB 11 branch creation, since
it would be a bit silly to ship this with only these three tips.  That will
give us time until the next release to add more.  I'll come up with some and
ask my colleagues who use GDB if they would have any ideas for some.

Simon

[PATCH v2 4/7] Add get_standard_config_dir function

[Tom Tromey]

This adds a new get_standard_config_dir, which returns the name of the
configuration directory.  In XDG, this is ~/.config/gdb/.  Future
patches will make use of this.

2020-06-22  Tom Tromey  <tom@tromey.com>

	* pathstuff.h (get_standard_config_dir): Declare.
	* pathstuff.cc (get_standard_config_dir): New function.
---
 gdbsupport/ChangeLog    |  5 +++++
 gdbsupport/pathstuff.cc | 32 ++++++++++++++++++++++++++++++++
 gdbsupport/pathstuff.h  | 14 ++++++++++++++
 3 files changed, 51 insertions(+)

diff --git a/gdbsupport/pathstuff.cc b/gdbsupport/pathstuff.cc
index 1f60fd0c986..9fb5e5cf614 100644
--- a/gdbsupport/pathstuff.cc
+++ b/gdbsupport/pathstuff.cc
@@ -266,6 +266,38 @@ get_standard_temp_dir ()
 #endif
 }
 
+/* See pathstuff.h.  */
+
+std::string
+get_standard_config_dir ()
+{
+#ifdef __APPLE__
+#define HOME_CONFIG_DIR "Library/Preferences"
+#else
+#define HOME_CONFIG_DIR ".config"
+#endif
+
+#ifndef __APPLE__
+  const char *xdg_config_home = getenv ("XDG_CONFIG_HOME");
+  if (xdg_config_home != NULL)
+    {
+      /* Make sure the path is absolute and tilde-expanded.  */
+      gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (xdg_config_home));
+      return string_printf ("%s/gdb", abs.get ());
+    }
+#endif
+
+  const char *home = getenv ("HOME");
+  if (home != NULL)
+    {
+      /* Make sure the path is absolute and tilde-expanded.  */
+      gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (home));
+      return string_printf ("%s/" HOME_CONFIG_DIR "/gdb", abs.get ());
+    }
+
+  return {};
+}
+
 /* See gdbsupport/pathstuff.h.  */
 
 const char *
diff --git a/gdbsupport/pathstuff.h b/gdbsupport/pathstuff.h
index 4bc0d892119..85241bc8c7c 100644
--- a/gdbsupport/pathstuff.h
+++ b/gdbsupport/pathstuff.h
@@ -85,6 +85,20 @@ extern std::string get_standard_cache_dir ();
 
 extern std::string get_standard_temp_dir ();
 
+/* Get the usual user config directory for the current platform.
+
+   On Linux, it follows the XDG Base Directory specification: use
+   $XDG_CONFIG_HOME/gdb if the XDG_CONFIG_HOME environment variable is
+   defined, otherwise $HOME/.config.
+
+   On macOS, it follows the local convention and uses
+   ~/Library/Preferences/gdb.
+
+  The return value is absolute and tilde-expanded.  Return an empty
+  string if neither XDG_CONFIG_HOME (on Linux) or HOME are defined.  */
+
+extern std::string get_standard_config_dir ();
+
 /* Return the file name of the user's shell.  Normally this comes from
    the SHELL environment variable.  */
 
-- 
2.17.2

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

[Tom Tromey]

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);
+}
+
+/* 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.  */
+	}
+    }
+}
+
+\f
 /* 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

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

[Tom Tromey]

>>>>> "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

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

[Andrew Burgess]

* 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.  */
> +	}
> +    }
> +}
> +
> +\f
>  /* 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
> 

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

[Andrew Burgess]

* 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 v2 6/7] Let the user control the startup style

[Tom Tromey]

Some users find the startup text highlighting to be distracting.  This
patch provides a way to change this, building on the startup file
infrastructure that was added earlier.

gdb/ChangeLog
2020-06-22  Tom Tromey  <tom@tromey.com>

	* tips: Add a tip.
	* NEWS: Add entry.
	* top.c (startup_style): Remove.
	(print_tip, print_gdb_version): Update.
	* cli/cli-style.h (class cli_style_option) <cli_style_option>: Add
	intensity parameter.
	<write>: Declare new method.
	(startup_style): Declare.
	* cli/cli-style.c (startup_style): New global.
	(cli_style_option): Add intensity parameter.
	(cli_style_option::write): New method.
	(_initialize_cli_style): Register new style and callbacks.

gdb/doc/ChangeLog
2020-06-22  Tom Tromey  <tom@tromey.com>

	* gdb.texinfo (Output Styling): Document "set style startup"
	commands.

gdb/testsuite/ChangeLog
2020-06-22  Tom Tromey  <tom@tromey.com>

	* gdb.base/persist.exp: New file.
---
 gdb/ChangeLog                      | 15 +++++++
 gdb/NEWS                           |  7 ++++
 gdb/cli/cli-style.c                | 36 ++++++++++++++++-
 gdb/cli/cli-style.h                |  9 ++++-
 gdb/doc/ChangeLog                  |  5 +++
 gdb/doc/gdb.texinfo                | 15 +++++++
 gdb/testsuite/ChangeLog            |  4 ++
 gdb/testsuite/gdb.base/persist.exp | 64 ++++++++++++++++++++++++++++++
 gdb/tips                           |  3 ++
 gdb/top.c                          | 12 +-----
 10 files changed, 157 insertions(+), 13 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/persist.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index e977630c445..8f474c51bf4 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -80,6 +80,13 @@ show exec-file-mismatch -- Show exec-file-mismatch handling (ask|warn|off).
   executable file; if 'warn', just display a warning; if 'off', don't
   attempt to detect a mismatch.
 
+set style startup foreground COLOR
+set style startup background COLOR
+set style startup intensity VALUE
+  Control the styling of startup text.  This saves the setting into
+  a special configuration file, so that it can be read during startup
+  and applied.
+
 tui new-layout NAME WINDOW WEIGHT [WINDOW WEIGHT]...
   Define a new TUI layout, specifying its name and the windows that
   will be displayed.
diff --git a/gdb/cli/cli-style.c b/gdb/cli/cli-style.c
index a0c3cc51801..8486d22ecd9 100644
--- a/gdb/cli/cli-style.c
+++ b/gdb/cli/cli-style.c
@@ -19,6 +19,7 @@
 
 #include "defs.h"
 #include "cli/cli-cmds.h"
+#include "cli/cli-setshow.h"
 #include "cli/cli-style.h"
 #include "source-cache.h"
 #include "observable.h"
@@ -98,13 +99,19 @@ cli_style_option metadata_style ("metadata", ui_file_style::DIM);
 
 /* See cli-style.h.  */
 
+cli_style_option startup_style ("startup", ui_file_style::MAGENTA,
+				ui_file_style::BOLD);
+
+/* See cli-style.h.  */
+
 cli_style_option::cli_style_option (const char *name,
-				    ui_file_style::basic_color fg)
+				    ui_file_style::basic_color fg,
+				    ui_file_style::intensity intensity)
   : changed (name),
     m_name (name),
     m_foreground (cli_colors[fg - ui_file_style::NONE]),
     m_background (cli_colors[0]),
-    m_intensity (cli_intensities[ui_file_style::NORMAL])
+    m_intensity (cli_intensities[intensity])
 {
 }
 
@@ -253,6 +260,17 @@ cli_style_option::add_setshow_commands (enum command_class theclass,
 			  &m_set_list, &m_show_list, (void *) this);
 }
 
+void
+cli_style_option::write (ui_file *outfile)
+{
+  fprintf_unfiltered (outfile, "set style %s background %s\n",
+		      m_name, m_background);
+  fprintf_unfiltered (outfile, "set style %s foreground %s\n",
+		      m_name, m_foreground);
+  fprintf_unfiltered (outfile, "set style %s intensity %s\n",
+		      m_name, m_intensity);
+}
+
 static cmd_list_element *style_set_list;
 static cmd_list_element *style_show_list;
 
@@ -382,4 +400,18 @@ TUI window that does have the focus."),
 						&style_set_list,
 						&style_show_list,
 						true);
+
+  startup_style.add_setshow_commands (no_class, _("\
+Startup display styling.\n\
+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)
+    {
+      startup_style.write (outfile);
+    });
+  /* Arrange to write the startup file whenever a startup style
+     setting changes.  */
+  startup_style.changed.attach (write_startup_file);
 }
diff --git a/gdb/cli/cli-style.h b/gdb/cli/cli-style.h
index 6422e5296a3..28859f88560 100644
--- a/gdb/cli/cli-style.h
+++ b/gdb/cli/cli-style.h
@@ -30,7 +30,8 @@ class cli_style_option
 public:
 
   /* Construct a CLI style option with a foreground color.  */
-  cli_style_option (const char *name, ui_file_style::basic_color fg);
+  cli_style_option (const char *name, ui_file_style::basic_color fg,
+		    ui_file_style::intensity = ui_file_style::NORMAL);
 
   /* Construct a CLI style option with an intensity.  */
   cli_style_option (const char *name, ui_file_style::intensity i);
@@ -56,6 +57,9 @@ class cli_style_option
   /* Same as SET_LIST but for the show command list.  */
   struct cmd_list_element *show_list () { return m_show_list; };
 
+  /* Write this style to FILE.  */
+  void write (ui_file *outfile);
+
   /* This style can be observed for any changes.  */
   gdb::observers::observable<> changed;
 
@@ -124,6 +128,9 @@ extern cli_style_option tui_border_style;
 /* The border style of a TUI window that does have the focus.  */
 extern cli_style_option tui_active_border_style;
 
+/* The style to use for (some) startup text.  */
+extern cli_style_option startup_style;
+
 /* True if source styling is enabled.  */
 extern bool source_styling;
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0a74992fbac..7c3fe7d38a2 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -25672,6 +25672,21 @@ Control the styling of addresses.  These are managed with the
 @code{set style address} family of commands.  By default, this style's
 foreground color is blue.
 
+@item startup
+Control the styling of some text that is printed at startup.  These
+are managed with the @code{set style startup} family of commands.  By
+default, this style's foreground color is magenta and it has bold
+intensity.  Changing these settings will cause them to automatically
+be saved in a special configuration file, which is read by
+@value{GDBN} early in its startup.
+
+The default value for this directory depends on the host platform.  On
+most systems, the index is cached in the @file{gdb} subdirectory of
+the directory pointed to by the @env{XDG_CONFIG_HOME} environment
+variable, if it is defined, else in the @file{.config/gdb} subdirectory
+of your home directory.  However, on some systems, the default may
+differ according to local convention.
+
 @item title
 Control the styling of titles.  These are managed with the
 @code{set style title} family of commands.  By default, this style's
diff --git a/gdb/testsuite/gdb.base/persist.exp b/gdb/testsuite/gdb.base/persist.exp
new file mode 100644
index 00000000000..76a55d558c3
--- /dev/null
+++ b/gdb/testsuite/gdb.base/persist.exp
@@ -0,0 +1,64 @@
+# Copyright 2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# This test relies on XDG_CONFIG_HOME, which does not work on all
+# platforms; so limit it to Linux native.
+if {![istarget "*-*-linux*"]} then {
+    continue
+}
+if { [target_info gdb_protocol] != "" } {
+    continue
+}
+
+# Check that the contents of FILENAME changed and that the contents
+# now contain the given command.  CONTENTS is the old contents.
+# COMMAND is the command to run, and also used to name the test.
+# Returns the new contents.
+proc require_changed {command filename contents} {
+    gdb_test_no_output $command
+
+    set fd [open $filename r]
+    set new [read $fd]
+    close $fd
+
+    set testname "$command check"
+    if {$contents == $new || [string first $command $new] == -1} {
+	fail $testname
+    } else {
+	pass $testname
+    }
+
+    return $new
+}
+
+save_vars { env(XDG_CONFIG_HOME) } {
+    set dirname [standard_output_file .]
+    file mkdir $dirname/gdb
+    set filename $dirname/gdb/startup-commands
+
+    # Create the file as empty.
+    set fd [open $filename w]
+    close $fd
+
+    # Current contents.
+    set contents ""
+
+    setenv XDG_CONFIG_HOME $dirname
+    clean_restart
+
+    set contents [require_changed "set style startup foreground green" $filename $contents]
+    set contents [require_changed "set style startup background green" $filename $contents]
+    set contents [require_changed "set style startup intensity dim" $filename $contents]
+}
diff --git a/gdb/tips b/gdb/tips
index 75957fb853d..a9c89747aa4 100644
--- a/gdb/tips
+++ b/gdb/tips
@@ -7,3 +7,6 @@ You can use "help news" to see what has changed in GDB.
 Type "apropos word" to search for commands related to "word".
 %
 Type "show configuration" for configuration details.
+%
+You can use the "set style startup" family of commands to change the
+style used for tips.
diff --git a/gdb/top.c b/gdb/top.c
index 62a2c706031..e86ce4020f3 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -130,14 +130,6 @@ current_ui_current_uiout_ptr ()
 
 int inhibit_gdbinit = 0;
 
-/* The style used for informational messages at startup.  */
-static ui_file_style startup_style =
-{
-  ui_file_style::MAGENTA,
-  ui_file_style::NONE,
-  ui_file_style::BOLD
-};
-
 /* Flag for whether we want to confirm potentially dangerous
    operations.  Default is yes.  */
 
@@ -1447,7 +1439,7 @@ print_tip (struct ui_file *stream)
   std::uniform_int_distribution<> distr (0, strings.size () - 1);
   int index = distr (mt);
   /* Note that the string will include a newline.  */
-  fprintf_styled (stream, startup_style, "\n%.*s",
+  fprintf_styled (stream, startup_style.style (), "\n%.*s",
 		  (int) strings[index].size (),
 		  strings[index].data ());
 }
@@ -1462,7 +1454,7 @@ print_gdb_version (struct ui_file *stream, bool interactive)
 
   ui_file_style style;
   if (interactive)
-    style = startup_style;
+    style = startup_style.style ();
   fprintf_styled (stream, style, "GNU gdb %s%s\n", PKGVERSION, version);
 
   /* Second line is a copyright notice.  */
-- 
2.17.2

Re: [PATCH v2 6/7] Let the user control the startup style

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Date: Tue, 23 Jun 2020 07:20:05 -0600
> Cc: Tom Tromey <tom@tromey.com>
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index e977630c445..8f474c51bf4 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -80,6 +80,13 @@ show exec-file-mismatch -- Show exec-file-mismatch handling (ask|warn|off).
>    executable file; if 'warn', just display a warning; if 'off', don't
>    attempt to detect a mismatch.
>  
> +set style startup foreground COLOR
> +set style startup background COLOR
> +set style startup intensity VALUE
> +  Control the styling of startup text.  This saves the setting into
> +  a special configuration file, so that it can be read during startup
> +  and applied.
> +

This is OK.

> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -25672,6 +25672,21 @@ Control the styling of addresses.  These are managed with the
>  @code{set style address} family of commands.  By default, this style's
>  foreground color is blue.
>  
> +@item startup
> +Control the styling of some text that is printed at startup.  These
> +are managed with the @code{set style startup} family of commands.  By
> +default, this style's foreground color is magenta and it has bold
> +intensity.  Changing these settings will cause them to automatically
> +be saved in a special configuration file, which is read by
> +@value{GDBN} early in its startup.
> +
> +The default value for this directory depends on the host platform.  On
                         ^^^^^^^^^^^^^^
The word "directory" doesn't appear in the previous paragraph, so it
is unclear what "this directory" refers to.

> +most systems, the index is cached in the @file{gdb} subdirectory of
                 ^^^^^^^^^^^^^^^^^^^
Likewise with "index".

The documentation parts are OK with those nits fixed.  Thanks.

Re: [PATCH v2 6/7] Let the user control the startup style

[Tom Tromey]

>> +@item startup
>> +Control the styling of some text that is printed at startup.  These
>> +are managed with the @code{set style startup} family of commands.  By
>> +default, this style's foreground color is magenta and it has bold
>> +intensity.  Changing these settings will cause them to automatically
>> +be saved in a special configuration file, which is read by
>> +@value{GDBN} early in its startup.
>> +
>> +The default value for this directory depends on the host platform.  On
Eli>                          ^^^^^^^^^^^^^^
Eli> The word "directory" doesn't appear in the previous paragraph, so it
Eli> is unclear what "this directory" refers to.

>> +most systems, the index is cached in the @file{gdb} subdirectory of
Eli>                  ^^^^^^^^^^^^^^^^^^^
Eli> Likewise with "index".

Eli> The documentation parts are OK with those nits fixed.  Thanks.

Thanks for the review.  I copied some text from the index cache section,
but I didn't sufficiently edit it.  This appeared in another patch in
this series as well.

In both spots now, I've changed this paragraph to read:

+The directory in which this file appears depends on the host platform.
+On most systems, the file is in the @file{gdb} subdirectory of the
+directory pointed to by the @env{XDG_CONFIG_HOME} environment
+variable, if it is defined, else in the @file{.config/gdb}
+subdirectory of your home directory.  However, on some systems, the
+default may differ according to local convention.

Tom

Re: [PATCH v2 6/7] Let the user control the startup style

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
> Date: Sun, 05 Jul 2020 12:50:23 -0600
> 
> +The directory in which this file appears depends on the host platform.
> +On most systems, the file is in the @file{gdb} subdirectory of the
> +directory pointed to by the @env{XDG_CONFIG_HOME} environment
> +variable, if it is defined, else in the @file{.config/gdb}
> +subdirectory of your home directory.  However, on some systems, the
> +default may differ according to local convention.

LGTM, thanks.

[PATCH v2 7/7] Add "set startup-quietly"

[Tom Tromey]

This adds a new command to change gdb to behave as though "-quiet"
were always given.  This is done by using the early startup file
infrastructure.

gdb/ChangeLog
2020-06-22  Tom Tromey  <tom@tromey.com>

	* tips: Add a tip.
	* NEWS: Add entry.
	* top.h (check_quiet_mode): Declare.
	* top.c (startup_quiet): New global.
	(check_quiet_mode, set_startup_quiet, show_startup_quiet)
	(write_startup_quietly): New functions.
	(init_main): Register new command and callback.
	* main.c (captured_main_1): Call check_quiet_mode.

gdb/doc/ChangeLog
2020-03-28  Tom Tromey  <tom@tromey.com>

	* gdb.texinfo (Mode Options): Mention "set startup-quietly".

gdb/testsuite/ChangeLog
2020-06-22  Tom Tromey  <tom@tromey.com>

	* gdb.base/persist.exp: Add startup-quietly test.
---
 gdb/ChangeLog                      | 11 +++++++
 gdb/NEWS                           |  7 +++++
 gdb/doc/ChangeLog                  |  4 +++
 gdb/doc/gdb.texinfo                | 13 ++++++++
 gdb/main.c                         |  7 ++++-
 gdb/testsuite/ChangeLog            |  4 +++
 gdb/testsuite/gdb.base/persist.exp |  1 +
 gdb/tips                           |  3 ++
 gdb/top.c                          | 48 ++++++++++++++++++++++++++++++
 gdb/top.h                          |  5 ++++
 10 files changed, 102 insertions(+), 1 deletion(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 8f474c51bf4..31cfdc95ae1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -87,6 +87,13 @@ set style startup intensity VALUE
   a special configuration file, so that it can be read during startup
   and applied.
 
+set startup-quietly on|off
+show startup-quietly
+  When enabled, this causes GDB to act as if "-silent" were always
+  passed on the command line.  This saves the setting into a special
+  configuration file, so that it can be read during startup and
+  applied.
+
 tui new-layout NAME WINDOW WEIGHT [WINDOW WEIGHT]...
   Define a new TUI layout, specifying its name and the windows that
   will be displayed.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7c3fe7d38a2..ba44369c10e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1124,6 +1124,19 @@ in your home directory.
 ``Quiet''.  Do not print the introductory and copyright messages.  These
 messages are also suppressed in batch mode.
 
+This can also be enabled using @code{set startup-quietly on}.  The
+default is @code{off}.  Use @code{show startup-quietly} to see the
+current setting.  Unlike most settings in @value{GDBN}, this one is
+automatically saved by creating a special file in a configuration
+directory.
+
+The default value for this directory depends on the host platform.  On
+most systems, the index is cached in the @file{gdb} subdirectory of
+the directory pointed to by the @env{XDG_CONFIG_HOME} environment
+variable, if it is defined, else in the @file{.config/gdb} subdirectory
+of your home directory.  However, on some systems, the default may
+differ according to local convention.
+
 @item -batch
 @cindex @code{--batch}
 Run in batch mode.  Exit with status @code{0} after processing all the
diff --git a/gdb/main.c b/gdb/main.c
index bd1f6d6b2c5..34752d27737 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -936,7 +936,12 @@ captured_main_1 (struct captured_main_args *context)
 
   /* Set the startup style.  */
   if (!inhibit_gdbinit && !inhibit_home_gdbinit)
-    read_startup_file ();
+    {
+      read_startup_file ();
+
+      if (!quiet)
+	quiet = check_quiet_mode ();
+    }
 
   /* Now that gdb_init has created the initial inferior, we're in
      position to set args for that inferior.  */
diff --git a/gdb/testsuite/gdb.base/persist.exp b/gdb/testsuite/gdb.base/persist.exp
index 76a55d558c3..adffa488aa5 100644
--- a/gdb/testsuite/gdb.base/persist.exp
+++ b/gdb/testsuite/gdb.base/persist.exp
@@ -61,4 +61,5 @@ save_vars { env(XDG_CONFIG_HOME) } {
     set contents [require_changed "set style startup foreground green" $filename $contents]
     set contents [require_changed "set style startup background green" $filename $contents]
     set contents [require_changed "set style startup intensity dim" $filename $contents]
+    set contents [require_changed "set startup-quietly on" $filename $contents]
 }
diff --git a/gdb/tips b/gdb/tips
index a9c89747aa4..a83dc2c9ddc 100644
--- a/gdb/tips
+++ b/gdb/tips
@@ -10,3 +10,6 @@ Type "show configuration" for configuration details.
 %
 You can use the "set style startup" family of commands to change the
 style used for tips.
+%
+You can use "set startup-quietly on" to have GDB always start up
+silently.
diff --git a/gdb/top.c b/gdb/top.c
index e86ce4020f3..910b9d0dfcb 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -2196,6 +2196,43 @@ set_history_filename (const char *args,
     }
 }
 
+/* Whether we're in quiet startup mode.  */
+
+static bool startup_quiet;
+
+/* See top.h.  */
+
+bool
+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
+set_startup_quiet (const char *args, int from_tty, struct cmd_list_element *c)
+{
+  write_startup_file ();
+}
+
+static void
+show_startup_quiet (struct ui_file *file, int from_tty,
+	      struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Whether to start up quietly is %s.\n"),
+		    value);
+}
+
 static void
 init_gdb_version_vars (void)
 {
@@ -2350,6 +2387,17 @@ input settings."),
                         show_interactive_mode,
                         &setlist, &showlist);
 
+  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);
+  /* Arrange to write "set startup-quietly" to the early startup
+     file.  */
+  add_startup_writer (write_startup_quietly);
+
   c = add_cmd ("new-ui", class_support, new_ui_command, _("\
 Create a new UI.\n\
 Usage: new-ui INTERPRETER TTY\n\
diff --git a/gdb/top.h b/gdb/top.h
index fd992977155..85e178c527a 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -298,4 +298,9 @@ extern char *handle_line_of_input (struct buffer *cmd_line_buffer,
 				   const char *rl, int repeat,
 				   const char *annotation_suffix);
 
+/* Call at startup to see if the user has requested that gdb start up
+   quietly.  */
+
+extern bool check_quiet_mode ();
+
 #endif
-- 
2.17.2

Re: [PATCH v2 7/7] Add "set startup-quietly"

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Date: Tue, 23 Jun 2020 07:20:06 -0600
> Cc: Tom Tromey <tom@tromey.com>
> 
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -87,6 +87,13 @@ set style startup intensity VALUE
>    a special configuration file, so that it can be read during startup
>    and applied.
>  
> +set startup-quietly on|off
> +show startup-quietly
> +  When enabled, this causes GDB to act as if "-silent" were always
> +  passed on the command line.  This saves the setting into a special
> +  configuration file, so that it can be read during startup and
> +  applied.

OK for this part.

> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -1124,6 +1124,19 @@ in your home directory.
>  ``Quiet''.  Do not print the introductory and copyright messages.  These
>  messages are also suppressed in batch mode.
>  
> +This can also be enabled using @code{set startup-quietly on}.  The
> +default is @code{off}.  Use @code{show startup-quietly} to see the
> +current setting.  Unlike most settings in @value{GDBN}, this one is
> +automatically saved by creating a special file in a configuration
> +directory.

Please index the new commands using @kindex, as we do for every
command.

The documentation parts are OK with this nit fixed, thanks.