[PATCH 0/3] [RFC] Load gdbinit files from a directory

[PATCH 0/3] [RFC] Load gdbinit files from a directory

[Christian Biesinger via gdb-patches]

This patch series is some refactoring and then a patch to load gdbinit
files from a directory, instead of only allowing a single file.

Fedora ships a system gdbinit file that does something similar; this
does this by default and also works if Python is disabled.

Christian Biesinger (3):
  Refactor get_init_files to use std::string
  Factor out the code to do the datadir-relocation for gdbinit
  Load system gdbinit files from a directory

 gdb/config.in    |   3 +
 gdb/configure    |  77 +++++++++++++++++++--
 gdb/configure.ac |   3 +
 gdb/main.c       | 175 ++++++++++++++++++++++++++++-------------------
 4 files changed, 183 insertions(+), 75 deletions(-)

-- 
2.23.0.rc1.153.gdeed80330f-goog

[PATCH 2/3] Factor out the code to do the datadir-relocation for gdbinit

[Christian Biesinger via gdb-patches]

gdb/ChangeLog:

2019-08-20  Christian Biesinger  <cbiesinger@google.com>

	* main.c (relocate_gdbinit_path_maybe_in_datadir): New function.
	(get_init_files): Update.
---
 gdb/main.c | 68 +++++++++++++++++++++++++++++-------------------------
 1 file changed, 37 insertions(+), 31 deletions(-)

diff --git a/gdb/main.c b/gdb/main.c
index b9e12589ab..a1d1904c9b 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -191,6 +191,41 @@ relocate_gdb_directory (const char *initial, int flag)
   return dir;
 }
 
+static std::string relocate_gdbinit_path_maybe_in_datadir (std::string file)
+{
+  int datadir_len = strlen (GDB_DATADIR);
+
+  /* If SYSTEM_GDBINIT lives in data-directory, and data-directory
+     has been provided, search for SYSTEM_GDBINIT there.  */
+  if (gdb_datadir_provided
+      && datadir_len < file.length ()
+      && filename_ncmp (file.c_str (), GDB_DATADIR, datadir_len) == 0
+      && IS_DIR_SEPARATOR (file[datadir_len]))
+    {
+      /* Append the part of SYSTEM_GDBINIT that follows GDB_DATADIR
+	 to gdb_datadir.  */
+
+      size_t start = datadir_len;
+      for (; IS_DIR_SEPARATOR (file[start]); ++start)
+	continue;
+      return std::string (gdb_datadir) + SLASH_STRING +
+	file.substr(start);
+    }
+  else
+    {
+      char *relocated = relocate_path (gdb_program_name,
+				       file.c_str(),
+				       SYSTEM_GDBINIT_RELOCATABLE);
+      if (relocated != nullptr)
+	{
+	  std::string retval(relocated);
+	  xfree (relocated);
+	  return retval;
+	}
+    }
+    return "";
+}
+
 /* Compute the locations of init files that GDB should source and
    return them in SYSTEM_GDBINIT, HOME_GDBINIT, LOCAL_GDBINIT.  If
    there is no system gdbinit (resp. home gdbinit and local gdbinit)
@@ -212,37 +247,8 @@ get_init_files (std::string *system_gdbinit,
 
       if (SYSTEM_GDBINIT[0])
 	{
-	  int datadir_len = strlen (GDB_DATADIR);
-	  int sys_gdbinit_len = strlen (SYSTEM_GDBINIT);
-	  std::string relocated_sysgdbinit;
-
-	  /* If SYSTEM_GDBINIT lives in data-directory, and data-directory
-	     has been provided, search for SYSTEM_GDBINIT there.  */
-	  if (gdb_datadir_provided
-	      && datadir_len < sys_gdbinit_len
-	      && filename_ncmp (SYSTEM_GDBINIT, GDB_DATADIR, datadir_len) == 0
-	      && IS_DIR_SEPARATOR (SYSTEM_GDBINIT[datadir_len]))
-	    {
-	      /* Append the part of SYSTEM_GDBINIT that follows GDB_DATADIR
-		 to gdb_datadir.  */
-
-	      size_t start = datadir_len;
-	      for (; IS_DIR_SEPARATOR (SYSTEM_GDBINIT[start]); ++start)
-		continue;
-	      relocated_sysgdbinit = std::string (gdb_datadir) + SLASH_STRING +
-		&SYSTEM_GDBINIT[start];
-	    }
-	  else
-	    {
-	      char *relocated = relocate_path (gdb_program_name,
-					       SYSTEM_GDBINIT,
-					       SYSTEM_GDBINIT_RELOCATABLE);
-	      if (relocated != nullptr)
-	        {
-		  relocated_sysgdbinit = relocated;
-		  xfree (relocated);
-		}
-	    }
+	  std::string relocated_sysgdbinit =
+	    relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT);
 	  if (!relocated_sysgdbinit.empty () &&
 	      stat (relocated_sysgdbinit.c_str (), &s) == 0)
 	    sysgdbinit = relocated_sysgdbinit;
-- 
2.23.0.rc1.153.gdeed80330f-goog

Re: [PATCH 2/3] Factor out the code to do the datadir-relocation for gdbinit

[Sergio Durigan Junior]

On Tuesday, August 20 2019, Christian Biesinger via gdb-patches wrote:

> gdb/ChangeLog:
>
> 2019-08-20  Christian Biesinger  <cbiesinger@google.com>
>
> 	* main.c (relocate_gdbinit_path_maybe_in_datadir): New function.
> 	(get_init_files): Update.

I'm afraid you'll need a descriptive commit message :-).

> ---
>  gdb/main.c | 68 +++++++++++++++++++++++++++++-------------------------
>  1 file changed, 37 insertions(+), 31 deletions(-)
>
> diff --git a/gdb/main.c b/gdb/main.c
> index b9e12589ab..a1d1904c9b 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -191,6 +191,41 @@ relocate_gdb_directory (const char *initial, int flag)
>    return dir;
>  }
>  
> +static std::string relocate_gdbinit_path_maybe_in_datadir (std::string file)

You should break the line after 'std::string':

  static std::string
  relocate_gdbinit_path_maybe_in_datadir (std::string file)

> +{
> +  int datadir_len = strlen (GDB_DATADIR);

size_t.

Also, you could declare a return variable here and just fill it
inside each 'if', instead of returning early (and then having to return
an empty string at the end (but that's a matter of style, I know).

> +
> +  /* If SYSTEM_GDBINIT lives in data-directory, and data-directory
> +     has been provided, search for SYSTEM_GDBINIT there.  */
> +  if (gdb_datadir_provided
> +      && datadir_len < file.length ()
> +      && filename_ncmp (file.c_str (), GDB_DATADIR, datadir_len) == 0
> +      && IS_DIR_SEPARATOR (file[datadir_len]))
> +    {
> +      /* Append the part of SYSTEM_GDBINIT that follows GDB_DATADIR
> +	 to gdb_datadir.  */
> +
> +      size_t start = datadir_len;
> +      for (; IS_DIR_SEPARATOR (file[start]); ++start)
> +	continue;

Same comment here: this loop seems strange (starting from 'start').

> +      return std::string (gdb_datadir) + SLASH_STRING +
> +	file.substr(start);
> +    }
> +  else
> +    {
> +      char *relocated = relocate_path (gdb_program_name,
> +				       file.c_str(),
> +				       SYSTEM_GDBINIT_RELOCATABLE);
> +      if (relocated != nullptr)
> +	{
> +	  std::string retval(relocated);

Space between variable name and open parenthesis.

> +	  xfree (relocated);
> +	  return retval;
> +	}
> +    }
> +    return "";
> +}
> +
>  /* Compute the locations of init files that GDB should source and
>     return them in SYSTEM_GDBINIT, HOME_GDBINIT, LOCAL_GDBINIT.  If
>     there is no system gdbinit (resp. home gdbinit and local gdbinit)
> @@ -212,37 +247,8 @@ get_init_files (std::string *system_gdbinit,
>  
>        if (SYSTEM_GDBINIT[0])
>  	{
> -	  int datadir_len = strlen (GDB_DATADIR);
> -	  int sys_gdbinit_len = strlen (SYSTEM_GDBINIT);
> -	  std::string relocated_sysgdbinit;
> -
> -	  /* If SYSTEM_GDBINIT lives in data-directory, and data-directory
> -	     has been provided, search for SYSTEM_GDBINIT there.  */
> -	  if (gdb_datadir_provided
> -	      && datadir_len < sys_gdbinit_len
> -	      && filename_ncmp (SYSTEM_GDBINIT, GDB_DATADIR, datadir_len) == 0
> -	      && IS_DIR_SEPARATOR (SYSTEM_GDBINIT[datadir_len]))
> -	    {
> -	      /* Append the part of SYSTEM_GDBINIT that follows GDB_DATADIR
> -		 to gdb_datadir.  */
> -
> -	      size_t start = datadir_len;
> -	      for (; IS_DIR_SEPARATOR (SYSTEM_GDBINIT[start]); ++start)
> -		continue;
> -	      relocated_sysgdbinit = std::string (gdb_datadir) + SLASH_STRING +
> -		&SYSTEM_GDBINIT[start];
> -	    }
> -	  else
> -	    {
> -	      char *relocated = relocate_path (gdb_program_name,
> -					       SYSTEM_GDBINIT,
> -					       SYSTEM_GDBINIT_RELOCATABLE);
> -	      if (relocated != nullptr)
> -	        {
> -		  relocated_sysgdbinit = relocated;
> -		  xfree (relocated);
> -		}
> -	    }
> +	  std::string relocated_sysgdbinit =
> +	    relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT);
>  	  if (!relocated_sysgdbinit.empty () &&
>  	      stat (relocated_sysgdbinit.c_str (), &s) == 0)
>  	    sysgdbinit = relocated_sysgdbinit;
> -- 
> 2.23.0.rc1.153.gdeed80330f-goog

Otherwise, LGTM.

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH 2/3] Factor out the code to do the datadir-relocation for gdbinit

[Christian Biesinger via gdb-patches]

On Wed, Aug 21, 2019 at 12:19 PM Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
>
> On Tuesday, August 20 2019, Christian Biesinger via gdb-patches wrote:
>
> > gdb/ChangeLog:
> >
> > 2019-08-20  Christian Biesinger  <cbiesinger@google.com>
> >
> >       * main.c (relocate_gdbinit_path_maybe_in_datadir): New function.
> >       (get_init_files): Update.
>
> I'm afraid you'll need a descriptive commit message :-).

Changed to:

2019-08-20  Christian Biesinger  <cbiesinger@google.com>

        * main.c (relocate_gdbinit_path_maybe_in_datadir): Factor this code
        out of get_init_files.
        (get_init_files): Update.

(I guess the title of the commit message is not enough?)

> > ---
> >  gdb/main.c | 68 +++++++++++++++++++++++++++++-------------------------
> >  1 file changed, 37 insertions(+), 31 deletions(-)
> >
> > diff --git a/gdb/main.c b/gdb/main.c
> > index b9e12589ab..a1d1904c9b 100644
> > --- a/gdb/main.c
> > +++ b/gdb/main.c
> > @@ -191,6 +191,41 @@ relocate_gdb_directory (const char *initial, int flag)
> >    return dir;
> >  }
> >
> > +static std::string relocate_gdbinit_path_maybe_in_datadir (std::string file)
>
> You should break the line after 'std::string':
>
>   static std::string
>   relocate_gdbinit_path_maybe_in_datadir (std::string file)

Thanks, done and also changed std::string to const std::string&.

> > +{
> > +  int datadir_len = strlen (GDB_DATADIR);
>
> size_t.
>
> Also, you could declare a return variable here and just fill it
> inside each 'if', instead of returning early (and then having to return
> an empty string at the end (but that's a matter of style, I know).

OK, if you prefer, sure. Done.

> > +
> > +  /* If SYSTEM_GDBINIT lives in data-directory, and data-directory
> > +     has been provided, search for SYSTEM_GDBINIT there.  */
> > +  if (gdb_datadir_provided
> > +      && datadir_len < file.length ()
> > +      && filename_ncmp (file.c_str (), GDB_DATADIR, datadir_len) == 0
> > +      && IS_DIR_SEPARATOR (file[datadir_len]))
> > +    {
> > +      /* Append the part of SYSTEM_GDBINIT that follows GDB_DATADIR
> > +      to gdb_datadir.  */
> > +
> > +      size_t start = datadir_len;
> > +      for (; IS_DIR_SEPARATOR (file[start]); ++start)
> > +     continue;
>
> Same comment here: this loop seems strange (starting from 'start').

See my response in the other thread.

> > +      return std::string (gdb_datadir) + SLASH_STRING +
> > +     file.substr(start);
> > +    }
> > +  else
> > +    {
> > +      char *relocated = relocate_path (gdb_program_name,
> > +                                    file.c_str(),
> > +                                    SYSTEM_GDBINIT_RELOCATABLE);
> > +      if (relocated != nullptr)
> > +     {
> > +       std::string retval(relocated);
>
> Space between variable name and open parenthesis.

Thanks! (Though irrelevant with the other change now)

> > +       xfree (relocated);
> > +       return retval;
> > +     }
> > +    }
> > +    return "";
> > +}
> > +
> >  /* Compute the locations of init files that GDB should source and
> >     return them in SYSTEM_GDBINIT, HOME_GDBINIT, LOCAL_GDBINIT.  If
> >     there is no system gdbinit (resp. home gdbinit and local gdbinit)
> > @@ -212,37 +247,8 @@ get_init_files (std::string *system_gdbinit,
> >
> >        if (SYSTEM_GDBINIT[0])
> >       {
> > -       int datadir_len = strlen (GDB_DATADIR);
> > -       int sys_gdbinit_len = strlen (SYSTEM_GDBINIT);
> > -       std::string relocated_sysgdbinit;
> > -
> > -       /* If SYSTEM_GDBINIT lives in data-directory, and data-directory
> > -          has been provided, search for SYSTEM_GDBINIT there.  */
> > -       if (gdb_datadir_provided
> > -           && datadir_len < sys_gdbinit_len
> > -           && filename_ncmp (SYSTEM_GDBINIT, GDB_DATADIR, datadir_len) == 0
> > -           && IS_DIR_SEPARATOR (SYSTEM_GDBINIT[datadir_len]))
> > -         {
> > -           /* Append the part of SYSTEM_GDBINIT that follows GDB_DATADIR
> > -              to gdb_datadir.  */
> > -
> > -           size_t start = datadir_len;
> > -           for (; IS_DIR_SEPARATOR (SYSTEM_GDBINIT[start]); ++start)
> > -             continue;
> > -           relocated_sysgdbinit = std::string (gdb_datadir) + SLASH_STRING +
> > -             &SYSTEM_GDBINIT[start];
> > -         }
> > -       else
> > -         {
> > -           char *relocated = relocate_path (gdb_program_name,
> > -                                            SYSTEM_GDBINIT,
> > -                                            SYSTEM_GDBINIT_RELOCATABLE);
> > -           if (relocated != nullptr)
> > -             {
> > -               relocated_sysgdbinit = relocated;
> > -               xfree (relocated);
> > -             }
> > -         }
> > +       std::string relocated_sysgdbinit =
> > +         relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT);
> >         if (!relocated_sysgdbinit.empty () &&
> >             stat (relocated_sysgdbinit.c_str (), &s) == 0)
> >           sysgdbinit = relocated_sysgdbinit;
> > --
> > 2.23.0.rc1.153.gdeed80330f-goog
>
> Otherwise, LGTM.

Thanks.

Christian

[PATCH 2/3 v2] Factor out the code to do the datadir-relocation for gdbinit

[Christian Biesinger via gdb-patches]

gdb/ChangeLog:

2019-08-20  Christian Biesinger  <cbiesinger@google.com>

	* main.c (relocate_gdbinit_path_maybe_in_datadir): Factor this code
	out of get_init_files.
	(get_init_files): Update.
---
 gdb/main.c | 70 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 39 insertions(+), 31 deletions(-)

diff --git a/gdb/main.c b/gdb/main.c
index 724b2928a7..089a7628e5 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -191,6 +191,43 @@ relocate_gdb_directory (const char *initial, int flag)
   return dir;
 }
 
+static std::string
+relocate_gdbinit_path_maybe_in_datadir (const std::string& file)
+{
+  size_t datadir_len = strlen (GDB_DATADIR);
+
+  std::string relocated_path;
+
+  /* If SYSTEM_GDBINIT lives in data-directory, and data-directory
+     has been provided, search for SYSTEM_GDBINIT there.  */
+  if (gdb_datadir_provided
+      && datadir_len < file.length ()
+      && filename_ncmp (file.c_str (), GDB_DATADIR, datadir_len) == 0
+      && IS_DIR_SEPARATOR (file[datadir_len]))
+    {
+      /* Append the part of SYSTEM_GDBINIT that follows GDB_DATADIR
+	 to gdb_datadir.  */
+
+      size_t start = datadir_len;
+      for (; IS_DIR_SEPARATOR (file[start]); ++start)
+	continue;
+      relocated_path = std::string (gdb_datadir) + SLASH_STRING +
+	file.substr(start);
+    }
+  else
+    {
+      char *relocated = relocate_path (gdb_program_name,
+				       file.c_str (),
+				       SYSTEM_GDBINIT_RELOCATABLE);
+      if (relocated != nullptr)
+	{
+	  relocated_path = relocated;
+	  xfree (relocated);
+	}
+    }
+    return relocated_path;
+}
+
 /* Compute the locations of init files that GDB should source and
    return them in SYSTEM_GDBINIT, HOME_GDBINIT, LOCAL_GDBINIT.  If
    there is no system gdbinit (resp. home gdbinit and local gdbinit)
@@ -212,37 +249,8 @@ get_init_files (std::string *system_gdbinit,
 
       if (SYSTEM_GDBINIT[0])
 	{
-	  size_t datadir_len = strlen (GDB_DATADIR);
-	  size_t sys_gdbinit_len = strlen (SYSTEM_GDBINIT);
-	  std::string relocated_sysgdbinit;
-
-	  /* If SYSTEM_GDBINIT lives in data-directory, and data-directory
-	     has been provided, search for SYSTEM_GDBINIT there.  */
-	  if (gdb_datadir_provided
-	      && datadir_len < sys_gdbinit_len
-	      && filename_ncmp (SYSTEM_GDBINIT, GDB_DATADIR, datadir_len) == 0
-	      && IS_DIR_SEPARATOR (SYSTEM_GDBINIT[datadir_len]))
-	    {
-	      /* Append the part of SYSTEM_GDBINIT that follows GDB_DATADIR
-		 to gdb_datadir.  */
-
-	      size_t start = datadir_len;
-	      for (; IS_DIR_SEPARATOR (SYSTEM_GDBINIT[start]); ++start)
-		continue;
-	      relocated_sysgdbinit = std::string (gdb_datadir) + SLASH_STRING +
-		&SYSTEM_GDBINIT[start];
-	    }
-	  else
-	    {
-	      char *relocated = relocate_path (gdb_program_name,
-					       SYSTEM_GDBINIT,
-					       SYSTEM_GDBINIT_RELOCATABLE);
-	      if (relocated != nullptr)
-	        {
-		  relocated_sysgdbinit = relocated;
-		  xfree (relocated);
-		}
-	    }
+	  std::string relocated_sysgdbinit =
+	    relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT);
 	  if (!relocated_sysgdbinit.empty () &&
 	      stat (relocated_sysgdbinit.c_str (), &s) == 0)
 	    sysgdbinit = relocated_sysgdbinit;
-- 
2.23.0.rc1.153.gdeed80330f-goog

Re: [PATCH 2/3] Factor out the code to do the datadir-relocation for gdbinit

[Sergio Durigan Junior]

On Wednesday, August 21 2019, Christian Biesinger wrote:

> On Wed, Aug 21, 2019 at 12:19 PM Sergio Durigan Junior
> <sergiodj@redhat.com> wrote:
>>
>> On Tuesday, August 20 2019, Christian Biesinger via gdb-patches wrote:
>>
>> > gdb/ChangeLog:
>> >
>> > 2019-08-20  Christian Biesinger  <cbiesinger@google.com>
>> >
>> >       * main.c (relocate_gdbinit_path_maybe_in_datadir): New function.
>> >       (get_init_files): Update.
>>
>> I'm afraid you'll need a descriptive commit message :-).
>
> Changed to:
>
> 2019-08-20  Christian Biesinger  <cbiesinger@google.com>
>
>         * main.c (relocate_gdbinit_path_maybe_in_datadir): Factor this code
>         out of get_init_files.
>         (get_init_files): Update.
>
> (I guess the title of the commit message is not enough?)

Sorry, I should have been more clear.

The changelog entry was fine; I was referring to the actual commit
message (the text you write before the changelog).  I think a very
simple text should be enough in this case, since it's a refactorization.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH 2/3] Factor out the code to do the datadir-relocation for gdbinit

[Christian Biesinger via gdb-patches]

On Wed, Aug 21, 2019 at 12:47 PM Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
>
> On Wednesday, August 21 2019, Christian Biesinger wrote:
>
> > On Wed, Aug 21, 2019 at 12:19 PM Sergio Durigan Junior
> > <sergiodj@redhat.com> wrote:
> >>
> >> On Tuesday, August 20 2019, Christian Biesinger via gdb-patches wrote:
> >>
> >> > gdb/ChangeLog:
> >> >
> >> > 2019-08-20  Christian Biesinger  <cbiesinger@google.com>
> >> >
> >> >       * main.c (relocate_gdbinit_path_maybe_in_datadir): New function.
> >> >       (get_init_files): Update.
> >>
> >> I'm afraid you'll need a descriptive commit message :-).
> >
> > Changed to:
> >
> > 2019-08-20  Christian Biesinger  <cbiesinger@google.com>
> >
> >         * main.c (relocate_gdbinit_path_maybe_in_datadir): Factor this code
> >         out of get_init_files.
> >         (get_init_files): Update.
> >
> > (I guess the title of the commit message is not enough?)
>
> Sorry, I should have been more clear.
>
> The changelog entry was fine; I was referring to the actual commit
> message (the text you write before the changelog).  I think a very
> simple text should be enough in this case, since it's a refactorization.

Oh OK, changed to the following:
    Factor out the code to do the datadir-relocation for gdbinit

    This simplifies get_init_files and makes it possible to reuse
    this code in an upcoming patch for SYSTEM_GDBINIT_DIR.

    gdb/ChangeLog:

    2019-08-20  Christian Biesinger  <cbiesinger@google.com>

            * main.c (relocate_gdbinit_path_maybe_in_datadir): Factor this code
            out of get_init_files.
            (get_init_files): Update.

(I assume it's fine not to resend the full patch, but let me know if
you want me to)

Christian

Re: [PATCH 2/3] Factor out the code to do the datadir-relocation for gdbinit

[Sergio Durigan Junior]

On Wednesday, August 21 2019, Christian Biesinger wrote:

> On Wed, Aug 21, 2019 at 12:47 PM Sergio Durigan Junior
> <sergiodj@redhat.com> wrote:
>>
>> On Wednesday, August 21 2019, Christian Biesinger wrote:
>>
>> > On Wed, Aug 21, 2019 at 12:19 PM Sergio Durigan Junior
>> > <sergiodj@redhat.com> wrote:
>> >>
>> >> On Tuesday, August 20 2019, Christian Biesinger via gdb-patches wrote:
>> >>
>> >> > gdb/ChangeLog:
>> >> >
>> >> > 2019-08-20  Christian Biesinger  <cbiesinger@google.com>
>> >> >
>> >> >       * main.c (relocate_gdbinit_path_maybe_in_datadir): New function.
>> >> >       (get_init_files): Update.
>> >>
>> >> I'm afraid you'll need a descriptive commit message :-).
>> >
>> > Changed to:
>> >
>> > 2019-08-20  Christian Biesinger  <cbiesinger@google.com>
>> >
>> >         * main.c (relocate_gdbinit_path_maybe_in_datadir): Factor this code
>> >         out of get_init_files.
>> >         (get_init_files): Update.
>> >
>> > (I guess the title of the commit message is not enough?)
>>
>> Sorry, I should have been more clear.
>>
>> The changelog entry was fine; I was referring to the actual commit
>> message (the text you write before the changelog).  I think a very
>> simple text should be enough in this case, since it's a refactorization.
>
> Oh OK, changed to the following:
>     Factor out the code to do the datadir-relocation for gdbinit
>
>     This simplifies get_init_files and makes it possible to reuse
>     this code in an upcoming patch for SYSTEM_GDBINIT_DIR.
>
>     gdb/ChangeLog:
>
>     2019-08-20  Christian Biesinger  <cbiesinger@google.com>
>
>             * main.c (relocate_gdbinit_path_maybe_in_datadir): Factor this code
>             out of get_init_files.
>             (get_init_files): Update.
>
> (I assume it's fine not to resend the full patch, but let me know if
> you want me to)

That's great, thanks.

I can't approve it as I'm not a GM, but this LGTM.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

[PATCH 3/3] Load system gdbinit files from a directory

[Christian Biesinger via gdb-patches]

gdb/ChangeLog:

2019-08-20  Christian Biesinger  <cbiesinger@google.com>

	* config.in: Add SYSTEM_GDBINIT_DIR.
	* configure: Regenerate.
	* configure.ac: Add new option --with-system-gdbinit-dir.
	* main.c (get_init_files): Change system_gdbinit argument to
	a vector and return the files in SYSTEM_GDBINIT_DIR in
	addition to SYSTEM_GDBINIT.
	(captured_main_1): Update.
	(print_gdb_help): Update.
---
 gdb/config.in    |  3 ++
 gdb/configure    | 77 ++++++++++++++++++++++++++++++++++++++++++++----
 gdb/configure.ac |  3 ++
 gdb/main.c       | 53 +++++++++++++++++++++++++++------
 4 files changed, 121 insertions(+), 15 deletions(-)

diff --git a/gdb/config.in b/gdb/config.in
index 26ca02f6a3..ca523fe4ab 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -684,6 +684,9 @@
 /* automatically load a system-wide gdbinit file */
 #undef SYSTEM_GDBINIT
 
+/* automatically load system-wide gdbinit files from this dir */
+#undef SYSTEM_GDBINIT_DIR
+
 /* Define if the system-gdbinit directory should be relocated when GDB is
    moved. */
 #undef SYSTEM_GDBINIT_RELOCATABLE
diff --git a/gdb/configure b/gdb/configure
index cb71bbf057..e5aa2e6b3b 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -693,6 +693,7 @@ WIN32LIBS
 SER_HARDWIRE
 WERROR_CFLAGS
 WARN_CFLAGS
+SYSTEM_GDBINIT_DIR
 SYSTEM_GDBINIT
 TARGET_SYSTEM_ROOT
 CONFIG_LDFLAGS
@@ -824,6 +825,7 @@ infodir
 docdir
 oldincludedir
 includedir
+runstatedir
 localstatedir
 sharedstatedir
 sysconfdir
@@ -884,6 +886,7 @@ with_libipt_prefix
 with_included_regex
 with_sysroot
 with_system_gdbinit
+with_system_gdbinit_dir
 enable_werror
 enable_build_warnings
 enable_gdb_build_warnings
@@ -956,6 +959,7 @@ datadir='${datarootdir}'
 sysconfdir='${prefix}/etc'
 sharedstatedir='${prefix}/com'
 localstatedir='${prefix}/var'
+runstatedir='${localstatedir}/run'
 includedir='${prefix}/include'
 oldincludedir='/usr/include'
 docdir='${datarootdir}/doc/${PACKAGE}'
@@ -1208,6 +1212,15 @@ do
   | -silent | --silent | --silen | --sile | --sil)
     silent=yes ;;
 
+  -runstatedir | --runstatedir | --runstatedi | --runstated \
+  | --runstate | --runstat | --runsta | --runst | --runs \
+  | --run | --ru | --r)
+    ac_prev=runstatedir ;;
+  -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
+  | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
+  | --run=* | --ru=* | --r=*)
+    runstatedir=$ac_optarg ;;
+
   -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
     ac_prev=sbindir ;;
   -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1345,7 +1358,7 @@ fi
 for ac_var in	exec_prefix prefix bindir sbindir libexecdir datarootdir \
 		datadir sysconfdir sharedstatedir localstatedir includedir \
 		oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
-		libdir localedir mandir
+		libdir localedir mandir runstatedir
 do
   eval ac_val=\$$ac_var
   # Remove trailing slashes.
@@ -1498,6 +1511,7 @@ Fine tuning of the installation directories:
   --sysconfdir=DIR        read-only single-machine data [PREFIX/etc]
   --sharedstatedir=DIR    modifiable architecture-independent data [PREFIX/com]
   --localstatedir=DIR     modifiable single-machine data [PREFIX/var]
+  --runstatedir=DIR       modifiable per-process data [LOCALSTATEDIR/run]
   --libdir=DIR            object code libraries [EPREFIX/lib]
   --includedir=DIR        C header files [PREFIX/include]
   --oldincludedir=DIR     C header files for non-gcc [/usr/include]
@@ -1618,6 +1632,9 @@ Optional Packages:
   --with-sysroot[=DIR]    search for usr/lib et al within DIR
   --with-system-gdbinit=PATH
                           automatically load a system-wide gdbinit file
+  --with-system-gdbinit-dir=PATH
+                          automatically load system-wide gdbinit files from
+                          this directory
   --with-lzma             support lzma compression (auto/yes/no)
   --with-liblzma-prefix[=DIR]  search for liblzma in DIR/include and DIR/lib
   --without-liblzma-prefix     don't search for liblzma in includedir and libdir
@@ -4683,7 +4700,7 @@ else
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -4729,7 +4746,7 @@ else
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -4753,7 +4770,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -4798,7 +4815,7 @@ else
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -4822,7 +4839,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
     We can't simply define LARGE_OFF_T to be 9223372036854775807,
     since some C++ compilers masquerading as C compilers
     incorrectly reject 9223372036854775807.  */
-#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
+#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
   int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
 		       && LARGE_OFF_T % 2147483647 == 1)
 		      ? 1 : -1];
@@ -12872,6 +12889,8 @@ main ()
     if (*(data + i) != *(data3 + i))
       return 14;
   close (fd);
+  free (data);
+  free (data3);
   return 0;
 }
 _ACEOF
@@ -15239,6 +15258,52 @@ _ACEOF
 
 
 
+# Check whether --with-system-gdbinit-dir was given.
+if test "${with_system_gdbinit_dir+set}" = set; then :
+  withval=$with_system_gdbinit_dir;
+    SYSTEM_GDBINIT_DIR=$withval
+else
+  SYSTEM_GDBINIT_DIR=
+fi
+
+
+  test "x$prefix" = xNONE && prefix="$ac_default_prefix"
+  test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
+  ac_define_dir=`eval echo $SYSTEM_GDBINIT_DIR`
+  ac_define_dir=`eval echo $ac_define_dir`
+
+cat >>confdefs.h <<_ACEOF
+#define SYSTEM_GDBINIT_DIR "$ac_define_dir"
+_ACEOF
+
+
+
+
+  if test "x$exec_prefix" = xNONE || test "x$exec_prefix" = 'x${prefix}'; then
+     if test "x$prefix" = xNONE; then
+     	test_prefix=/usr/local
+     else
+	test_prefix=$prefix
+     fi
+  else
+     test_prefix=$exec_prefix
+  fi
+  value=0
+  case ${ac_define_dir} in
+     "${test_prefix}"|"${test_prefix}/"*|\
+	'${exec_prefix}'|'${exec_prefix}/'*)
+     value=1
+     ;;
+  esac
+
+cat >>confdefs.h <<_ACEOF
+#define SYSTEM_GDBINIT_DIR_RELOCATABLE $value
+_ACEOF
+
+
+
+
+
 # Check whether --enable-werror was given.
 if test "${enable_werror+set}" = set; then :
   enableval=$enable_werror; case "${enableval}" in
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 5a18c16405..27d67ead3e 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1844,6 +1844,9 @@ GDB_AC_DEFINE_RELOCATABLE(TARGET_SYSTEM_ROOT, sysroot, ${ac_define_dir})
 GDB_AC_WITH_DIR(SYSTEM_GDBINIT, system-gdbinit,
     [automatically load a system-wide gdbinit file],
     [])
+GDB_AC_WITH_DIR(SYSTEM_GDBINIT_DIR, system-gdbinit-dir,
+    [automatically load system-wide gdbinit files from this directory],
+    [])
 
 AM_GDB_WARNINGS
 AM_GDB_UBSAN
diff --git a/gdb/main.c b/gdb/main.c
index a1d1904c9b..39957db4bd 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -45,6 +45,7 @@
 #include "event-top.h"
 #include "infrun.h"
 #include "gdbsupport/signals-state-save-restore.h"
+#include <algorithm>
 #include <vector>
 #include "gdbsupport/pathstuff.h"
 #include "cli/cli-style.h"
@@ -232,11 +233,11 @@ static std::string relocate_gdbinit_path_maybe_in_datadir (std::string file)
    to be loaded, then SYSTEM_GDBINIT (resp. HOME_GDBINIT and
    LOCAL_GDBINIT) is set to the empty string.  */
 static void
-get_init_files (std::string *system_gdbinit,
+get_init_files (std::vector<std::string> *system_gdbinit,
 		std::string *home_gdbinit,
 		std::string *local_gdbinit)
 {
-  static std::string sysgdbinit;
+  static std::vector<std::string> sysgdbinit;
   static std::string homeinit;
   static std::string localinit;
   static int initialized = 0;
@@ -251,7 +252,28 @@ get_init_files (std::string *system_gdbinit,
 	    relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT);
 	  if (!relocated_sysgdbinit.empty () &&
 	      stat (relocated_sysgdbinit.c_str (), &s) == 0)
-	    sysgdbinit = relocated_sysgdbinit;
+	    sysgdbinit.push_back(relocated_sysgdbinit);
+	}
+      if (SYSTEM_GDBINIT_DIR[0])
+        {
+	  std::string relocated_gdbinit_dir =
+	    relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT_DIR);
+	  DIR* dir;
+	  if (!relocated_gdbinit_dir.empty () &&
+	    (dir = opendir (relocated_gdbinit_dir.c_str ())) != nullptr)
+	    {
+	      std::vector<std::string> files;
+	      struct dirent* ent;
+	      while ((ent = readdir (dir)) != nullptr)
+	        {
+		  if (ent->d_name[0] != '.')
+		    files.push_back (relocated_gdbinit_dir + SLASH_STRING +
+				     ent->d_name);
+		}
+	      closedir (dir);
+	      std::sort (files.begin (), files.end ());
+	      sysgdbinit.insert (sysgdbinit.end (), files.begin (), files.end ());
+	    }
 	}
 
       const char *homedir = getenv ("HOME");
@@ -909,7 +931,8 @@ captured_main_1 (struct captured_main_args *context)
   /* Lookup gdbinit files.  Note that the gdbinit file name may be
      overriden during file initialization, so get_init_files should be
      called after gdb_init.  */
-  std::string system_gdbinit, home_gdbinit, local_gdbinit;
+  std::vector<std::string> system_gdbinit;
+  std::string home_gdbinit, local_gdbinit;
   get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
 
   /* Do these (and anything which might call wrap_here or *_filtered)
@@ -987,7 +1010,10 @@ captured_main_1 (struct captured_main_args *context)
      processed; it sets global parameters, which are independent of
      what file you are debugging or what directory you are in.  */
   if (!system_gdbinit.empty () && !inhibit_gdbinit)
-    ret = catch_command_errors (source_script, system_gdbinit.c_str (), 0);
+    {
+      for (const std::string& file : system_gdbinit)
+	ret = catch_command_errors (source_script, file.c_str (), 0);
+    }
 
   /* Read and execute $HOME/.gdbinit file, if it exists.  This is done
      *before* all the command line arguments are processed; it sets
@@ -1205,7 +1231,7 @@ gdb_main (struct captured_main_args *args)
 static void
 print_gdb_help (struct ui_file *stream)
 {
-  std::string system_gdbinit;
+  std::vector<std::string> system_gdbinit;
   std::string home_gdbinit;
   std::string local_gdbinit;
 
@@ -1286,9 +1312,18 @@ Other options:\n\n\
 At startup, GDB reads the following init files and executes their commands:\n\
 "), stream);
   if (!system_gdbinit.empty ())
-    fprintf_unfiltered (stream, _("\
-   * system-wide init file: %s\n\
-"), system_gdbinit.c_str ());
+    {
+      std::string output;
+      for (size_t idx = 0; idx < system_gdbinit.size(); ++idx)
+        {
+	  output += system_gdbinit[idx];
+	  if (idx < system_gdbinit.size() - 1)
+	    output += ", ";
+	}
+      fprintf_unfiltered (stream, _("\
+   * system-wide init files: %s\n\
+"), output.c_str ());
+    }
   if (!home_gdbinit.empty ())
     fprintf_unfiltered (stream, _("\
    * user-specific init file: %s\n\
-- 
2.23.0.rc1.153.gdeed80330f-goog

Re: [PATCH 3/3] Load system gdbinit files from a directory

[Sergio Durigan Junior]

On Tuesday, August 20 2019, Christian Biesinger via gdb-patches wrote:

> gdb/ChangeLog:
>
> 2019-08-20  Christian Biesinger  <cbiesinger@google.com>
>
> 	* config.in: Add SYSTEM_GDBINIT_DIR.

config.in is also regenerated.

> 	* configure: Regenerate.
> 	* configure.ac: Add new option --with-system-gdbinit-dir.
> 	* main.c (get_init_files): Change system_gdbinit argument to
> 	a vector and return the files in SYSTEM_GDBINIT_DIR in
> 	addition to SYSTEM_GDBINIT.
> 	(captured_main_1): Update.
> 	(print_gdb_help): Update.

You'll need a descriptive commit log.

> ---
>  gdb/config.in    |  3 ++
>  gdb/configure    | 77 ++++++++++++++++++++++++++++++++++++++++++++----
>  gdb/configure.ac |  3 ++
>  gdb/main.c       | 53 +++++++++++++++++++++++++++------
>  4 files changed, 121 insertions(+), 15 deletions(-)
>
> diff --git a/gdb/config.in b/gdb/config.in
> index 26ca02f6a3..ca523fe4ab 100644
> --- a/gdb/config.in
> +++ b/gdb/config.in
> @@ -684,6 +684,9 @@
>  /* automatically load a system-wide gdbinit file */
>  #undef SYSTEM_GDBINIT
>  
> +/* automatically load system-wide gdbinit files from this dir */
> +#undef SYSTEM_GDBINIT_DIR
> +
>  /* Define if the system-gdbinit directory should be relocated when GDB is
>     moved. */
>  #undef SYSTEM_GDBINIT_RELOCATABLE
> diff --git a/gdb/configure b/gdb/configure
> index cb71bbf057..e5aa2e6b3b 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -693,6 +693,7 @@ WIN32LIBS
>  SER_HARDWIRE
>  WERROR_CFLAGS
>  WARN_CFLAGS
> +SYSTEM_GDBINIT_DIR
>  SYSTEM_GDBINIT
>  TARGET_SYSTEM_ROOT
>  CONFIG_LDFLAGS
> @@ -824,6 +825,7 @@ infodir
>  docdir
>  oldincludedir
>  includedir
> +runstatedir
>  localstatedir
>  sharedstatedir
>  sysconfdir
> @@ -884,6 +886,7 @@ with_libipt_prefix
>  with_included_regex
>  with_sysroot
>  with_system_gdbinit
> +with_system_gdbinit_dir
>  enable_werror
>  enable_build_warnings
>  enable_gdb_build_warnings
> @@ -956,6 +959,7 @@ datadir='${datarootdir}'
>  sysconfdir='${prefix}/etc'
>  sharedstatedir='${prefix}/com'
>  localstatedir='${prefix}/var'
> +runstatedir='${localstatedir}/run'
>  includedir='${prefix}/include'
>  oldincludedir='/usr/include'
>  docdir='${datarootdir}/doc/${PACKAGE}'
> @@ -1208,6 +1212,15 @@ do
>    | -silent | --silent | --silen | --sile | --sil)
>      silent=yes ;;
>  
> +  -runstatedir | --runstatedir | --runstatedi | --runstated \
> +  | --runstate | --runstat | --runsta | --runst | --runs \
> +  | --run | --ru | --r)
> +    ac_prev=runstatedir ;;
> +  -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
> +  | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
> +  | --run=* | --ru=* | --r=*)
> +    runstatedir=$ac_optarg ;;
> +
>    -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
>      ac_prev=sbindir ;;
>    -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
> @@ -1345,7 +1358,7 @@ fi
>  for ac_var in	exec_prefix prefix bindir sbindir libexecdir datarootdir \
>  		datadir sysconfdir sharedstatedir localstatedir includedir \
>  		oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
> -		libdir localedir mandir
> +		libdir localedir mandir runstatedir
>  do
>    eval ac_val=\$$ac_var
>    # Remove trailing slashes.
> @@ -1498,6 +1511,7 @@ Fine tuning of the installation directories:
>    --sysconfdir=DIR        read-only single-machine data [PREFIX/etc]
>    --sharedstatedir=DIR    modifiable architecture-independent data [PREFIX/com]
>    --localstatedir=DIR     modifiable single-machine data [PREFIX/var]
> +  --runstatedir=DIR       modifiable per-process data [LOCALSTATEDIR/run]
>    --libdir=DIR            object code libraries [EPREFIX/lib]
>    --includedir=DIR        C header files [PREFIX/include]
>    --oldincludedir=DIR     C header files for non-gcc [/usr/include]
> @@ -1618,6 +1632,9 @@ Optional Packages:
>    --with-sysroot[=DIR]    search for usr/lib et al within DIR
>    --with-system-gdbinit=PATH
>                            automatically load a system-wide gdbinit file
> +  --with-system-gdbinit-dir=PATH
> +                          automatically load system-wide gdbinit files from
> +                          this directory
>    --with-lzma             support lzma compression (auto/yes/no)
>    --with-liblzma-prefix[=DIR]  search for liblzma in DIR/include and DIR/lib
>    --without-liblzma-prefix     don't search for liblzma in includedir and libdir
> @@ -4683,7 +4700,7 @@ else
>      We can't simply define LARGE_OFF_T to be 9223372036854775807,
>      since some C++ compilers masquerading as C compilers
>      incorrectly reject 9223372036854775807.  */
> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>  		       && LARGE_OFF_T % 2147483647 == 1)
>  		      ? 1 : -1];
> @@ -4729,7 +4746,7 @@ else
>      We can't simply define LARGE_OFF_T to be 9223372036854775807,
>      since some C++ compilers masquerading as C compilers
>      incorrectly reject 9223372036854775807.  */
> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>  		       && LARGE_OFF_T % 2147483647 == 1)
>  		      ? 1 : -1];
> @@ -4753,7 +4770,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>      We can't simply define LARGE_OFF_T to be 9223372036854775807,
>      since some C++ compilers masquerading as C compilers
>      incorrectly reject 9223372036854775807.  */
> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>  		       && LARGE_OFF_T % 2147483647 == 1)
>  		      ? 1 : -1];
> @@ -4798,7 +4815,7 @@ else
>      We can't simply define LARGE_OFF_T to be 9223372036854775807,
>      since some C++ compilers masquerading as C compilers
>      incorrectly reject 9223372036854775807.  */
> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>  		       && LARGE_OFF_T % 2147483647 == 1)
>  		      ? 1 : -1];
> @@ -4822,7 +4839,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>      We can't simply define LARGE_OFF_T to be 9223372036854775807,
>      since some C++ compilers masquerading as C compilers
>      incorrectly reject 9223372036854775807.  */
> -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
>    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
>  		       && LARGE_OFF_T % 2147483647 == 1)
>  		      ? 1 : -1];
> @@ -12872,6 +12889,8 @@ main ()
>      if (*(data + i) != *(data3 + i))
>        return 14;
>    close (fd);
> +  free (data);
> +  free (data3);
>    return 0;
>  }
>  _ACEOF
> @@ -15239,6 +15258,52 @@ _ACEOF
>  
>  
>  
> +# Check whether --with-system-gdbinit-dir was given.
> +if test "${with_system_gdbinit_dir+set}" = set; then :
> +  withval=$with_system_gdbinit_dir;
> +    SYSTEM_GDBINIT_DIR=$withval
> +else
> +  SYSTEM_GDBINIT_DIR=
> +fi
> +
> +
> +  test "x$prefix" = xNONE && prefix="$ac_default_prefix"
> +  test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
> +  ac_define_dir=`eval echo $SYSTEM_GDBINIT_DIR`
> +  ac_define_dir=`eval echo $ac_define_dir`
> +
> +cat >>confdefs.h <<_ACEOF
> +#define SYSTEM_GDBINIT_DIR "$ac_define_dir"
> +_ACEOF
> +
> +
> +
> +
> +  if test "x$exec_prefix" = xNONE || test "x$exec_prefix" = 'x${prefix}'; then
> +     if test "x$prefix" = xNONE; then
> +     	test_prefix=/usr/local
> +     else
> +	test_prefix=$prefix
> +     fi
> +  else
> +     test_prefix=$exec_prefix
> +  fi
> +  value=0
> +  case ${ac_define_dir} in
> +     "${test_prefix}"|"${test_prefix}/"*|\
> +	'${exec_prefix}'|'${exec_prefix}/'*)
> +     value=1
> +     ;;
> +  esac
> +
> +cat >>confdefs.h <<_ACEOF
> +#define SYSTEM_GDBINIT_DIR_RELOCATABLE $value
> +_ACEOF
> +
> +
> +
> +
> +
>  # Check whether --enable-werror was given.
>  if test "${enable_werror+set}" = set; then :
>    enableval=$enable_werror; case "${enableval}" in
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 5a18c16405..27d67ead3e 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -1844,6 +1844,9 @@ GDB_AC_DEFINE_RELOCATABLE(TARGET_SYSTEM_ROOT, sysroot, ${ac_define_dir})
>  GDB_AC_WITH_DIR(SYSTEM_GDBINIT, system-gdbinit,
>      [automatically load a system-wide gdbinit file],
>      [])
> +GDB_AC_WITH_DIR(SYSTEM_GDBINIT_DIR, system-gdbinit-dir,
> +    [automatically load system-wide gdbinit files from this directory],
> +    [])
>  
>  AM_GDB_WARNINGS
>  AM_GDB_UBSAN
> diff --git a/gdb/main.c b/gdb/main.c
> index a1d1904c9b..39957db4bd 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -45,6 +45,7 @@
>  #include "event-top.h"
>  #include "infrun.h"
>  #include "gdbsupport/signals-state-save-restore.h"
> +#include <algorithm>
>  #include <vector>
>  #include "gdbsupport/pathstuff.h"
>  #include "cli/cli-style.h"
> @@ -232,11 +233,11 @@ static std::string relocate_gdbinit_path_maybe_in_datadir (std::string file)
>     to be loaded, then SYSTEM_GDBINIT (resp. HOME_GDBINIT and
>     LOCAL_GDBINIT) is set to the empty string.  */
>  static void
> -get_init_files (std::string *system_gdbinit,
> +get_init_files (std::vector<std::string> *system_gdbinit,
>  		std::string *home_gdbinit,
>  		std::string *local_gdbinit)
>  {
> -  static std::string sysgdbinit;
> +  static std::vector<std::string> sysgdbinit;
>    static std::string homeinit;
>    static std::string localinit;
>    static int initialized = 0;
> @@ -251,7 +252,28 @@ get_init_files (std::string *system_gdbinit,
>  	    relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT);
>  	  if (!relocated_sysgdbinit.empty () &&
>  	      stat (relocated_sysgdbinit.c_str (), &s) == 0)
> -	    sysgdbinit = relocated_sysgdbinit;
> +	    sysgdbinit.push_back(relocated_sysgdbinit);
> +	}
> +      if (SYSTEM_GDBINIT_DIR[0])
> +        {

The indentation looks funny here.

> +	  std::string relocated_gdbinit_dir =
> +	    relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT_DIR);
> +	  DIR* dir;
> +	  if (!relocated_gdbinit_dir.empty () &&
> +	    (dir = opendir (relocated_gdbinit_dir.c_str ())) != nullptr)

I may be wrong, but I think we discourage variable assignment inside
conditions (but things are changing so fast with C++ that I don't know
anymore!).

> +	    {
> +	      std::vector<std::string> files;
> +	      struct dirent* ent;
> +	      while ((ent = readdir (dir)) != nullptr)

Likewise.

> +	        {
> +		  if (ent->d_name[0] != '.')

I think we should also check 'ent->d_type' and make sure it's DT_REG (in
which case, the 'ent->d_name' check above could be removed).  It's
questionable whether we should also deal DT_LINK, but I don't think so
(for now).

> +		    files.push_back (relocated_gdbinit_dir + SLASH_STRING +
> +				     ent->d_name);
> +		}
> +	      closedir (dir);
> +	      std::sort (files.begin (), files.end ());
> +	      sysgdbinit.insert (sysgdbinit.end (), files.begin (), files.end ());
> +	    }
>  	}
>  
>        const char *homedir = getenv ("HOME");
> @@ -909,7 +931,8 @@ captured_main_1 (struct captured_main_args *context)
>    /* Lookup gdbinit files.  Note that the gdbinit file name may be
>       overriden during file initialization, so get_init_files should be
>       called after gdb_init.  */
> -  std::string system_gdbinit, home_gdbinit, local_gdbinit;
> +  std::vector<std::string> system_gdbinit;
> +  std::string home_gdbinit, local_gdbinit;
>    get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
>  
>    /* Do these (and anything which might call wrap_here or *_filtered)
> @@ -987,7 +1010,10 @@ captured_main_1 (struct captured_main_args *context)
>       processed; it sets global parameters, which are independent of
>       what file you are debugging or what directory you are in.  */
>    if (!system_gdbinit.empty () && !inhibit_gdbinit)
> -    ret = catch_command_errors (source_script, system_gdbinit.c_str (), 0);
> +    {
> +      for (const std::string& file : system_gdbinit)
> +	ret = catch_command_errors (source_script, file.c_str (), 0);
> +    }
>  
>    /* Read and execute $HOME/.gdbinit file, if it exists.  This is done
>       *before* all the command line arguments are processed; it sets
> @@ -1205,7 +1231,7 @@ gdb_main (struct captured_main_args *args)
>  static void
>  print_gdb_help (struct ui_file *stream)
>  {
> -  std::string system_gdbinit;
> +  std::vector<std::string> system_gdbinit;
>    std::string home_gdbinit;
>    std::string local_gdbinit;
>  
> @@ -1286,9 +1312,18 @@ Other options:\n\n\
>  At startup, GDB reads the following init files and executes their commands:\n\
>  "), stream);
>    if (!system_gdbinit.empty ())
> -    fprintf_unfiltered (stream, _("\
> -   * system-wide init file: %s\n\
> -"), system_gdbinit.c_str ());
> +    {
> +      std::string output;
> +      for (size_t idx = 0; idx < system_gdbinit.size(); ++idx)

Space between variable name and parens.

> +        {
> +	  output += system_gdbinit[idx];
> +	  if (idx < system_gdbinit.size() - 1)
> +	    output += ", ";
> +	}
> +      fprintf_unfiltered (stream, _("\
> +   * system-wide init files: %s\n\
> +"), output.c_str ());
> +    }
>    if (!home_gdbinit.empty ())
>      fprintf_unfiltered (stream, _("\
>     * user-specific init file: %s\n\
> -- 
> 2.23.0.rc1.153.gdeed80330f-goog

This looks good, but it needs a testcase, documentation updates and a
NEWS entry.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH 3/3] Load system gdbinit files from a directory

[Christian Biesinger via gdb-patches]

On Wed, Aug 21, 2019 at 1:32 PM Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
>
> On Tuesday, August 20 2019, Christian Biesinger via gdb-patches wrote:
>
> > gdb/ChangeLog:
> >
> > 2019-08-20  Christian Biesinger  <cbiesinger@google.com>
> >
> >       * config.in: Add SYSTEM_GDBINIT_DIR.
>
> config.in is also regenerated.

Done.

> >       * configure: Regenerate.
> >       * configure.ac: Add new option --with-system-gdbinit-dir.
> >       * main.c (get_init_files): Change system_gdbinit argument to
> >       a vector and return the files in SYSTEM_GDBINIT_DIR in
> >       addition to SYSTEM_GDBINIT.
> >       (captured_main_1): Update.
> >       (print_gdb_help): Update.
>
> You'll need a descriptive commit log.

Done.

> > ---
> >  gdb/config.in    |  3 ++
> >  gdb/configure    | 77 ++++++++++++++++++++++++++++++++++++++++++++----
> >  gdb/configure.ac |  3 ++
> >  gdb/main.c       | 53 +++++++++++++++++++++++++++------
> >  4 files changed, 121 insertions(+), 15 deletions(-)
> >
> > diff --git a/gdb/config.in b/gdb/config.in
> > index 26ca02f6a3..ca523fe4ab 100644
> > --- a/gdb/config.in
> > +++ b/gdb/config.in
> > @@ -684,6 +684,9 @@
> >  /* automatically load a system-wide gdbinit file */
> >  #undef SYSTEM_GDBINIT
> >
> > +/* automatically load system-wide gdbinit files from this dir */
> > +#undef SYSTEM_GDBINIT_DIR
> > +
> >  /* Define if the system-gdbinit directory should be relocated when GDB is
> >     moved. */
> >  #undef SYSTEM_GDBINIT_RELOCATABLE
> > diff --git a/gdb/configure b/gdb/configure
> > index cb71bbf057..e5aa2e6b3b 100755
> > --- a/gdb/configure
> > +++ b/gdb/configure
> > @@ -693,6 +693,7 @@ WIN32LIBS
> >  SER_HARDWIRE
> >  WERROR_CFLAGS
> >  WARN_CFLAGS
> > +SYSTEM_GDBINIT_DIR
> >  SYSTEM_GDBINIT
> >  TARGET_SYSTEM_ROOT
> >  CONFIG_LDFLAGS
> > @@ -824,6 +825,7 @@ infodir
> >  docdir
> >  oldincludedir
> >  includedir
> > +runstatedir
> >  localstatedir
> >  sharedstatedir
> >  sysconfdir
> > @@ -884,6 +886,7 @@ with_libipt_prefix
> >  with_included_regex
> >  with_sysroot
> >  with_system_gdbinit
> > +with_system_gdbinit_dir
> >  enable_werror
> >  enable_build_warnings
> >  enable_gdb_build_warnings
> > @@ -956,6 +959,7 @@ datadir='${datarootdir}'
> >  sysconfdir='${prefix}/etc'
> >  sharedstatedir='${prefix}/com'
> >  localstatedir='${prefix}/var'
> > +runstatedir='${localstatedir}/run'
> >  includedir='${prefix}/include'
> >  oldincludedir='/usr/include'
> >  docdir='${datarootdir}/doc/${PACKAGE}'
> > @@ -1208,6 +1212,15 @@ do
> >    | -silent | --silent | --silen | --sile | --sil)
> >      silent=yes ;;
> >
> > +  -runstatedir | --runstatedir | --runstatedi | --runstated \
> > +  | --runstate | --runstat | --runsta | --runst | --runs \
> > +  | --run | --ru | --r)
> > +    ac_prev=runstatedir ;;
> > +  -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
> > +  | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
> > +  | --run=* | --ru=* | --r=*)
> > +    runstatedir=$ac_optarg ;;
> > +
> >    -sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
> >      ac_prev=sbindir ;;
> >    -sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
> > @@ -1345,7 +1358,7 @@ fi
> >  for ac_var in        exec_prefix prefix bindir sbindir libexecdir datarootdir \
> >               datadir sysconfdir sharedstatedir localstatedir includedir \
> >               oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
> > -             libdir localedir mandir
> > +             libdir localedir mandir runstatedir
> >  do
> >    eval ac_val=\$$ac_var
> >    # Remove trailing slashes.
> > @@ -1498,6 +1511,7 @@ Fine tuning of the installation directories:
> >    --sysconfdir=DIR        read-only single-machine data [PREFIX/etc]
> >    --sharedstatedir=DIR    modifiable architecture-independent data [PREFIX/com]
> >    --localstatedir=DIR     modifiable single-machine data [PREFIX/var]
> > +  --runstatedir=DIR       modifiable per-process data [LOCALSTATEDIR/run]
> >    --libdir=DIR            object code libraries [EPREFIX/lib]
> >    --includedir=DIR        C header files [PREFIX/include]
> >    --oldincludedir=DIR     C header files for non-gcc [/usr/include]
> > @@ -1618,6 +1632,9 @@ Optional Packages:
> >    --with-sysroot[=DIR]    search for usr/lib et al within DIR
> >    --with-system-gdbinit=PATH
> >                            automatically load a system-wide gdbinit file
> > +  --with-system-gdbinit-dir=PATH
> > +                          automatically load system-wide gdbinit files from
> > +                          this directory
> >    --with-lzma             support lzma compression (auto/yes/no)
> >    --with-liblzma-prefix[=DIR]  search for liblzma in DIR/include and DIR/lib
> >    --without-liblzma-prefix     don't search for liblzma in includedir and libdir
> > @@ -4683,7 +4700,7 @@ else
> >      We can't simply define LARGE_OFF_T to be 9223372036854775807,
> >      since some C++ compilers masquerading as C compilers
> >      incorrectly reject 9223372036854775807.  */
> > -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> > +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
> >    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
> >                      && LARGE_OFF_T % 2147483647 == 1)
> >                     ? 1 : -1];
> > @@ -4729,7 +4746,7 @@ else
> >      We can't simply define LARGE_OFF_T to be 9223372036854775807,
> >      since some C++ compilers masquerading as C compilers
> >      incorrectly reject 9223372036854775807.  */
> > -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> > +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
> >    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
> >                      && LARGE_OFF_T % 2147483647 == 1)
> >                     ? 1 : -1];
> > @@ -4753,7 +4770,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
> >      We can't simply define LARGE_OFF_T to be 9223372036854775807,
> >      since some C++ compilers masquerading as C compilers
> >      incorrectly reject 9223372036854775807.  */
> > -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> > +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
> >    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
> >                      && LARGE_OFF_T % 2147483647 == 1)
> >                     ? 1 : -1];
> > @@ -4798,7 +4815,7 @@ else
> >      We can't simply define LARGE_OFF_T to be 9223372036854775807,
> >      since some C++ compilers masquerading as C compilers
> >      incorrectly reject 9223372036854775807.  */
> > -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> > +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
> >    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
> >                      && LARGE_OFF_T % 2147483647 == 1)
> >                     ? 1 : -1];
> > @@ -4822,7 +4839,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
> >      We can't simply define LARGE_OFF_T to be 9223372036854775807,
> >      since some C++ compilers masquerading as C compilers
> >      incorrectly reject 9223372036854775807.  */
> > -#define LARGE_OFF_T (((off_t) 1 << 62) - 1 + ((off_t) 1 << 62))
> > +#define LARGE_OFF_T ((((off_t) 1 << 31) << 31) - 1 + (((off_t) 1 << 31) << 31))
> >    int off_t_is_large[(LARGE_OFF_T % 2147483629 == 721
> >                      && LARGE_OFF_T % 2147483647 == 1)
> >                     ? 1 : -1];
> > @@ -12872,6 +12889,8 @@ main ()
> >      if (*(data + i) != *(data3 + i))
> >        return 14;
> >    close (fd);
> > +  free (data);
> > +  free (data3);
> >    return 0;
> >  }
> >  _ACEOF
> > @@ -15239,6 +15258,52 @@ _ACEOF
> >
> >
> >
> > +# Check whether --with-system-gdbinit-dir was given.
> > +if test "${with_system_gdbinit_dir+set}" = set; then :
> > +  withval=$with_system_gdbinit_dir;
> > +    SYSTEM_GDBINIT_DIR=$withval
> > +else
> > +  SYSTEM_GDBINIT_DIR=
> > +fi
> > +
> > +
> > +  test "x$prefix" = xNONE && prefix="$ac_default_prefix"
> > +  test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
> > +  ac_define_dir=`eval echo $SYSTEM_GDBINIT_DIR`
> > +  ac_define_dir=`eval echo $ac_define_dir`
> > +
> > +cat >>confdefs.h <<_ACEOF
> > +#define SYSTEM_GDBINIT_DIR "$ac_define_dir"
> > +_ACEOF
> > +
> > +
> > +
> > +
> > +  if test "x$exec_prefix" = xNONE || test "x$exec_prefix" = 'x${prefix}'; then
> > +     if test "x$prefix" = xNONE; then
> > +             test_prefix=/usr/local
> > +     else
> > +     test_prefix=$prefix
> > +     fi
> > +  else
> > +     test_prefix=$exec_prefix
> > +  fi
> > +  value=0
> > +  case ${ac_define_dir} in
> > +     "${test_prefix}"|"${test_prefix}/"*|\
> > +     '${exec_prefix}'|'${exec_prefix}/'*)
> > +     value=1
> > +     ;;
> > +  esac
> > +
> > +cat >>confdefs.h <<_ACEOF
> > +#define SYSTEM_GDBINIT_DIR_RELOCATABLE $value
> > +_ACEOF
> > +
> > +
> > +
> > +
> > +
> >  # Check whether --enable-werror was given.
> >  if test "${enable_werror+set}" = set; then :
> >    enableval=$enable_werror; case "${enableval}" in
> > diff --git a/gdb/configure.ac b/gdb/configure.ac
> > index 5a18c16405..27d67ead3e 100644
> > --- a/gdb/configure.ac
> > +++ b/gdb/configure.ac
> > @@ -1844,6 +1844,9 @@ GDB_AC_DEFINE_RELOCATABLE(TARGET_SYSTEM_ROOT, sysroot, ${ac_define_dir})
> >  GDB_AC_WITH_DIR(SYSTEM_GDBINIT, system-gdbinit,
> >      [automatically load a system-wide gdbinit file],
> >      [])
> > +GDB_AC_WITH_DIR(SYSTEM_GDBINIT_DIR, system-gdbinit-dir,
> > +    [automatically load system-wide gdbinit files from this directory],
> > +    [])
> >
> >  AM_GDB_WARNINGS
> >  AM_GDB_UBSAN
> > diff --git a/gdb/main.c b/gdb/main.c
> > index a1d1904c9b..39957db4bd 100644
> > --- a/gdb/main.c
> > +++ b/gdb/main.c
> > @@ -45,6 +45,7 @@
> >  #include "event-top.h"
> >  #include "infrun.h"
> >  #include "gdbsupport/signals-state-save-restore.h"
> > +#include <algorithm>
> >  #include <vector>
> >  #include "gdbsupport/pathstuff.h"
> >  #include "cli/cli-style.h"
> > @@ -232,11 +233,11 @@ static std::string relocate_gdbinit_path_maybe_in_datadir (std::string file)
> >     to be loaded, then SYSTEM_GDBINIT (resp. HOME_GDBINIT and
> >     LOCAL_GDBINIT) is set to the empty string.  */
> >  static void
> > -get_init_files (std::string *system_gdbinit,
> > +get_init_files (std::vector<std::string> *system_gdbinit,
> >               std::string *home_gdbinit,
> >               std::string *local_gdbinit)
> >  {
> > -  static std::string sysgdbinit;
> > +  static std::vector<std::string> sysgdbinit;
> >    static std::string homeinit;
> >    static std::string localinit;
> >    static int initialized = 0;
> > @@ -251,7 +252,28 @@ get_init_files (std::string *system_gdbinit,
> >           relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT);
> >         if (!relocated_sysgdbinit.empty () &&
> >             stat (relocated_sysgdbinit.c_str (), &s) == 0)
> > -         sysgdbinit = relocated_sysgdbinit;
> > +         sysgdbinit.push_back(relocated_sysgdbinit);
> > +     }
> > +      if (SYSTEM_GDBINIT_DIR[0])
> > +        {
>
> The indentation looks funny here.

Can you elaborate? I think it is correct, but perhaps the diff makes
it look weird because the + is messing with the tabs.

> > +       std::string relocated_gdbinit_dir =
> > +         relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT_DIR);
> > +       DIR* dir;
> > +       if (!relocated_gdbinit_dir.empty () &&
> > +         (dir = opendir (relocated_gdbinit_dir.c_str ())) != nullptr)
>
> I may be wrong, but I think we discourage variable assignment inside
> conditions (but things are changing so fast with C++ that I don't know
> anymore!).

Done.

> > +         {
> > +           std::vector<std::string> files;
> > +           struct dirent* ent;
> > +           while ((ent = readdir (dir)) != nullptr)
>
> Likewise.

Done.

> > +             {
> > +               if (ent->d_name[0] != '.')
>
> I think we should also check 'ent->d_type' and make sure it's DT_REG (in
> which case, the 'ent->d_name' check above could be removed).  It's
> questionable whether we should also deal DT_LINK, but I don't think so
> (for now).

Done.

> > +                 files.push_back (relocated_gdbinit_dir + SLASH_STRING +
> > +                                  ent->d_name);
> > +             }
> > +           closedir (dir);
> > +           std::sort (files.begin (), files.end ());
> > +           sysgdbinit.insert (sysgdbinit.end (), files.begin (), files.end ());
> > +         }
> >       }
> >
> >        const char *homedir = getenv ("HOME");
> > @@ -909,7 +931,8 @@ captured_main_1 (struct captured_main_args *context)
> >    /* Lookup gdbinit files.  Note that the gdbinit file name may be
> >       overriden during file initialization, so get_init_files should be
> >       called after gdb_init.  */
> > -  std::string system_gdbinit, home_gdbinit, local_gdbinit;
> > +  std::vector<std::string> system_gdbinit;
> > +  std::string home_gdbinit, local_gdbinit;
> >    get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
> >
> >    /* Do these (and anything which might call wrap_here or *_filtered)
> > @@ -987,7 +1010,10 @@ captured_main_1 (struct captured_main_args *context)
> >       processed; it sets global parameters, which are independent of
> >       what file you are debugging or what directory you are in.  */
> >    if (!system_gdbinit.empty () && !inhibit_gdbinit)
> > -    ret = catch_command_errors (source_script, system_gdbinit.c_str (), 0);
> > +    {
> > +      for (const std::string& file : system_gdbinit)
> > +     ret = catch_command_errors (source_script, file.c_str (), 0);
> > +    }
> >
> >    /* Read and execute $HOME/.gdbinit file, if it exists.  This is done
> >       *before* all the command line arguments are processed; it sets
> > @@ -1205,7 +1231,7 @@ gdb_main (struct captured_main_args *args)
> >  static void
> >  print_gdb_help (struct ui_file *stream)
> >  {
> > -  std::string system_gdbinit;
> > +  std::vector<std::string> system_gdbinit;
> >    std::string home_gdbinit;
> >    std::string local_gdbinit;
> >
> > @@ -1286,9 +1312,18 @@ Other options:\n\n\
> >  At startup, GDB reads the following init files and executes their commands:\n\
> >  "), stream);
> >    if (!system_gdbinit.empty ())
> > -    fprintf_unfiltered (stream, _("\
> > -   * system-wide init file: %s\n\
> > -"), system_gdbinit.c_str ());
> > +    {
> > +      std::string output;
> > +      for (size_t idx = 0; idx < system_gdbinit.size(); ++idx)
>
> Space between variable name and parens.

Done, here and a couple lines below.

> > +        {
> > +       output += system_gdbinit[idx];
> > +       if (idx < system_gdbinit.size() - 1)
> > +         output += ", ";
> > +     }
> > +      fprintf_unfiltered (stream, _("\
> > +   * system-wide init files: %s\n\
> > +"), output.c_str ());
> > +    }
> >    if (!home_gdbinit.empty ())
> >      fprintf_unfiltered (stream, _("\
> >     * user-specific init file: %s\n\
> > --
> > 2.23.0.rc1.153.gdeed80330f-goog
>
> This looks good, but it needs a testcase, documentation updates and a
> NEWS entry.

Added NEWS and documentation, but how should I write a testcase for
it? It depends on a configure option... (we don't seem to have a test
for --with-system-gdbinit either, unless I missed it)

Will send a new patch shortly, also with a fixed configure script.

Christian

[PATCH 3/3 v2] Load system gdbinit files from a directory

[Christian Biesinger via gdb-patches]

[Adds NEWS entry, documentation, and fixes Sergio's review comments. This
should also be suitable to replace Fedora's custom gdbinit because it does
support loading .py files correctly, because it calls source_script (this
was also the case in the previous version.

I don't know how to write a test for this]

Adds a configure option --with-system-gdbinit-dir to specify a directory
in which to look for gdbinit files. All files in this directory are
loaded on startup (subject to -n/-nx as usual).

gdb/ChangeLog:

2019-08-20  Christian Biesinger  <cbiesinger@google.com>

	* NEWS: Mention new --with-system-gdbinit-dir option.
	* config.in: Regenerate.
	* configure: Regenerate.
	* configure.ac: Add new option --with-system-gdbinit-dir.
	* main.c (get_init_files): Change system_gdbinit argument to
	a vector and return the files in SYSTEM_GDBINIT_DIR in
	addition to SYSTEM_GDBINIT.
	(captured_main_1): Update.
	(print_gdb_help): Update.

gdb/doc/ChangeLog:

2019-08-25  Christian Biesinger  <cbiesinger@google.com>

	* gdb.texinfo (many sections): Document new --with-system-gdbinit-dir
	option.
---
 gdb/NEWS            |  5 ++++
 gdb/config.in       |  7 ++++++
 gdb/configure       | 51 +++++++++++++++++++++++++++++++++++++
 gdb/configure.ac    |  3 +++
 gdb/doc/gdb.texinfo | 61 +++++++++++++++++++++++++++++++++++++++------
 gdb/main.c          | 56 ++++++++++++++++++++++++++++++++++-------
 6 files changed, 166 insertions(+), 17 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 0a4e0f260f..fcc9c6f41f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -27,6 +27,11 @@
   provide the exitcode or exit status of the shell commands launched by
   GDB commands such as "shell", "pipe" and "make".
 
+* If configured with --with-system-gdbinit-dir, GDB will now also load
+  all files in that directory as system gdbinit files, unless the -nx
+  or -n flag is provided. These files can be written in any supported
+  scripting language as long as they have a matching file extension.
+
 * Python API
 
   ** The gdb.Value type has a new method 'format_string' which returns a
diff --git a/gdb/config.in b/gdb/config.in
index 26ca02f6a3..c0745c90a3 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -684,6 +684,13 @@
 /* automatically load a system-wide gdbinit file */
 #undef SYSTEM_GDBINIT
 
+/* automatically load system-wide gdbinit files from this directory */
+#undef SYSTEM_GDBINIT_DIR
+
+/* Define if the system-gdbinit-dir directory should be relocated when GDB is
+   moved. */
+#undef SYSTEM_GDBINIT_DIR_RELOCATABLE
+
 /* Define if the system-gdbinit directory should be relocated when GDB is
    moved. */
 #undef SYSTEM_GDBINIT_RELOCATABLE
diff --git a/gdb/configure b/gdb/configure
index 22a5f6051d..8436f06c9f 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -693,6 +693,7 @@ WIN32LIBS
 SER_HARDWIRE
 WERROR_CFLAGS
 WARN_CFLAGS
+SYSTEM_GDBINIT_DIR
 SYSTEM_GDBINIT
 TARGET_SYSTEM_ROOT
 CONFIG_LDFLAGS
@@ -884,6 +885,7 @@ with_libipt_prefix
 with_included_regex
 with_sysroot
 with_system_gdbinit
+with_system_gdbinit_dir
 enable_werror
 enable_build_warnings
 enable_gdb_build_warnings
@@ -1618,6 +1620,9 @@ Optional Packages:
   --with-sysroot[=DIR]    search for usr/lib et al within DIR
   --with-system-gdbinit=PATH
                           automatically load a system-wide gdbinit file
+  --with-system-gdbinit-dir=PATH
+                          automatically load system-wide gdbinit files from
+                          this directory
   --with-lzma             support lzma compression (auto/yes/no)
   --with-liblzma-prefix[=DIR]  search for liblzma in DIR/include and DIR/lib
   --without-liblzma-prefix     don't search for liblzma in includedir and libdir
@@ -15238,6 +15243,52 @@ _ACEOF
 
 
 
+# Check whether --with-system-gdbinit-dir was given.
+if test "${with_system_gdbinit_dir+set}" = set; then :
+  withval=$with_system_gdbinit_dir;
+    SYSTEM_GDBINIT_DIR=$withval
+else
+  SYSTEM_GDBINIT_DIR=
+fi
+
+
+  test "x$prefix" = xNONE && prefix="$ac_default_prefix"
+  test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
+  ac_define_dir=`eval echo $SYSTEM_GDBINIT_DIR`
+  ac_define_dir=`eval echo $ac_define_dir`
+
+cat >>confdefs.h <<_ACEOF
+#define SYSTEM_GDBINIT_DIR "$ac_define_dir"
+_ACEOF
+
+
+
+
+  if test "x$exec_prefix" = xNONE || test "x$exec_prefix" = 'x${prefix}'; then
+     if test "x$prefix" = xNONE; then
+     	test_prefix=/usr/local
+     else
+	test_prefix=$prefix
+     fi
+  else
+     test_prefix=$exec_prefix
+  fi
+  value=0
+  case ${ac_define_dir} in
+     "${test_prefix}"|"${test_prefix}/"*|\
+	'${exec_prefix}'|'${exec_prefix}/'*)
+     value=1
+     ;;
+  esac
+
+cat >>confdefs.h <<_ACEOF
+#define SYSTEM_GDBINIT_DIR_RELOCATABLE $value
+_ACEOF
+
+
+
+
+
 # Check whether --enable-werror was given.
 if test "${enable_werror+set}" = set; then :
   enableval=$enable_werror; case "${enableval}" in
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 9da8818fb5..f3f5bd0635 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1843,6 +1843,9 @@ GDB_AC_DEFINE_RELOCATABLE(TARGET_SYSTEM_ROOT, sysroot, ${ac_define_dir})
 GDB_AC_WITH_DIR(SYSTEM_GDBINIT, system-gdbinit,
     [automatically load a system-wide gdbinit file],
     [])
+GDB_AC_WITH_DIR(SYSTEM_GDBINIT_DIR, system-gdbinit-dir,
+    [automatically load system-wide gdbinit files from this directory],
+    [])
 
 AM_GDB_WARNINGS
 AM_GDB_UBSAN
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index bcf0420779..a43c4651c0 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1083,6 +1083,13 @@ Its location is specified with the @code{--with-system-gdbinit}
 configure option (@pxref{System-wide configuration}).
 It is loaded first when @value{GDBN} starts, before command line options
 have been processed.
+@item @file{system.gdbinit.d}
+This is the system-wide init directory.
+Its location is specified with the @code{--with-system-gdbinit-dir}
+configure option (@pxref{System-wide configuration}).
+Files in this directory are loaded immediately after system.gdbinit (if
+enabled) when @value{GDBN} starts, before command line options have been
+processed.
 @item @file{~/.gdbinit}
 This is the init file in your home directory.
 It is loaded next, after @file{system.gdbinit}, and before
@@ -1315,8 +1322,11 @@ Sets up the command interpreter as specified by the command line
 @cindex init file
 Reads the system-wide @dfn{init file} (if @option{--with-system-gdbinit} was
 used when building @value{GDBN}; @pxref{System-wide configuration,
- ,System-wide configuration and settings}) and executes all the commands in
-that file.
+ ,System-wide configuration and settings}) and the files in the system-wide
+gdbinit directory (if @option{--with-system-gdbinit-dir} was used) and executes
+all the commands in those files. If scripting languages are enabled, the files
+can be written in any of the supported languages as long as the extension matches
+the language.
 
 @anchor{Home Directory Init File}
 @item
@@ -37010,6 +37020,14 @@ directory under the configured prefix, and @value{GDBN} is moved to
 another location after being built, the location of the system-wide
 init file will be adjusted accordingly.
 
+@item --with-system-gdbinit-dir=@var{directory}
+Configure @value{GDBN} to automatically load init files from a
+system-wide directory.  @var{directory} should be an absolute path.
+If @var{directory} is in a directory under the configured prefix, and
+@value{GDBN} is moved to another location after being built, the
+location of the system-wide init directory will be adjusted
+accordingly.
+
 @item --enable-build-warnings
 When building the @value{GDBN} sources, ask the compiler to warn about
 any code which looks even vaguely suspicious.  It passes many
@@ -37035,24 +37053,27 @@ was first introduced in GCC 4.9.
 @section System-wide configuration and settings
 @cindex system-wide init file
 
-@value{GDBN} can be configured to have a system-wide init file;
-this file will be read and executed at startup (@pxref{Startup, , What
-@value{GDBN} does during startup}).
+@value{GDBN} can be configured to have a system-wide init file and a system-wide
+init file directory; this file and files in that directory will be read and
+executed at startup (@pxref{Startup, , What @value{GDBN} does during startup}).
 
-Here is the corresponding configure option:
+Here are the corresponding configure options:
 
 @table @code
 @item --with-system-gdbinit=@var{file}
 Specify that the default location of the system-wide init file is
 @var{file}.
+@item --with-system-gdbinit-dir=@var{directory}
+Specify that the default location of the system-wide init file directory
+is @var{directory}.
 @end table
 
 If @value{GDBN} has been configured with the option @option{--prefix=$prefix},
-it may be subject to relocation.  Two possible cases:
+they may be subject to relocation.  Two possible cases:
 
 @itemize @bullet
 @item 
-If the default location of this init file contains @file{$prefix},
+If the default location of this init file/directory contains @file{$prefix},
 it will be subject to relocation.  Suppose that the configure options
 are @option{--prefix=$prefix --with-system-gdbinit=$prefix/etc/gdbinit};
 if @value{GDBN} is moved from @file{$prefix} to @file{$install}, the system
@@ -37078,6 +37099,12 @@ initialization.  If the data-directory is changed after @value{GDBN} has
 started with the @code{set data-directory} command, the file will not be
 reread.
 
+This applies similarly to the system-wide directory specified in
+@option{--with-system-gdbinit-dir}.
+
+Any supported scripting language can be used for these init files, as long
+as the file extension matches the scripting language.
+
 @menu
 * System-wide Configuration Scripts::  Installed System-wide Configuration Scripts
 @end menu
@@ -45598,6 +45625,10 @@ Richard M. Stallman and Roland H. Pesch, July 1991.
 @value{SYSTEM_GDBINIT}
 @end ifset
 
+@ifset SYSTEM_GDBINIT
+@value{SYSTEM_GDBINIT_DIR}/*
+@end ifset
+
 ~/.gdbinit
 
 ./.gdbinit
@@ -45639,6 +45670,20 @@ See more in
 the @value{GDBN} manual in node @code{System-wide configuration}
 -- shell command @code{info -f gdb -n 'System-wide configuration'}.
 @end ifset
+@ifset SYSTEM_GDBINIT_DIR
+@item @value{SYSTEM_GDBINIT_DIR}
+@end ifset
+@ifclear SYSTEM_GDBINIT_DIR
+@item (not enabled with @code{--with-system-gdbinit-dir} during compilation)
+@end ifclear
+System-wide initialization directory.  All files in this directory are
+executed on startup unless user specified @value{GDBN} option @code{-nx} or
+@code{-n}.
+See more in
+@ifset man
+the @value{GDBN} manual in node @code{System-wide configuration}
+-- shell command @code{info -f gdb -n 'System-wide configuration'}.
+@end ifset
 @ifclear man
 @ref{System-wide configuration}.
 @end ifclear
diff --git a/gdb/main.c b/gdb/main.c
index 089a7628e5..19cd723ba8 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -45,6 +45,7 @@
 #include "event-top.h"
 #include "infrun.h"
 #include "gdbsupport/signals-state-save-restore.h"
+#include <algorithm>
 #include <vector>
 #include "gdbsupport/pathstuff.h"
 #include "cli/cli-style.h"
@@ -234,11 +235,11 @@ relocate_gdbinit_path_maybe_in_datadir (const std::string& file)
    to be loaded, then SYSTEM_GDBINIT (resp. HOME_GDBINIT and
    LOCAL_GDBINIT) is set to the empty string.  */
 static void
-get_init_files (std::string *system_gdbinit,
+get_init_files (std::vector<std::string> *system_gdbinit,
 		std::string *home_gdbinit,
 		std::string *local_gdbinit)
 {
-  static std::string sysgdbinit;
+  static std::vector<std::string> sysgdbinit;
   static std::string homeinit;
   static std::string localinit;
   static int initialized = 0;
@@ -253,7 +254,32 @@ get_init_files (std::string *system_gdbinit,
 	    relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT);
 	  if (!relocated_sysgdbinit.empty () &&
 	      stat (relocated_sysgdbinit.c_str (), &s) == 0)
-	    sysgdbinit = relocated_sysgdbinit;
+	    sysgdbinit.push_back(relocated_sysgdbinit);
+	}
+      if (SYSTEM_GDBINIT_DIR[0])
+        {
+	  std::string relocated_gdbinit_dir =
+	    relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT_DIR);
+	  DIR* dir = nullptr;
+	  if (!relocated_gdbinit_dir.empty ())
+	    dir = opendir (relocated_gdbinit_dir.c_str ());
+	  if (dir != nullptr) 
+	    {
+	      std::vector<std::string> files;
+	      struct dirent* ent;
+	      for (;;)
+	        {
+		  ent = readdir (dir);
+		  if (ent == nullptr)
+		    break;
+		  if (ent->d_type == DT_REG && ent->d_name[0] != '.')
+		    files.push_back (relocated_gdbinit_dir + SLASH_STRING +
+				     ent->d_name);
+		}
+	      closedir (dir);
+	      std::sort (files.begin (), files.end ());
+	      sysgdbinit.insert (sysgdbinit.end (), files.begin (), files.end ());
+	    }
 	}
 
       const char *homedir = getenv ("HOME");
@@ -911,7 +937,7 @@ captured_main_1 (struct captured_main_args *context)
   /* Lookup gdbinit files.  Note that the gdbinit file name may be
      overriden during file initialization, so get_init_files should be
      called after gdb_init.  */
-  std::string system_gdbinit;
+  std::vector<std::string> system_gdbinit;
   std::string home_gdbinit;
   std::string local_gdbinit;
   get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
@@ -991,7 +1017,10 @@ captured_main_1 (struct captured_main_args *context)
      processed; it sets global parameters, which are independent of
      what file you are debugging or what directory you are in.  */
   if (!system_gdbinit.empty () && !inhibit_gdbinit)
-    ret = catch_command_errors (source_script, system_gdbinit.c_str (), 0);
+    {
+      for (const std::string& file : system_gdbinit)
+	ret = catch_command_errors (source_script, file.c_str (), 0);
+    }
 
   /* Read and execute $HOME/.gdbinit file, if it exists.  This is done
      *before* all the command line arguments are processed; it sets
@@ -1209,7 +1238,7 @@ gdb_main (struct captured_main_args *args)
 static void
 print_gdb_help (struct ui_file *stream)
 {
-  std::string system_gdbinit;
+  std::vector<std::string> system_gdbinit;
   std::string home_gdbinit;
   std::string local_gdbinit;
 
@@ -1290,9 +1319,18 @@ Other options:\n\n\
 At startup, GDB reads the following init files and executes their commands:\n\
 "), stream);
   if (!system_gdbinit.empty ())
-    fprintf_unfiltered (stream, _("\
-   * system-wide init file: %s\n\
-"), system_gdbinit.c_str ());
+    {
+      std::string output;
+      for (size_t idx = 0; idx < system_gdbinit.size (); ++idx)
+        {
+	  output += system_gdbinit[idx];
+	  if (idx < system_gdbinit.size () - 1)
+	    output += ", ";
+	}
+      fprintf_unfiltered (stream, _("\
+   * system-wide init files: %s\n\
+"), output.c_str ());
+    }
   if (!home_gdbinit.empty ())
     fprintf_unfiltered (stream, _("\
    * user-specific init file: %s\n\
-- 
2.23.0.187.g17f5b7556c-goog

Re: [PATCH 3/3 v2] Load system gdbinit files from a directory

[Eli Zaretskii]

> Date: Sun, 25 Aug 2019 19:33:10 -0500
> From: "Christian Biesinger via gdb-patches" <gdb-patches@sourceware.org>
> Cc: Christian Biesinger <cbiesinger@google.com>
> 
> Adds a configure option --with-system-gdbinit-dir to specify a directory
> in which to look for gdbinit files. All files in this directory are
> loaded on startup (subject to -n/-nx as usual).

I think this option's value should be displayed by "show
configuration" and by "gdb --config".

> +* If configured with --with-system-gdbinit-dir, GDB will now also load

I think "also" here might be confusing, because it lacks context.  I
think we should say explicitly "in addition to the system-wide gdbinit
file" instead.

> +  all files in that directory as system gdbinit files, unless the -nx
> +  or -n flag is provided. These files can be written in any supported

Two spaces between sentences (here and elsewhere throughout the
patch).

> +@item @file{system.gdbinit.d}
> +This is the system-wide init directory.
> +Its location is specified with the @code{--with-system-gdbinit-dir}
> +configure option (@pxref{System-wide configuration}).
> +Files in this directory are loaded immediately after system.gdbinit (if
> +enabled) when @value{GDBN} starts, before command line options have been
> +processed.

I'm not sure I understand: _all_ files in that directory will be
loaded, regardless of how they are named?  If so, I think we should
say that explicitly.  We should probably also say that the order the
files are loaded is arbitrary.  Also, we should say something about
that directory including subdirectories, because I think the reader
might wonder about that.

>  @cindex init file
>  Reads the system-wide @dfn{init file} (if @option{--with-system-gdbinit} was
>  used when building @value{GDBN}; @pxref{System-wide configuration,
> - ,System-wide configuration and settings}) and executes all the commands in
> -that file.
> + ,System-wide configuration and settings}) and the files in the system-wide
> +gdbinit directory (if @option{--with-system-gdbinit-dir} was used) and executes
> +all the commands in those files. If scripting languages are enabled, the files
> +can be written in any of the supported languages as long as the extension matches
> +the language.

Is the order of reading as described, i.e. the file first, then the
files in the directory?

Btw, how does this option interact with auto-load safe-path?  Would
GDB refuse to load init files from this directory if it considers them
"unsafe"?

> +@item --with-system-gdbinit-dir=@var{directory}
> +Configure @value{GDBN} to automatically load init files from a
> +system-wide directory.  @var{directory} should be an absolute path.
                                                        ^^^^^^^^^^^^^
"absolute file name".  The Gnu Coding Standards frown on using "path"
for anything that is not PATH-style lists of directories.

> +@ifset SYSTEM_GDBINIT
> +@value{SYSTEM_GDBINIT_DIR}/*
> +@end ifset

@ifset SYSTEM_GDBINIT or @ifset SYSTEM_GDBINIT_DIR?

Thanks.

Re: [PATCH 3/3 v2] Load system gdbinit files from a directory

[Christian Biesinger via gdb-patches]

Thanks for the review!

On Mon, Aug 26, 2019 at 2:22 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > Date: Sun, 25 Aug 2019 19:33:10 -0500
> > From: "Christian Biesinger via gdb-patches" <gdb-patches@sourceware.org>
> > Cc: Christian Biesinger <cbiesinger@google.com>
> >
> > Adds a configure option --with-system-gdbinit-dir to specify a directory
> > in which to look for gdbinit files. All files in this directory are
> > loaded on startup (subject to -n/-nx as usual).
>
> I think this option's value should be displayed by "show
> configuration" and by "gdb --config".

Done.

> > +* If configured with --with-system-gdbinit-dir, GDB will now also load
>
> I think "also" here might be confusing, because it lacks context.  I
> think we should say explicitly "in addition to the system-wide gdbinit
> file" instead.

Done.

> > +  all files in that directory as system gdbinit files, unless the -nx
> > +  or -n flag is provided. These files can be written in any supported
>
> Two spaces between sentences (here and elsewhere throughout the
> patch).

I think I fixed all occurrences. (searching gdb.texinfo for "\. [^ ]"
found a lot of existing places that do it wrong :( )

> > +@item @file{system.gdbinit.d}
> > +This is the system-wide init directory.
> > +Its location is specified with the @code{--with-system-gdbinit-dir}
> > +configure option (@pxref{System-wide configuration}).
> > +Files in this directory are loaded immediately after system.gdbinit (if
> > +enabled) when @value{GDBN} starts, before command line options have been
> > +processed.
>
> I'm not sure I understand: _all_ files in that directory will be
> loaded, regardless of how they are named?  If so, I think we should
> say that explicitly.

I changed it now so that only .gdb/.py/.scm is loaded, and documented that.

> We should probably also say that the order the
> files are loaded is arbitrary.

They're actually alphabetically sorted; added a note in the
documentation about that.

>  Also, we should say something about
> that directory including subdirectories, because I think the reader
> might wonder about that.

Done.

> >  @cindex init file
> >  Reads the system-wide @dfn{init file} (if @option{--with-system-gdbinit} was
> >  used when building @value{GDBN}; @pxref{System-wide configuration,
> > - ,System-wide configuration and settings}) and executes all the commands in
> > -that file.
> > + ,System-wide configuration and settings}) and the files in the system-wide
> > +gdbinit directory (if @option{--with-system-gdbinit-dir} was used) and executes
> > +all the commands in those files. If scripting languages are enabled, the files
> > +can be written in any of the supported languages as long as the extension matches
> > +the language.
>
> Is the order of reading as described, i.e. the file first, then the
> files in the directory?

Yes.

> Btw, how does this option interact with auto-load safe-path?  Would
> GDB refuse to load init files from this directory if it considers them
> "unsafe"?

No, that is not consulted for these files (if it were, the system
gdbinit file or ~/.gdbinit would not work in the common case, because
the auto-load path is $debugdir:$datadir/auto-load)

> > +@item --with-system-gdbinit-dir=@var{directory}
> > +Configure @value{GDBN} to automatically load init files from a
> > +system-wide directory.  @var{directory} should be an absolute path.
>                                                         ^^^^^^^^^^^^^
> "absolute file name".  The Gnu Coding Standards frown on using "path"
> for anything that is not PATH-style lists of directories.

Changed to "absolute directory name"; I didn't want to use "file name"
for a directory.

(looks like a number of other occurrences in gdb.texinfo should be
changed too...)

> > +@ifset SYSTEM_GDBINIT
> > +@value{SYSTEM_GDBINIT_DIR}/*
> > +@end ifset
>
> @ifset SYSTEM_GDBINIT or @ifset SYSTEM_GDBINIT_DIR?

Thanks, fixed.

Will send a new patch in a second.

Christian

[PATCH 3/3 v3] Load system gdbinit files from a directory

[Christian Biesinger via gdb-patches]

[Looks like I never did send this updated patch :( Please take another
look!]

Adds a configure option --with-system-gdbinit-dir to specify a directory
in which to look for gdbinit files.  All files in this directory are
loaded on startup (subject to -n/-nx as usual) as long as the extension
matches a known and enabled scripting language (.gdb/.py/.scm).

This also changes get_ext_lang_of_file to support ".gdb" files, similar
to get_ext_lang_defn's handling of EXT_LANG_GDB.

gdb/ChangeLog:

2019-08-20  Christian Biesinger  <cbiesinger@google.com>

	* NEWS: Mention new --with-system-gdbinit-dir option.
	* config.in: Regenerate.
	* configure: Regenerate.
	* configure.ac: Add new option --with-system-gdbinit-dir.
	* extension.c (get_ext_lang_of_file): Return extension_language_gdb
	for a ".gdb" suffix.
	* main.c (get_init_files): Change system_gdbinit argument to
	a vector and return the files in SYSTEM_GDBINIT_DIR in
	addition to SYSTEM_GDBINIT.
	(captured_main_1): Update.
	(print_gdb_help): Update.
	* top.c (print_gdb_configuration): Also print the value of
	SYSTEM_GDBINIT_DIR.

gdb/doc/ChangeLog:

2019-08-25  Christian Biesinger  <cbiesinger@google.com>

	* gdb.texinfo (many sections): Document new --with-system-gdbinit-dir
	option.
---
 gdb/NEWS            |  6 ++++
 gdb/config.in       |  7 ++++
 gdb/configure       | 51 +++++++++++++++++++++++++++++
 gdb/configure.ac    |  3 ++
 gdb/doc/gdb.texinfo | 67 ++++++++++++++++++++++++++++++++-----
 gdb/extension.c     |  3 ++
 gdb/main.c          | 80 +++++++++++++++++++++++++++++++++++++--------
 gdb/top.c           |  4 +++
 8 files changed, 199 insertions(+), 22 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 779fd91d3a..1d0f56a119 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -34,6 +34,12 @@
 
 * GDB can now be compiled with Python 3 on Windows.
 
+* In addition to the system-wide gdbinit file, if configured with
+ --with-system-gdbinit-dir, GDB will now also load files in that directory
+ as system gdbinit files, unless the -nx or -n flag is provided.  Files
+ with extensions .gdb, .py and .scm are supported as long as GDB was
+ compiled with support for that language.
+
 * Python API
 
   ** The gdb.Value type has a new method 'format_string' which returns a
diff --git a/gdb/config.in b/gdb/config.in
index 26ca02f6a3..c0745c90a3 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -684,6 +684,13 @@
 /* automatically load a system-wide gdbinit file */
 #undef SYSTEM_GDBINIT
 
+/* automatically load system-wide gdbinit files from this directory */
+#undef SYSTEM_GDBINIT_DIR
+
+/* Define if the system-gdbinit-dir directory should be relocated when GDB is
+   moved. */
+#undef SYSTEM_GDBINIT_DIR_RELOCATABLE
+
 /* Define if the system-gdbinit directory should be relocated when GDB is
    moved. */
 #undef SYSTEM_GDBINIT_RELOCATABLE
diff --git a/gdb/configure b/gdb/configure
index 22a5f6051d..8436f06c9f 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -693,6 +693,7 @@ WIN32LIBS
 SER_HARDWIRE
 WERROR_CFLAGS
 WARN_CFLAGS
+SYSTEM_GDBINIT_DIR
 SYSTEM_GDBINIT
 TARGET_SYSTEM_ROOT
 CONFIG_LDFLAGS
@@ -884,6 +885,7 @@ with_libipt_prefix
 with_included_regex
 with_sysroot
 with_system_gdbinit
+with_system_gdbinit_dir
 enable_werror
 enable_build_warnings
 enable_gdb_build_warnings
@@ -1618,6 +1620,9 @@ Optional Packages:
   --with-sysroot[=DIR]    search for usr/lib et al within DIR
   --with-system-gdbinit=PATH
                           automatically load a system-wide gdbinit file
+  --with-system-gdbinit-dir=PATH
+                          automatically load system-wide gdbinit files from
+                          this directory
   --with-lzma             support lzma compression (auto/yes/no)
   --with-liblzma-prefix[=DIR]  search for liblzma in DIR/include and DIR/lib
   --without-liblzma-prefix     don't search for liblzma in includedir and libdir
@@ -15238,6 +15243,52 @@ _ACEOF
 
 
 
+# Check whether --with-system-gdbinit-dir was given.
+if test "${with_system_gdbinit_dir+set}" = set; then :
+  withval=$with_system_gdbinit_dir;
+    SYSTEM_GDBINIT_DIR=$withval
+else
+  SYSTEM_GDBINIT_DIR=
+fi
+
+
+  test "x$prefix" = xNONE && prefix="$ac_default_prefix"
+  test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
+  ac_define_dir=`eval echo $SYSTEM_GDBINIT_DIR`
+  ac_define_dir=`eval echo $ac_define_dir`
+
+cat >>confdefs.h <<_ACEOF
+#define SYSTEM_GDBINIT_DIR "$ac_define_dir"
+_ACEOF
+
+
+
+
+  if test "x$exec_prefix" = xNONE || test "x$exec_prefix" = 'x${prefix}'; then
+     if test "x$prefix" = xNONE; then
+     	test_prefix=/usr/local
+     else
+	test_prefix=$prefix
+     fi
+  else
+     test_prefix=$exec_prefix
+  fi
+  value=0
+  case ${ac_define_dir} in
+     "${test_prefix}"|"${test_prefix}/"*|\
+	'${exec_prefix}'|'${exec_prefix}/'*)
+     value=1
+     ;;
+  esac
+
+cat >>confdefs.h <<_ACEOF
+#define SYSTEM_GDBINIT_DIR_RELOCATABLE $value
+_ACEOF
+
+
+
+
+
 # Check whether --enable-werror was given.
 if test "${enable_werror+set}" = set; then :
   enableval=$enable_werror; case "${enableval}" in
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 9da8818fb5..f3f5bd0635 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1843,6 +1843,9 @@ GDB_AC_DEFINE_RELOCATABLE(TARGET_SYSTEM_ROOT, sysroot, ${ac_define_dir})
 GDB_AC_WITH_DIR(SYSTEM_GDBINIT, system-gdbinit,
     [automatically load a system-wide gdbinit file],
     [])
+GDB_AC_WITH_DIR(SYSTEM_GDBINIT_DIR, system-gdbinit-dir,
+    [automatically load system-wide gdbinit files from this directory],
+    [])
 
 AM_GDB_WARNINGS
 AM_GDB_UBSAN
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f2713c0396..eb385ae1ee 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1083,6 +1083,16 @@ Its location is specified with the @code{--with-system-gdbinit}
 configure option (@pxref{System-wide configuration}).
 It is loaded first when @value{GDBN} starts, before command line options
 have been processed.
+@item @file{system.gdbinit.d}
+This is the system-wide init directory.
+Its location is specified with the @code{--with-system-gdbinit-dir}
+configure option (@pxref{System-wide configuration}).
+Files in this directory are loaded in alphabetical order immediately after
+system.gdbinit (if enabled) when @value{GDBN} starts, before command line
+options have been processed.  Files need to have a recognized scripting
+language extension (@code{.py}/@code{.scm}) or be named with a @code{.gdb}
+extension to be interpreted as regular @value{GDBN} commands.  @value{GDBN}
+will not recurse into any subdirectories of this directory.
 @item @file{~/.gdbinit}
 This is the init file in your home directory.
 It is loaded next, after @file{system.gdbinit}, and before
@@ -1315,8 +1325,11 @@ Sets up the command interpreter as specified by the command line
 @cindex init file
 Reads the system-wide @dfn{init file} (if @option{--with-system-gdbinit} was
 used when building @value{GDBN}; @pxref{System-wide configuration,
- ,System-wide configuration and settings}) and executes all the commands in
-that file.
+ ,System-wide configuration and settings}) and the files in the system-wide
+gdbinit directory (if @option{--with-system-gdbinit-dir} was used) and executes
+all the commands in those files.  The files need to be named with a @code{.gdb}
+extension to be interpreted as @value{GDBN} commands, or they can be written
+in a supported scripting language with an appropriate file extension.
 
 @anchor{Home Directory Init File}
 @item
@@ -37034,6 +37047,14 @@ directory under the configured prefix, and @value{GDBN} is moved to
 another location after being built, the location of the system-wide
 init file will be adjusted accordingly.
 
+@item --with-system-gdbinit-dir=@var{directory}
+Configure @value{GDBN} to automatically load init files from a
+system-wide directory.  @var{directory} should be an absolute directory
+name.  If @var{directory} is in a directory under the configured
+prefix, and @value{GDBN} is moved to another location after being
+built, the location of the system-wide init directory will be
+adjusted accordingly.
+
 @item --enable-build-warnings
 When building the @value{GDBN} sources, ask the compiler to warn about
 any code which looks even vaguely suspicious.  It passes many
@@ -37059,24 +37080,28 @@ was first introduced in GCC 4.9.
 @section System-wide configuration and settings
 @cindex system-wide init file
 
-@value{GDBN} can be configured to have a system-wide init file;
-this file will be read and executed at startup (@pxref{Startup, , What
-@value{GDBN} does during startup}).
+@value{GDBN} can be configured to have a system-wide init file and a
+system-wide init file directory; this file and files in that directory
+(if they have a recognized file extension) will be read and executed at
+startup (@pxref{Startup, , What @value{GDBN} does during startup}).
 
-Here is the corresponding configure option:
+Here are the corresponding configure options:
 
 @table @code
 @item --with-system-gdbinit=@var{file}
 Specify that the default location of the system-wide init file is
 @var{file}.
+@item --with-system-gdbinit-dir=@var{directory}
+Specify that the default location of the system-wide init file directory
+is @var{directory}.
 @end table
 
 If @value{GDBN} has been configured with the option @option{--prefix=$prefix},
-it may be subject to relocation.  Two possible cases:
+they may be subject to relocation.  Two possible cases:
 
 @itemize @bullet
 @item 
-If the default location of this init file contains @file{$prefix},
+If the default location of this init file/directory contains @file{$prefix},
 it will be subject to relocation.  Suppose that the configure options
 are @option{--prefix=$prefix --with-system-gdbinit=$prefix/etc/gdbinit};
 if @value{GDBN} is moved from @file{$prefix} to @file{$install}, the system
@@ -37102,6 +37127,14 @@ initialization.  If the data-directory is changed after @value{GDBN} has
 started with the @code{set data-directory} command, the file will not be
 reread.
 
+This applies similarly to the system-wide directory specified in
+@option{--with-system-gdbinit-dir}.
+
+Any supported scripting language can be used for these init files, as long
+as the file extension matches the scripting language.  To be interpreted
+as regular @value{GDBN} commands, the files needs to have a @code{.gdb}
+extension.
+
 @menu
 * System-wide Configuration Scripts::  Installed System-wide Configuration Scripts
 @end menu
@@ -45591,6 +45624,10 @@ Richard M. Stallman and Roland H. Pesch, July 1991.
 @value{SYSTEM_GDBINIT}
 @end ifset
 
+@ifset SYSTEM_GDBINIT_DIR
+@value{SYSTEM_GDBINIT_DIR}/*
+@end ifset
+
 ~/.gdbinit
 
 ./.gdbinit
@@ -45632,6 +45669,20 @@ See more in
 the @value{GDBN} manual in node @code{System-wide configuration}
 -- shell command @code{info -f gdb -n 'System-wide configuration'}.
 @end ifset
+@ifset SYSTEM_GDBINIT_DIR
+@item @value{SYSTEM_GDBINIT_DIR}
+@end ifset
+@ifclear SYSTEM_GDBINIT_DIR
+@item (not enabled with @code{--with-system-gdbinit-dir} during compilation)
+@end ifclear
+System-wide initialization directory.  All files in this directory are
+executed on startup unless user specified @value{GDBN} option @code{-nx} or
+@code{-n}, as long as they have a recognized file extension.
+See more in
+@ifset man
+the @value{GDBN} manual in node @code{System-wide configuration}
+-- shell command @code{info -f gdb -n 'System-wide configuration'}.
+@end ifset
 @ifclear man
 @ref{System-wide configuration}.
 @end ifclear
diff --git a/gdb/extension.c b/gdb/extension.c
index 8637bc53f2..1fb4b48003 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -154,6 +154,9 @@ get_ext_lang_of_file (const char *file)
   int i;
   const struct extension_language_defn *extlang;
 
+  if (has_extension (file, extension_language_gdb.suffix))
+    return &extension_language_gdb;
+
   ALL_EXTENSION_LANGUAGES (i, extlang)
     {
       if (has_extension (file, extlang->suffix))
diff --git a/gdb/main.c b/gdb/main.c
index 7fab8ff8da..ed902371cb 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -45,6 +45,7 @@
 #include "event-top.h"
 #include "infrun.h"
 #include "gdbsupport/signals-state-save-restore.h"
+#include <algorithm>
 #include <vector>
 #include "gdbsupport/pathstuff.h"
 #include "cli/cli-style.h"
@@ -195,7 +196,8 @@ relocate_gdb_directory (const char *initial, bool relocatable)
    otherwise.  */
 
 static std::string
-relocate_gdbinit_path_maybe_in_datadir (const std::string& file)
+relocate_gdbinit_path_maybe_in_datadir (const std::string& file,
+					bool relocatable)
 {
   size_t datadir_len = strlen (GDB_DATADIR);
 
@@ -219,9 +221,8 @@ relocate_gdbinit_path_maybe_in_datadir (const std::string& file)
     }
   else
     {
-      relocated_path = relocate_path (gdb_program_name,
-				      file.c_str (),
-				      SYSTEM_GDBINIT_RELOCATABLE);
+      relocated_path = relocate_path (gdb_program_name, file.c_str (),
+				      relocatable);
     }
     return relocated_path;
 }
@@ -232,11 +233,11 @@ relocate_gdbinit_path_maybe_in_datadir (const std::string& file)
    to be loaded, then SYSTEM_GDBINIT (resp. HOME_GDBINIT and
    LOCAL_GDBINIT) is set to the empty string.  */
 static void
-get_init_files (std::string *system_gdbinit,
+get_init_files (std::vector<std::string> *system_gdbinit,
 		std::string *home_gdbinit,
 		std::string *local_gdbinit)
 {
-  static std::string sysgdbinit;
+  static std::vector<std::string> sysgdbinit;
   static std::string homeinit;
   static std::string localinit;
   static int initialized = 0;
@@ -248,10 +249,49 @@ get_init_files (std::string *system_gdbinit,
       if (SYSTEM_GDBINIT[0])
 	{
 	  std::string relocated_sysgdbinit
-	    = relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT);
+	    = relocate_gdbinit_path_maybe_in_datadir
+		(SYSTEM_GDBINIT, SYSTEM_GDBINIT_RELOCATABLE);
 	  if (!relocated_sysgdbinit.empty ()
 	      && stat (relocated_sysgdbinit.c_str (), &s) == 0)
-	    sysgdbinit = relocated_sysgdbinit;
+	    sysgdbinit.push_back (relocated_sysgdbinit);
+	}
+      if (SYSTEM_GDBINIT_DIR[0])
+	{
+	  std::string relocated_gdbinit_dir
+	    = relocate_gdbinit_path_maybe_in_datadir
+		(SYSTEM_GDBINIT_DIR, SYSTEM_GDBINIT_DIR_RELOCATABLE);
+	  DIR *dir = nullptr;
+	  if (!relocated_gdbinit_dir.empty ())
+	    dir = opendir (relocated_gdbinit_dir.c_str ());
+	  if (dir != nullptr)
+	    {
+	      std::vector<std::string> files;
+	      struct dirent *ent;
+	      for (;;)
+		{
+		  ent = readdir (dir);
+		  if (ent == nullptr)
+		    break;
+		  if (ent->d_name[0] == '.')
+		    continue;
+		  /* ent->d_type is not available on all systems (e.g. mingw,
+		     Solaris), so we have to call stat().  */
+		  std::string filename
+		    = relocated_gdbinit_dir + SLASH_STRING + ent->d_name;
+		  if (stat(filename.c_str (), &s) != 0 || !S_ISREG (s.st_mode))
+		    continue;
+		  const struct extension_language_defn *extlang
+		    = get_ext_lang_of_file (filename.c_str ());
+		  /* We effectively don't support "set script-extension off/soft",
+		     because we are loading system init files here, so it
+		     does not really make sense to depend on a setting.  */
+		  if (extlang != nullptr && ext_lang_present_p (extlang))
+		    files.push_back (filename);
+		}
+	      closedir (dir);
+	      std::sort (files.begin (), files.end ());
+	      sysgdbinit.insert (sysgdbinit.end (), files.begin (), files.end ());
+	    }
 	}
 
       const char *homedir = getenv ("HOME");
@@ -916,7 +956,7 @@ captured_main_1 (struct captured_main_args *context)
   /* Lookup gdbinit files.  Note that the gdbinit file name may be
      overriden during file initialization, so get_init_files should be
      called after gdb_init.  */
-  std::string system_gdbinit;
+  std::vector<std::string> system_gdbinit;
   std::string home_gdbinit;
   std::string local_gdbinit;
   get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
@@ -996,7 +1036,10 @@ captured_main_1 (struct captured_main_args *context)
      processed; it sets global parameters, which are independent of
      what file you are debugging or what directory you are in.  */
   if (!system_gdbinit.empty () && !inhibit_gdbinit)
-    ret = catch_command_errors (source_script, system_gdbinit.c_str (), 0);
+    {
+      for (const std::string& file : system_gdbinit)
+	ret = catch_command_errors (source_script, file.c_str (), 0);
+    }
 
   /* Read and execute $HOME/.gdbinit file, if it exists.  This is done
      *before* all the command line arguments are processed; it sets
@@ -1214,7 +1257,7 @@ gdb_main (struct captured_main_args *args)
 static void
 print_gdb_help (struct ui_file *stream)
 {
-  std::string system_gdbinit;
+  std::vector<std::string> system_gdbinit;
   std::string home_gdbinit;
   std::string local_gdbinit;
 
@@ -1295,9 +1338,18 @@ Other options:\n\n\
 At startup, GDB reads the following init files and executes their commands:\n\
 "), stream);
   if (!system_gdbinit.empty ())
-    fprintf_unfiltered (stream, _("\
-   * system-wide init file: %s\n\
-"), system_gdbinit.c_str ());
+    {
+      std::string output;
+      for (size_t idx = 0; idx < system_gdbinit.size (); ++idx)
+        {
+	  output += system_gdbinit[idx];
+	  if (idx < system_gdbinit.size () - 1)
+	    output += ", ";
+	}
+      fprintf_unfiltered (stream, _("\
+   * system-wide init files: %s\n\
+"), output.c_str ());
+    }
   if (!home_gdbinit.empty ())
     fprintf_unfiltered (stream, _("\
    * user-specific init file: %s\n\
diff --git a/gdb/top.c b/gdb/top.c
index 49e6daae94..7a45f1b4aa 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1515,6 +1515,10 @@ This GDB was configured as follows:\n\
     fprintf_filtered (stream, _("\
              --with-system-gdbinit=%s%s\n\
 "), SYSTEM_GDBINIT, SYSTEM_GDBINIT_RELOCATABLE ? " (relocatable)" : "");
+  if (SYSTEM_GDBINIT_DIR[0])
+    fprintf_filtered (stream, _("\
+             --with-system-gdbinit-dir=%s%s\n\
+"), SYSTEM_GDBINIT_DIR, SYSTEM_GDBINIT_DIR_RELOCATABLE ? " (relocatable)" : "");
     /* We assume "relocatable" will be printed at least once, thus we always
        print this text.  It's a reasonably safe assumption for now.  */
     fprintf_filtered (stream, _("\n\
-- 
2.23.0.351.gc4317032e6-goog

Re: [PATCH 3/3 v3] Load system gdbinit files from a directory

[Christian Biesinger via gdb-patches]

Ping.

(Eli -- please re-review the documentation parts of this patch, I updated
them per your comments)

Christian

On Tue, Sep 24, 2019 at 11:29 AM Christian Biesinger <cbiesinger@google.com>
wrote:

> [Looks like I never did send this updated patch :( Please take another
> look!]
>
> Adds a configure option --with-system-gdbinit-dir to specify a directory
> in which to look for gdbinit files.  All files in this directory are
> loaded on startup (subject to -n/-nx as usual) as long as the extension
> matches a known and enabled scripting language (.gdb/.py/.scm).
>
> This also changes get_ext_lang_of_file to support ".gdb" files, similar
> to get_ext_lang_defn's handling of EXT_LANG_GDB.
>
> gdb/ChangeLog:
>
> 2019-08-20  Christian Biesinger  <cbiesinger@google.com>
>
>         * NEWS: Mention new --with-system-gdbinit-dir option.
>         * config.in: Regenerate.
>         * configure: Regenerate.
>         * configure.ac: Add new option --with-system-gdbinit-dir.
>         * extension.c (get_ext_lang_of_file): Return extension_language_gdb
>         for a ".gdb" suffix.
>         * main.c (get_init_files): Change system_gdbinit argument to
>         a vector and return the files in SYSTEM_GDBINIT_DIR in
>         addition to SYSTEM_GDBINIT.
>         (captured_main_1): Update.
>         (print_gdb_help): Update.
>         * top.c (print_gdb_configuration): Also print the value of
>         SYSTEM_GDBINIT_DIR.
>
> gdb/doc/ChangeLog:
>
> 2019-08-25  Christian Biesinger  <cbiesinger@google.com>
>
>         * gdb.texinfo (many sections): Document new
> --with-system-gdbinit-dir
>         option.
> ---
>  gdb/NEWS            |  6 ++++
>  gdb/config.in       |  7 ++++
>  gdb/configure       | 51 +++++++++++++++++++++++++++++
>  gdb/configure.ac    |  3 ++
>  gdb/doc/gdb.texinfo | 67 ++++++++++++++++++++++++++++++++-----
>  gdb/extension.c     |  3 ++
>  gdb/main.c          | 80 +++++++++++++++++++++++++++++++++++++--------
>  gdb/top.c           |  4 +++
>  8 files changed, 199 insertions(+), 22 deletions(-)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 779fd91d3a..1d0f56a119 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -34,6 +34,12 @@
>
>  * GDB can now be compiled with Python 3 on Windows.
>
> +* In addition to the system-wide gdbinit file, if configured with
> + --with-system-gdbinit-dir, GDB will now also load files in that directory
> + as system gdbinit files, unless the -nx or -n flag is provided.  Files
> + with extensions .gdb, .py and .scm are supported as long as GDB was
> + compiled with support for that language.
> +
>  * Python API
>
>    ** The gdb.Value type has a new method 'format_string' which returns a
> diff --git a/gdb/config.in b/gdb/config.in
> index 26ca02f6a3..c0745c90a3 100644
> --- a/gdb/config.in
> +++ b/gdb/config.in
> @@ -684,6 +684,13 @@
>  /* automatically load a system-wide gdbinit file */
>  #undef SYSTEM_GDBINIT
>
> +/* automatically load system-wide gdbinit files from this directory */
> +#undef SYSTEM_GDBINIT_DIR
> +
> +/* Define if the system-gdbinit-dir directory should be relocated when
> GDB is
> +   moved. */
> +#undef SYSTEM_GDBINIT_DIR_RELOCATABLE
> +
>  /* Define if the system-gdbinit directory should be relocated when GDB is
>     moved. */
>  #undef SYSTEM_GDBINIT_RELOCATABLE
> diff --git a/gdb/configure b/gdb/configure
> index 22a5f6051d..8436f06c9f 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -693,6 +693,7 @@ WIN32LIBS
>  SER_HARDWIRE
>  WERROR_CFLAGS
>  WARN_CFLAGS
> +SYSTEM_GDBINIT_DIR
>  SYSTEM_GDBINIT
>  TARGET_SYSTEM_ROOT
>  CONFIG_LDFLAGS
> @@ -884,6 +885,7 @@ with_libipt_prefix
>  with_included_regex
>  with_sysroot
>  with_system_gdbinit
> +with_system_gdbinit_dir
>  enable_werror
>  enable_build_warnings
>  enable_gdb_build_warnings
> @@ -1618,6 +1620,9 @@ Optional Packages:
>    --with-sysroot[=DIR]    search for usr/lib et al within DIR
>    --with-system-gdbinit=PATH
>                            automatically load a system-wide gdbinit file
> +  --with-system-gdbinit-dir=PATH
> +                          automatically load system-wide gdbinit files
> from
> +                          this directory
>    --with-lzma             support lzma compression (auto/yes/no)
>    --with-liblzma-prefix[=DIR]  search for liblzma in DIR/include and
> DIR/lib
>    --without-liblzma-prefix     don't search for liblzma in includedir and
> libdir
> @@ -15238,6 +15243,52 @@ _ACEOF
>
>
>
> +# Check whether --with-system-gdbinit-dir was given.
> +if test "${with_system_gdbinit_dir+set}" = set; then :
> +  withval=$with_system_gdbinit_dir;
> +    SYSTEM_GDBINIT_DIR=$withval
> +else
> +  SYSTEM_GDBINIT_DIR=
> +fi
> +
> +
> +  test "x$prefix" = xNONE && prefix="$ac_default_prefix"
> +  test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
> +  ac_define_dir=`eval echo $SYSTEM_GDBINIT_DIR`
> +  ac_define_dir=`eval echo $ac_define_dir`
> +
> +cat >>confdefs.h <<_ACEOF
> +#define SYSTEM_GDBINIT_DIR "$ac_define_dir"
> +_ACEOF
> +
> +
> +
> +
> +  if test "x$exec_prefix" = xNONE || test "x$exec_prefix" = 'x${prefix}';
> then
> +     if test "x$prefix" = xNONE; then
> +       test_prefix=/usr/local
> +     else
> +       test_prefix=$prefix
> +     fi
> +  else
> +     test_prefix=$exec_prefix
> +  fi
> +  value=0
> +  case ${ac_define_dir} in
> +     "${test_prefix}"|"${test_prefix}/"*|\
> +       '${exec_prefix}'|'${exec_prefix}/'*)
> +     value=1
> +     ;;
> +  esac
> +
> +cat >>confdefs.h <<_ACEOF
> +#define SYSTEM_GDBINIT_DIR_RELOCATABLE $value
> +_ACEOF
> +
> +
> +
> +
> +
>  # Check whether --enable-werror was given.
>  if test "${enable_werror+set}" = set; then :
>    enableval=$enable_werror; case "${enableval}" in
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 9da8818fb5..f3f5bd0635 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -1843,6 +1843,9 @@ GDB_AC_DEFINE_RELOCATABLE(TARGET_SYSTEM_ROOT,
> sysroot, ${ac_define_dir})
>  GDB_AC_WITH_DIR(SYSTEM_GDBINIT, system-gdbinit,
>      [automatically load a system-wide gdbinit file],
>      [])
> +GDB_AC_WITH_DIR(SYSTEM_GDBINIT_DIR, system-gdbinit-dir,
> +    [automatically load system-wide gdbinit files from this directory],
> +    [])
>
>  AM_GDB_WARNINGS
>  AM_GDB_UBSAN
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index f2713c0396..eb385ae1ee 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -1083,6 +1083,16 @@ Its location is specified with the
> @code{--with-system-gdbinit}
>  configure option (@pxref{System-wide configuration}).
>  It is loaded first when @value{GDBN} starts, before command line options
>  have been processed.
> +@item @file{system.gdbinit.d}
> +This is the system-wide init directory.
> +Its location is specified with the @code{--with-system-gdbinit-dir}
> +configure option (@pxref{System-wide configuration}).
> +Files in this directory are loaded in alphabetical order immediately after
> +system.gdbinit (if enabled) when @value{GDBN} starts, before command line
> +options have been processed.  Files need to have a recognized scripting
> +language extension (@code{.py}/@code{.scm}) or be named with a @code{.gdb}
> +extension to be interpreted as regular @value{GDBN} commands.
> @value{GDBN}
> +will not recurse into any subdirectories of this directory.
>  @item @file{~/.gdbinit}
>  This is the init file in your home directory.
>  It is loaded next, after @file{system.gdbinit}, and before
> @@ -1315,8 +1325,11 @@ Sets up the command interpreter as specified by the
> command line
>  @cindex init file
>  Reads the system-wide @dfn{init file} (if @option{--with-system-gdbinit}
> was
>  used when building @value{GDBN}; @pxref{System-wide configuration,
> - ,System-wide configuration and settings}) and executes all the commands
> in
> -that file.
> + ,System-wide configuration and settings}) and the files in the
> system-wide
> +gdbinit directory (if @option{--with-system-gdbinit-dir} was used) and
> executes
> +all the commands in those files.  The files need to be named with a
> @code{.gdb}
> +extension to be interpreted as @value{GDBN} commands, or they can be
> written
> +in a supported scripting language with an appropriate file extension.
>
>  @anchor{Home Directory Init File}
>  @item
> @@ -37034,6 +37047,14 @@ directory under the configured prefix, and
> @value{GDBN} is moved to
>  another location after being built, the location of the system-wide
>  init file will be adjusted accordingly.
>
> +@item --with-system-gdbinit-dir=@var{directory}
> +Configure @value{GDBN} to automatically load init files from a
> +system-wide directory.  @var{directory} should be an absolute directory
> +name.  If @var{directory} is in a directory under the configured
> +prefix, and @value{GDBN} is moved to another location after being
> +built, the location of the system-wide init directory will be
> +adjusted accordingly.
> +
>  @item --enable-build-warnings
>  When building the @value{GDBN} sources, ask the compiler to warn about
>  any code which looks even vaguely suspicious.  It passes many
> @@ -37059,24 +37080,28 @@ was first introduced in GCC 4.9.
>  @section System-wide configuration and settings
>  @cindex system-wide init file
>
> -@value{GDBN} can be configured to have a system-wide init file;
> -this file will be read and executed at startup (@pxref{Startup, , What
> -@value{GDBN} does during startup}).
> +@value{GDBN} can be configured to have a system-wide init file and a
> +system-wide init file directory; this file and files in that directory
> +(if they have a recognized file extension) will be read and executed at
> +startup (@pxref{Startup, , What @value{GDBN} does during startup}).
>
> -Here is the corresponding configure option:
> +Here are the corresponding configure options:
>
>  @table @code
>  @item --with-system-gdbinit=@var{file}
>  Specify that the default location of the system-wide init file is
>  @var{file}.
> +@item --with-system-gdbinit-dir=@var{directory}
> +Specify that the default location of the system-wide init file directory
> +is @var{directory}.
>  @end table
>
>  If @value{GDBN} has been configured with the option
> @option{--prefix=$prefix},
> -it may be subject to relocation.  Two possible cases:
> +they may be subject to relocation.  Two possible cases:
>
>  @itemize @bullet
>  @item
> -If the default location of this init file contains @file{$prefix},
> +If the default location of this init file/directory contains
> @file{$prefix},
>  it will be subject to relocation.  Suppose that the configure options
>  are @option{--prefix=$prefix --with-system-gdbinit=$prefix/etc/gdbinit};
>  if @value{GDBN} is moved from @file{$prefix} to @file{$install}, the
> system
> @@ -37102,6 +37127,14 @@ initialization.  If the data-directory is changed
> after @value{GDBN} has
>  started with the @code{set data-directory} command, the file will not be
>  reread.
>
> +This applies similarly to the system-wide directory specified in
> +@option{--with-system-gdbinit-dir}.
> +
> +Any supported scripting language can be used for these init files, as long
> +as the file extension matches the scripting language.  To be interpreted
> +as regular @value{GDBN} commands, the files needs to have a @code{.gdb}
> +extension.
> +
>  @menu
>  * System-wide Configuration Scripts::  Installed System-wide
> Configuration Scripts
>  @end menu
> @@ -45591,6 +45624,10 @@ Richard M. Stallman and Roland H. Pesch, July
> 1991.
>  @value{SYSTEM_GDBINIT}
>  @end ifset
>
> +@ifset SYSTEM_GDBINIT_DIR
> +@value{SYSTEM_GDBINIT_DIR}/*
> +@end ifset
> +
>  ~/.gdbinit
>
>  ./.gdbinit
> @@ -45632,6 +45669,20 @@ See more in
>  the @value{GDBN} manual in node @code{System-wide configuration}
>  -- shell command @code{info -f gdb -n 'System-wide configuration'}.
>  @end ifset
> +@ifset SYSTEM_GDBINIT_DIR
> +@item @value{SYSTEM_GDBINIT_DIR}
> +@end ifset
> +@ifclear SYSTEM_GDBINIT_DIR
> +@item (not enabled with @code{--with-system-gdbinit-dir} during
> compilation)
> +@end ifclear
> +System-wide initialization directory.  All files in this directory are
> +executed on startup unless user specified @value{GDBN} option @code{-nx}
> or
> +@code{-n}, as long as they have a recognized file extension.
> +See more in
> +@ifset man
> +the @value{GDBN} manual in node @code{System-wide configuration}
> +-- shell command @code{info -f gdb -n 'System-wide configuration'}.
> +@end ifset
>  @ifclear man
>  @ref{System-wide configuration}.
>  @end ifclear
> diff --git a/gdb/extension.c b/gdb/extension.c
> index 8637bc53f2..1fb4b48003 100644
> --- a/gdb/extension.c
> +++ b/gdb/extension.c
> @@ -154,6 +154,9 @@ get_ext_lang_of_file (const char *file)
>    int i;
>    const struct extension_language_defn *extlang;
>
> +  if (has_extension (file, extension_language_gdb.suffix))
> +    return &extension_language_gdb;
> +
>    ALL_EXTENSION_LANGUAGES (i, extlang)
>      {
>        if (has_extension (file, extlang->suffix))
> diff --git a/gdb/main.c b/gdb/main.c
> index 7fab8ff8da..ed902371cb 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -45,6 +45,7 @@
>  #include "event-top.h"
>  #include "infrun.h"
>  #include "gdbsupport/signals-state-save-restore.h"
> +#include <algorithm>
>  #include <vector>
>  #include "gdbsupport/pathstuff.h"
>  #include "cli/cli-style.h"
> @@ -195,7 +196,8 @@ relocate_gdb_directory (const char *initial, bool
> relocatable)
>     otherwise.  */
>
>  static std::string
> -relocate_gdbinit_path_maybe_in_datadir (const std::string& file)
> +relocate_gdbinit_path_maybe_in_datadir (const std::string& file,
> +                                       bool relocatable)
>  {
>    size_t datadir_len = strlen (GDB_DATADIR);
>
> @@ -219,9 +221,8 @@ relocate_gdbinit_path_maybe_in_datadir (const
> std::string& file)
>      }
>    else
>      {
> -      relocated_path = relocate_path (gdb_program_name,
> -                                     file.c_str (),
> -                                     SYSTEM_GDBINIT_RELOCATABLE);
> +      relocated_path = relocate_path (gdb_program_name, file.c_str (),
> +                                     relocatable);
>      }
>      return relocated_path;
>  }
> @@ -232,11 +233,11 @@ relocate_gdbinit_path_maybe_in_datadir (const
> std::string& file)
>     to be loaded, then SYSTEM_GDBINIT (resp. HOME_GDBINIT and
>     LOCAL_GDBINIT) is set to the empty string.  */
>  static void
> -get_init_files (std::string *system_gdbinit,
> +get_init_files (std::vector<std::string> *system_gdbinit,
>                 std::string *home_gdbinit,
>                 std::string *local_gdbinit)
>  {
> -  static std::string sysgdbinit;
> +  static std::vector<std::string> sysgdbinit;
>    static std::string homeinit;
>    static std::string localinit;
>    static int initialized = 0;
> @@ -248,10 +249,49 @@ get_init_files (std::string *system_gdbinit,
>        if (SYSTEM_GDBINIT[0])
>         {
>           std::string relocated_sysgdbinit
> -           = relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT);
> +           = relocate_gdbinit_path_maybe_in_datadir
> +               (SYSTEM_GDBINIT, SYSTEM_GDBINIT_RELOCATABLE);
>           if (!relocated_sysgdbinit.empty ()
>               && stat (relocated_sysgdbinit.c_str (), &s) == 0)
> -           sysgdbinit = relocated_sysgdbinit;
> +           sysgdbinit.push_back (relocated_sysgdbinit);
> +       }
> +      if (SYSTEM_GDBINIT_DIR[0])
> +       {
> +         std::string relocated_gdbinit_dir
> +           = relocate_gdbinit_path_maybe_in_datadir
> +               (SYSTEM_GDBINIT_DIR, SYSTEM_GDBINIT_DIR_RELOCATABLE);
> +         DIR *dir = nullptr;
> +         if (!relocated_gdbinit_dir.empty ())
> +           dir = opendir (relocated_gdbinit_dir.c_str ());
> +         if (dir != nullptr)
> +           {
> +             std::vector<std::string> files;
> +             struct dirent *ent;
> +             for (;;)
> +               {
> +                 ent = readdir (dir);
> +                 if (ent == nullptr)
> +                   break;
> +                 if (ent->d_name[0] == '.')
> +                   continue;
> +                 /* ent->d_type is not available on all systems (e.g.
> mingw,
> +                    Solaris), so we have to call stat().  */
> +                 std::string filename
> +                   = relocated_gdbinit_dir + SLASH_STRING + ent->d_name;
> +                 if (stat(filename.c_str (), &s) != 0 || !S_ISREG
> (s.st_mode))
> +                   continue;
> +                 const struct extension_language_defn *extlang
> +                   = get_ext_lang_of_file (filename.c_str ());
> +                 /* We effectively don't support "set script-extension
> off/soft",
> +                    because we are loading system init files here, so it
> +                    does not really make sense to depend on a setting.  */
> +                 if (extlang != nullptr && ext_lang_present_p (extlang))
> +                   files.push_back (filename);
> +               }
> +             closedir (dir);
> +             std::sort (files.begin (), files.end ());
> +             sysgdbinit.insert (sysgdbinit.end (), files.begin (),
> files.end ());
> +           }
>         }
>
>        const char *homedir = getenv ("HOME");
> @@ -916,7 +956,7 @@ captured_main_1 (struct captured_main_args *context)
>    /* Lookup gdbinit files.  Note that the gdbinit file name may be
>       overriden during file initialization, so get_init_files should be
>       called after gdb_init.  */
> -  std::string system_gdbinit;
> +  std::vector<std::string> system_gdbinit;
>    std::string home_gdbinit;
>    std::string local_gdbinit;
>    get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
> @@ -996,7 +1036,10 @@ captured_main_1 (struct captured_main_args *context)
>       processed; it sets global parameters, which are independent of
>       what file you are debugging or what directory you are in.  */
>    if (!system_gdbinit.empty () && !inhibit_gdbinit)
> -    ret = catch_command_errors (source_script, system_gdbinit.c_str (),
> 0);
> +    {
> +      for (const std::string& file : system_gdbinit)
> +       ret = catch_command_errors (source_script, file.c_str (), 0);
> +    }
>
>    /* Read and execute $HOME/.gdbinit file, if it exists.  This is done
>       *before* all the command line arguments are processed; it sets
> @@ -1214,7 +1257,7 @@ gdb_main (struct captured_main_args *args)
>  static void
>  print_gdb_help (struct ui_file *stream)
>  {
> -  std::string system_gdbinit;
> +  std::vector<std::string> system_gdbinit;
>    std::string home_gdbinit;
>    std::string local_gdbinit;
>
> @@ -1295,9 +1338,18 @@ Other options:\n\n\
>  At startup, GDB reads the following init files and executes their
> commands:\n\
>  "), stream);
>    if (!system_gdbinit.empty ())
> -    fprintf_unfiltered (stream, _("\
> -   * system-wide init file: %s\n\
> -"), system_gdbinit.c_str ());
> +    {
> +      std::string output;
> +      for (size_t idx = 0; idx < system_gdbinit.size (); ++idx)
> +        {
> +         output += system_gdbinit[idx];
> +         if (idx < system_gdbinit.size () - 1)
> +           output += ", ";
> +       }
> +      fprintf_unfiltered (stream, _("\
> +   * system-wide init files: %s\n\
> +"), output.c_str ());
> +    }
>    if (!home_gdbinit.empty ())
>      fprintf_unfiltered (stream, _("\
>     * user-specific init file: %s\n\
> diff --git a/gdb/top.c b/gdb/top.c
> index 49e6daae94..7a45f1b4aa 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -1515,6 +1515,10 @@ This GDB was configured as follows:\n\
>      fprintf_filtered (stream, _("\
>               --with-system-gdbinit=%s%s\n\
>  "), SYSTEM_GDBINIT, SYSTEM_GDBINIT_RELOCATABLE ? " (relocatable)" : "");
> +  if (SYSTEM_GDBINIT_DIR[0])
> +    fprintf_filtered (stream, _("\
> +             --with-system-gdbinit-dir=%s%s\n\
> +"), SYSTEM_GDBINIT_DIR, SYSTEM_GDBINIT_DIR_RELOCATABLE ? " (relocatable)"
> : "");
>      /* We assume "relocatable" will be printed at least once, thus we
> always
>         print this text.  It's a reasonably safe assumption for now.  */
>      fprintf_filtered (stream, _("\n\
> --
> 2.23.0.351.gc4317032e6-goog
>
>

Re: [PATCH 3/3 v3] Load system gdbinit files from a directory

[Christian Biesinger via gdb-patches]

Ping

On Thu, Oct 3, 2019 at 2:41 PM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> Ping.
>
> (Eli -- please re-review the documentation parts of this patch, I updated them per your comments)
>
> Christian
>
> On Tue, Sep 24, 2019 at 11:29 AM Christian Biesinger <cbiesinger@google.com> wrote:
>>
>> [Looks like I never did send this updated patch :( Please take another
>> look!]
>>
>> Adds a configure option --with-system-gdbinit-dir to specify a directory
>> in which to look for gdbinit files.  All files in this directory are
>> loaded on startup (subject to -n/-nx as usual) as long as the extension
>> matches a known and enabled scripting language (.gdb/.py/.scm).
>>
>> This also changes get_ext_lang_of_file to support ".gdb" files, similar
>> to get_ext_lang_defn's handling of EXT_LANG_GDB.
>>
>> gdb/ChangeLog:
>>
>> 2019-08-20  Christian Biesinger  <cbiesinger@google.com>
>>
>>         * NEWS: Mention new --with-system-gdbinit-dir option.
>>         * config.in: Regenerate.
>>         * configure: Regenerate.
>>         * configure.ac: Add new option --with-system-gdbinit-dir.
>>         * extension.c (get_ext_lang_of_file): Return extension_language_gdb
>>         for a ".gdb" suffix.
>>         * main.c (get_init_files): Change system_gdbinit argument to
>>         a vector and return the files in SYSTEM_GDBINIT_DIR in
>>         addition to SYSTEM_GDBINIT.
>>         (captured_main_1): Update.
>>         (print_gdb_help): Update.
>>         * top.c (print_gdb_configuration): Also print the value of
>>         SYSTEM_GDBINIT_DIR.
>>
>> gdb/doc/ChangeLog:
>>
>> 2019-08-25  Christian Biesinger  <cbiesinger@google.com>
>>
>>         * gdb.texinfo (many sections): Document new --with-system-gdbinit-dir
>>         option.
>> ---
>>  gdb/NEWS            |  6 ++++
>>  gdb/config.in       |  7 ++++
>>  gdb/configure       | 51 +++++++++++++++++++++++++++++
>>  gdb/configure.ac    |  3 ++
>>  gdb/doc/gdb.texinfo | 67 ++++++++++++++++++++++++++++++++-----
>>  gdb/extension.c     |  3 ++
>>  gdb/main.c          | 80 +++++++++++++++++++++++++++++++++++++--------
>>  gdb/top.c           |  4 +++
>>  8 files changed, 199 insertions(+), 22 deletions(-)
>>
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index 779fd91d3a..1d0f56a119 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -34,6 +34,12 @@
>>
>>  * GDB can now be compiled with Python 3 on Windows.
>>
>> +* In addition to the system-wide gdbinit file, if configured with
>> + --with-system-gdbinit-dir, GDB will now also load files in that directory
>> + as system gdbinit files, unless the -nx or -n flag is provided.  Files
>> + with extensions .gdb, .py and .scm are supported as long as GDB was
>> + compiled with support for that language.
>> +
>>  * Python API
>>
>>    ** The gdb.Value type has a new method 'format_string' which returns a
>> diff --git a/gdb/config.in b/gdb/config.in
>> index 26ca02f6a3..c0745c90a3 100644
>> --- a/gdb/config.in
>> +++ b/gdb/config.in
>> @@ -684,6 +684,13 @@
>>  /* automatically load a system-wide gdbinit file */
>>  #undef SYSTEM_GDBINIT
>>
>> +/* automatically load system-wide gdbinit files from this directory */
>> +#undef SYSTEM_GDBINIT_DIR
>> +
>> +/* Define if the system-gdbinit-dir directory should be relocated when GDB is
>> +   moved. */
>> +#undef SYSTEM_GDBINIT_DIR_RELOCATABLE
>> +
>>  /* Define if the system-gdbinit directory should be relocated when GDB is
>>     moved. */
>>  #undef SYSTEM_GDBINIT_RELOCATABLE
>> diff --git a/gdb/configure b/gdb/configure
>> index 22a5f6051d..8436f06c9f 100755
>> --- a/gdb/configure
>> +++ b/gdb/configure
>> @@ -693,6 +693,7 @@ WIN32LIBS
>>  SER_HARDWIRE
>>  WERROR_CFLAGS
>>  WARN_CFLAGS
>> +SYSTEM_GDBINIT_DIR
>>  SYSTEM_GDBINIT
>>  TARGET_SYSTEM_ROOT
>>  CONFIG_LDFLAGS
>> @@ -884,6 +885,7 @@ with_libipt_prefix
>>  with_included_regex
>>  with_sysroot
>>  with_system_gdbinit
>> +with_system_gdbinit_dir
>>  enable_werror
>>  enable_build_warnings
>>  enable_gdb_build_warnings
>> @@ -1618,6 +1620,9 @@ Optional Packages:
>>    --with-sysroot[=DIR]    search for usr/lib et al within DIR
>>    --with-system-gdbinit=PATH
>>                            automatically load a system-wide gdbinit file
>> +  --with-system-gdbinit-dir=PATH
>> +                          automatically load system-wide gdbinit files from
>> +                          this directory
>>    --with-lzma             support lzma compression (auto/yes/no)
>>    --with-liblzma-prefix[=DIR]  search for liblzma in DIR/include and DIR/lib
>>    --without-liblzma-prefix     don't search for liblzma in includedir and libdir
>> @@ -15238,6 +15243,52 @@ _ACEOF
>>
>>
>>
>> +# Check whether --with-system-gdbinit-dir was given.
>> +if test "${with_system_gdbinit_dir+set}" = set; then :
>> +  withval=$with_system_gdbinit_dir;
>> +    SYSTEM_GDBINIT_DIR=$withval
>> +else
>> +  SYSTEM_GDBINIT_DIR=
>> +fi
>> +
>> +
>> +  test "x$prefix" = xNONE && prefix="$ac_default_prefix"
>> +  test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
>> +  ac_define_dir=`eval echo $SYSTEM_GDBINIT_DIR`
>> +  ac_define_dir=`eval echo $ac_define_dir`
>> +
>> +cat >>confdefs.h <<_ACEOF
>> +#define SYSTEM_GDBINIT_DIR "$ac_define_dir"
>> +_ACEOF
>> +
>> +
>> +
>> +
>> +  if test "x$exec_prefix" = xNONE || test "x$exec_prefix" = 'x${prefix}'; then
>> +     if test "x$prefix" = xNONE; then
>> +       test_prefix=/usr/local
>> +     else
>> +       test_prefix=$prefix
>> +     fi
>> +  else
>> +     test_prefix=$exec_prefix
>> +  fi
>> +  value=0
>> +  case ${ac_define_dir} in
>> +     "${test_prefix}"|"${test_prefix}/"*|\
>> +       '${exec_prefix}'|'${exec_prefix}/'*)
>> +     value=1
>> +     ;;
>> +  esac
>> +
>> +cat >>confdefs.h <<_ACEOF
>> +#define SYSTEM_GDBINIT_DIR_RELOCATABLE $value
>> +_ACEOF
>> +
>> +
>> +
>> +
>> +
>>  # Check whether --enable-werror was given.
>>  if test "${enable_werror+set}" = set; then :
>>    enableval=$enable_werror; case "${enableval}" in
>> diff --git a/gdb/configure.ac b/gdb/configure.ac
>> index 9da8818fb5..f3f5bd0635 100644
>> --- a/gdb/configure.ac
>> +++ b/gdb/configure.ac
>> @@ -1843,6 +1843,9 @@ GDB_AC_DEFINE_RELOCATABLE(TARGET_SYSTEM_ROOT, sysroot, ${ac_define_dir})
>>  GDB_AC_WITH_DIR(SYSTEM_GDBINIT, system-gdbinit,
>>      [automatically load a system-wide gdbinit file],
>>      [])
>> +GDB_AC_WITH_DIR(SYSTEM_GDBINIT_DIR, system-gdbinit-dir,
>> +    [automatically load system-wide gdbinit files from this directory],
>> +    [])
>>
>>  AM_GDB_WARNINGS
>>  AM_GDB_UBSAN
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index f2713c0396..eb385ae1ee 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -1083,6 +1083,16 @@ Its location is specified with the @code{--with-system-gdbinit}
>>  configure option (@pxref{System-wide configuration}).
>>  It is loaded first when @value{GDBN} starts, before command line options
>>  have been processed.
>> +@item @file{system.gdbinit.d}
>> +This is the system-wide init directory.
>> +Its location is specified with the @code{--with-system-gdbinit-dir}
>> +configure option (@pxref{System-wide configuration}).
>> +Files in this directory are loaded in alphabetical order immediately after
>> +system.gdbinit (if enabled) when @value{GDBN} starts, before command line
>> +options have been processed.  Files need to have a recognized scripting
>> +language extension (@code{.py}/@code{.scm}) or be named with a @code{.gdb}
>> +extension to be interpreted as regular @value{GDBN} commands.  @value{GDBN}
>> +will not recurse into any subdirectories of this directory.
>>  @item @file{~/.gdbinit}
>>  This is the init file in your home directory.
>>  It is loaded next, after @file{system.gdbinit}, and before
>> @@ -1315,8 +1325,11 @@ Sets up the command interpreter as specified by the command line
>>  @cindex init file
>>  Reads the system-wide @dfn{init file} (if @option{--with-system-gdbinit} was
>>  used when building @value{GDBN}; @pxref{System-wide configuration,
>> - ,System-wide configuration and settings}) and executes all the commands in
>> -that file.
>> + ,System-wide configuration and settings}) and the files in the system-wide
>> +gdbinit directory (if @option{--with-system-gdbinit-dir} was used) and executes
>> +all the commands in those files.  The files need to be named with a @code{.gdb}
>> +extension to be interpreted as @value{GDBN} commands, or they can be written
>> +in a supported scripting language with an appropriate file extension.
>>
>>  @anchor{Home Directory Init File}
>>  @item
>> @@ -37034,6 +37047,14 @@ directory under the configured prefix, and @value{GDBN} is moved to
>>  another location after being built, the location of the system-wide
>>  init file will be adjusted accordingly.
>>
>> +@item --with-system-gdbinit-dir=@var{directory}
>> +Configure @value{GDBN} to automatically load init files from a
>> +system-wide directory.  @var{directory} should be an absolute directory
>> +name.  If @var{directory} is in a directory under the configured
>> +prefix, and @value{GDBN} is moved to another location after being
>> +built, the location of the system-wide init directory will be
>> +adjusted accordingly.
>> +
>>  @item --enable-build-warnings
>>  When building the @value{GDBN} sources, ask the compiler to warn about
>>  any code which looks even vaguely suspicious.  It passes many
>> @@ -37059,24 +37080,28 @@ was first introduced in GCC 4.9.
>>  @section System-wide configuration and settings
>>  @cindex system-wide init file
>>
>> -@value{GDBN} can be configured to have a system-wide init file;
>> -this file will be read and executed at startup (@pxref{Startup, , What
>> -@value{GDBN} does during startup}).
>> +@value{GDBN} can be configured to have a system-wide init file and a
>> +system-wide init file directory; this file and files in that directory
>> +(if they have a recognized file extension) will be read and executed at
>> +startup (@pxref{Startup, , What @value{GDBN} does during startup}).
>>
>> -Here is the corresponding configure option:
>> +Here are the corresponding configure options:
>>
>>  @table @code
>>  @item --with-system-gdbinit=@var{file}
>>  Specify that the default location of the system-wide init file is
>>  @var{file}.
>> +@item --with-system-gdbinit-dir=@var{directory}
>> +Specify that the default location of the system-wide init file directory
>> +is @var{directory}.
>>  @end table
>>
>>  If @value{GDBN} has been configured with the option @option{--prefix=$prefix},
>> -it may be subject to relocation.  Two possible cases:
>> +they may be subject to relocation.  Two possible cases:
>>
>>  @itemize @bullet
>>  @item
>> -If the default location of this init file contains @file{$prefix},
>> +If the default location of this init file/directory contains @file{$prefix},
>>  it will be subject to relocation.  Suppose that the configure options
>>  are @option{--prefix=$prefix --with-system-gdbinit=$prefix/etc/gdbinit};
>>  if @value{GDBN} is moved from @file{$prefix} to @file{$install}, the system
>> @@ -37102,6 +37127,14 @@ initialization.  If the data-directory is changed after @value{GDBN} has
>>  started with the @code{set data-directory} command, the file will not be
>>  reread.
>>
>> +This applies similarly to the system-wide directory specified in
>> +@option{--with-system-gdbinit-dir}.
>> +
>> +Any supported scripting language can be used for these init files, as long
>> +as the file extension matches the scripting language.  To be interpreted
>> +as regular @value{GDBN} commands, the files needs to have a @code{.gdb}
>> +extension.
>> +
>>  @menu
>>  * System-wide Configuration Scripts::  Installed System-wide Configuration Scripts
>>  @end menu
>> @@ -45591,6 +45624,10 @@ Richard M. Stallman and Roland H. Pesch, July 1991.
>>  @value{SYSTEM_GDBINIT}
>>  @end ifset
>>
>> +@ifset SYSTEM_GDBINIT_DIR
>> +@value{SYSTEM_GDBINIT_DIR}/*
>> +@end ifset
>> +
>>  ~/.gdbinit
>>
>>  ./.gdbinit
>> @@ -45632,6 +45669,20 @@ See more in
>>  the @value{GDBN} manual in node @code{System-wide configuration}
>>  -- shell command @code{info -f gdb -n 'System-wide configuration'}.
>>  @end ifset
>> +@ifset SYSTEM_GDBINIT_DIR
>> +@item @value{SYSTEM_GDBINIT_DIR}
>> +@end ifset
>> +@ifclear SYSTEM_GDBINIT_DIR
>> +@item (not enabled with @code{--with-system-gdbinit-dir} during compilation)
>> +@end ifclear
>> +System-wide initialization directory.  All files in this directory are
>> +executed on startup unless user specified @value{GDBN} option @code{-nx} or
>> +@code{-n}, as long as they have a recognized file extension.
>> +See more in
>> +@ifset man
>> +the @value{GDBN} manual in node @code{System-wide configuration}
>> +-- shell command @code{info -f gdb -n 'System-wide configuration'}.
>> +@end ifset
>>  @ifclear man
>>  @ref{System-wide configuration}.
>>  @end ifclear
>> diff --git a/gdb/extension.c b/gdb/extension.c
>> index 8637bc53f2..1fb4b48003 100644
>> --- a/gdb/extension.c
>> +++ b/gdb/extension.c
>> @@ -154,6 +154,9 @@ get_ext_lang_of_file (const char *file)
>>    int i;
>>    const struct extension_language_defn *extlang;
>>
>> +  if (has_extension (file, extension_language_gdb.suffix))
>> +    return &extension_language_gdb;
>> +
>>    ALL_EXTENSION_LANGUAGES (i, extlang)
>>      {
>>        if (has_extension (file, extlang->suffix))
>> diff --git a/gdb/main.c b/gdb/main.c
>> index 7fab8ff8da..ed902371cb 100644
>> --- a/gdb/main.c
>> +++ b/gdb/main.c
>> @@ -45,6 +45,7 @@
>>  #include "event-top.h"
>>  #include "infrun.h"
>>  #include "gdbsupport/signals-state-save-restore.h"
>> +#include <algorithm>
>>  #include <vector>
>>  #include "gdbsupport/pathstuff.h"
>>  #include "cli/cli-style.h"
>> @@ -195,7 +196,8 @@ relocate_gdb_directory (const char *initial, bool relocatable)
>>     otherwise.  */
>>
>>  static std::string
>> -relocate_gdbinit_path_maybe_in_datadir (const std::string& file)
>> +relocate_gdbinit_path_maybe_in_datadir (const std::string& file,
>> +                                       bool relocatable)
>>  {
>>    size_t datadir_len = strlen (GDB_DATADIR);
>>
>> @@ -219,9 +221,8 @@ relocate_gdbinit_path_maybe_in_datadir (const std::string& file)
>>      }
>>    else
>>      {
>> -      relocated_path = relocate_path (gdb_program_name,
>> -                                     file.c_str (),
>> -                                     SYSTEM_GDBINIT_RELOCATABLE);
>> +      relocated_path = relocate_path (gdb_program_name, file.c_str (),
>> +                                     relocatable);
>>      }
>>      return relocated_path;
>>  }
>> @@ -232,11 +233,11 @@ relocate_gdbinit_path_maybe_in_datadir (const std::string& file)
>>     to be loaded, then SYSTEM_GDBINIT (resp. HOME_GDBINIT and
>>     LOCAL_GDBINIT) is set to the empty string.  */
>>  static void
>> -get_init_files (std::string *system_gdbinit,
>> +get_init_files (std::vector<std::string> *system_gdbinit,
>>                 std::string *home_gdbinit,
>>                 std::string *local_gdbinit)
>>  {
>> -  static std::string sysgdbinit;
>> +  static std::vector<std::string> sysgdbinit;
>>    static std::string homeinit;
>>    static std::string localinit;
>>    static int initialized = 0;
>> @@ -248,10 +249,49 @@ get_init_files (std::string *system_gdbinit,
>>        if (SYSTEM_GDBINIT[0])
>>         {
>>           std::string relocated_sysgdbinit
>> -           = relocate_gdbinit_path_maybe_in_datadir (SYSTEM_GDBINIT);
>> +           = relocate_gdbinit_path_maybe_in_datadir
>> +               (SYSTEM_GDBINIT, SYSTEM_GDBINIT_RELOCATABLE);
>>           if (!relocated_sysgdbinit.empty ()
>>               && stat (relocated_sysgdbinit.c_str (), &s) == 0)
>> -           sysgdbinit = relocated_sysgdbinit;
>> +           sysgdbinit.push_back (relocated_sysgdbinit);
>> +       }
>> +      if (SYSTEM_GDBINIT_DIR[0])
>> +       {
>> +         std::string relocated_gdbinit_dir
>> +           = relocate_gdbinit_path_maybe_in_datadir
>> +               (SYSTEM_GDBINIT_DIR, SYSTEM_GDBINIT_DIR_RELOCATABLE);
>> +         DIR *dir = nullptr;
>> +         if (!relocated_gdbinit_dir.empty ())
>> +           dir = opendir (relocated_gdbinit_dir.c_str ());
>> +         if (dir != nullptr)
>> +           {
>> +             std::vector<std::string> files;
>> +             struct dirent *ent;
>> +             for (;;)
>> +               {
>> +                 ent = readdir (dir);
>> +                 if (ent == nullptr)
>> +                   break;
>> +                 if (ent->d_name[0] == '.')
>> +                   continue;
>> +                 /* ent->d_type is not available on all systems (e.g. mingw,
>> +                    Solaris), so we have to call stat().  */
>> +                 std::string filename
>> +                   = relocated_gdbinit_dir + SLASH_STRING + ent->d_name;
>> +                 if (stat(filename.c_str (), &s) != 0 || !S_ISREG (s.st_mode))
>> +                   continue;
>> +                 const struct extension_language_defn *extlang
>> +                   = get_ext_lang_of_file (filename.c_str ());
>> +                 /* We effectively don't support "set script-extension off/soft",
>> +                    because we are loading system init files here, so it
>> +                    does not really make sense to depend on a setting.  */
>> +                 if (extlang != nullptr && ext_lang_present_p (extlang))
>> +                   files.push_back (filename);
>> +               }
>> +             closedir (dir);
>> +             std::sort (files.begin (), files.end ());
>> +             sysgdbinit.insert (sysgdbinit.end (), files.begin (), files.end ());
>> +           }
>>         }
>>
>>        const char *homedir = getenv ("HOME");
>> @@ -916,7 +956,7 @@ captured_main_1 (struct captured_main_args *context)
>>    /* Lookup gdbinit files.  Note that the gdbinit file name may be
>>       overriden during file initialization, so get_init_files should be
>>       called after gdb_init.  */
>> -  std::string system_gdbinit;
>> +  std::vector<std::string> system_gdbinit;
>>    std::string home_gdbinit;
>>    std::string local_gdbinit;
>>    get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
>> @@ -996,7 +1036,10 @@ captured_main_1 (struct captured_main_args *context)
>>       processed; it sets global parameters, which are independent of
>>       what file you are debugging or what directory you are in.  */
>>    if (!system_gdbinit.empty () && !inhibit_gdbinit)
>> -    ret = catch_command_errors (source_script, system_gdbinit.c_str (), 0);
>> +    {
>> +      for (const std::string& file : system_gdbinit)
>> +       ret = catch_command_errors (source_script, file.c_str (), 0);
>> +    }
>>
>>    /* Read and execute $HOME/.gdbinit file, if it exists.  This is done
>>       *before* all the command line arguments are processed; it sets
>> @@ -1214,7 +1257,7 @@ gdb_main (struct captured_main_args *args)
>>  static void
>>  print_gdb_help (struct ui_file *stream)
>>  {
>> -  std::string system_gdbinit;
>> +  std::vector<std::string> system_gdbinit;
>>    std::string home_gdbinit;
>>    std::string local_gdbinit;
>>
>> @@ -1295,9 +1338,18 @@ Other options:\n\n\
>>  At startup, GDB reads the following init files and executes their commands:\n\
>>  "), stream);
>>    if (!system_gdbinit.empty ())
>> -    fprintf_unfiltered (stream, _("\
>> -   * system-wide init file: %s\n\
>> -"), system_gdbinit.c_str ());
>> +    {
>> +      std::string output;
>> +      for (size_t idx = 0; idx < system_gdbinit.size (); ++idx)
>> +        {
>> +         output += system_gdbinit[idx];
>> +         if (idx < system_gdbinit.size () - 1)
>> +           output += ", ";
>> +       }
>> +      fprintf_unfiltered (stream, _("\
>> +   * system-wide init files: %s\n\
>> +"), output.c_str ());
>> +    }
>>    if (!home_gdbinit.empty ())
>>      fprintf_unfiltered (stream, _("\
>>     * user-specific init file: %s\n\
>> diff --git a/gdb/top.c b/gdb/top.c
>> index 49e6daae94..7a45f1b4aa 100644
>> --- a/gdb/top.c
>> +++ b/gdb/top.c
>> @@ -1515,6 +1515,10 @@ This GDB was configured as follows:\n\
>>      fprintf_filtered (stream, _("\
>>               --with-system-gdbinit=%s%s\n\
>>  "), SYSTEM_GDBINIT, SYSTEM_GDBINIT_RELOCATABLE ? " (relocatable)" : "");
>> +  if (SYSTEM_GDBINIT_DIR[0])
>> +    fprintf_filtered (stream, _("\
>> +             --with-system-gdbinit-dir=%s%s\n\
>> +"), SYSTEM_GDBINIT_DIR, SYSTEM_GDBINIT_DIR_RELOCATABLE ? " (relocatable)" : "");
>>      /* We assume "relocatable" will be printed at least once, thus we always
>>         print this text.  It's a reasonably safe assumption for now.  */
>>      fprintf_filtered (stream, _("\n\
>> --
>> 2.23.0.351.gc4317032e6-goog
>>

Re: [PATCH 3/3] Load system gdbinit files from a directory

[Sergio Durigan Junior]

On Tuesday, August 20 2019, Christian Biesinger via gdb-patches wrote:

> diff --git a/gdb/configure b/gdb/configure
> index cb71bbf057..e5aa2e6b3b 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -693,6 +693,7 @@ WIN32LIBS
>  SER_HARDWIRE
>  WERROR_CFLAGS
>  WARN_CFLAGS
> +SYSTEM_GDBINIT_DIR
>  SYSTEM_GDBINIT
>  TARGET_SYSTEM_ROOT
>  CONFIG_LDFLAGS
> @@ -824,6 +825,7 @@ infodir
>  docdir
>  oldincludedir
>  includedir
> +runstatedir
>  localstatedir
>  sharedstatedir
>  sysconfdir
> @@ -884,6 +886,7 @@ with_libipt_prefix
>  with_included_regex
>  with_sysroot
>  with_system_gdbinit
> +with_system_gdbinit_dir
>  enable_werror
>  enable_build_warnings
>  enable_gdb_build_warnings
> @@ -956,6 +959,7 @@ datadir='${datarootdir}'
>  sysconfdir='${prefix}/etc'
>  sharedstatedir='${prefix}/com'
>  localstatedir='${prefix}/var'
> +runstatedir='${localstatedir}/run'
>  includedir='${prefix}/include'
>  oldincludedir='/usr/include'
>  docdir='${datarootdir}/doc/${PACKAGE}'
> @@ -1208,6 +1212,15 @@ do
>    | -silent | --silent | --silen | --sile | --sil)
>      silent=yes ;;
>  
> +  -runstatedir | --runstatedir | --runstatedi | --runstated \
> +  | --runstate | --runstat | --runsta | --runst | --runs \
> +  | --run | --ru | --r)
> +    ac_prev=runstatedir ;;
> +  -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
> +  | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
> +  | --run=* | --ru=* | --r=*)
> +    runstatedir=$ac_optarg ;;
> +

Something else I forgot to comment: this 'runstatedir' addition seems
unrelated to the patch; it's probably due to the autoconf version you're
using to regenerate these files.

Make sure you have the correct versions installed:

  https://sourceware.org/gdb/wiki/DeveloperTips#Editing_configure.ac

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH 3/3] Load system gdbinit files from a directory

[Christian Biesinger via gdb-patches]

On Wed, Aug 21, 2019 at 1:15 PM Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
>
> On Tuesday, August 20 2019, Christian Biesinger via gdb-patches wrote:
>
> > diff --git a/gdb/configure b/gdb/configure
> > index cb71bbf057..e5aa2e6b3b 100755
> > --- a/gdb/configure
> > +++ b/gdb/configure
> > @@ -693,6 +693,7 @@ WIN32LIBS
> >  SER_HARDWIRE
> >  WERROR_CFLAGS
> >  WARN_CFLAGS
> > +SYSTEM_GDBINIT_DIR
> >  SYSTEM_GDBINIT
> >  TARGET_SYSTEM_ROOT
> >  CONFIG_LDFLAGS
> > @@ -824,6 +825,7 @@ infodir
> >  docdir
> >  oldincludedir
> >  includedir
> > +runstatedir
> >  localstatedir
> >  sharedstatedir
> >  sysconfdir
> > @@ -884,6 +886,7 @@ with_libipt_prefix
> >  with_included_regex
> >  with_sysroot
> >  with_system_gdbinit
> > +with_system_gdbinit_dir
> >  enable_werror
> >  enable_build_warnings
> >  enable_gdb_build_warnings
> > @@ -956,6 +959,7 @@ datadir='${datarootdir}'
> >  sysconfdir='${prefix}/etc'
> >  sharedstatedir='${prefix}/com'
> >  localstatedir='${prefix}/var'
> > +runstatedir='${localstatedir}/run'
> >  includedir='${prefix}/include'
> >  oldincludedir='/usr/include'
> >  docdir='${datarootdir}/doc/${PACKAGE}'
> > @@ -1208,6 +1212,15 @@ do
> >    | -silent | --silent | --silen | --sile | --sil)
> >      silent=yes ;;
> >
> > +  -runstatedir | --runstatedir | --runstatedi | --runstated \
> > +  | --runstate | --runstat | --runsta | --runst | --runs \
> > +  | --run | --ru | --r)
> > +    ac_prev=runstatedir ;;
> > +  -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
> > +  | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
> > +  | --run=* | --ru=* | --r=*)
> > +    runstatedir=$ac_optarg ;;
> > +
>
> Something else I forgot to comment: this 'runstatedir' addition seems
> unrelated to the patch; it's probably due to the autoconf version you're
> using to regenerate these files.
>
> Make sure you have the correct versions installed:
>
>   https://sourceware.org/gdb/wiki/DeveloperTips#Editing_configure.ac

Turns out the debian version of autoconf has a local patch to do this
runstate thing :(
https://sources.debian.org/src/autoconf/2.69-11/debian/patches/add-runstatedir.patch/

Anyway, I'll hold off on updating this patch pending the discussion
about the approach in the other thread. (But patches 1 and 2 may still
be good to get committed)

Christian

[PATCH 1/3] Refactor get_init_files to use std::string

[Christian Biesinger via gdb-patches]

To avoid manual memory management.

Tested on buildbot.

gdb/ChangeLog:

2019-08-20  Christian Biesinger  <cbiesinger@google.com>

	* main.c (get_init_files): Change to use std::string.
	(captured_main_1): Update.
	(print_gdb_help): Update.
---
 gdb/main.c | 96 ++++++++++++++++++++++++++----------------------------
 1 file changed, 46 insertions(+), 50 deletions(-)

diff --git a/gdb/main.c b/gdb/main.c
index 678c413021..b9e12589ab 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -195,27 +195,26 @@ relocate_gdb_directory (const char *initial, int flag)
    return them in SYSTEM_GDBINIT, HOME_GDBINIT, LOCAL_GDBINIT.  If
    there is no system gdbinit (resp. home gdbinit and local gdbinit)
    to be loaded, then SYSTEM_GDBINIT (resp. HOME_GDBINIT and
-   LOCAL_GDBINIT) is set to NULL.  */
+   LOCAL_GDBINIT) is set to the empty string.  */
 static void
-get_init_files (const char **system_gdbinit,
-		const char **home_gdbinit,
-		const char **local_gdbinit)
+get_init_files (std::string *system_gdbinit,
+		std::string *home_gdbinit,
+		std::string *local_gdbinit)
 {
-  static const char *sysgdbinit = NULL;
-  static char *homeinit = NULL;
-  static const char *localinit = NULL;
+  static std::string sysgdbinit;
+  static std::string homeinit;
+  static std::string localinit;
   static int initialized = 0;
 
   if (!initialized)
     {
       struct stat homebuf, cwdbuf, s;
-      const char *homedir;
 
       if (SYSTEM_GDBINIT[0])
 	{
 	  int datadir_len = strlen (GDB_DATADIR);
 	  int sys_gdbinit_len = strlen (SYSTEM_GDBINIT);
-	  char *relocated_sysgdbinit;
+	  std::string relocated_sysgdbinit;
 
 	  /* If SYSTEM_GDBINIT lives in data-directory, and data-directory
 	     has been provided, search for SYSTEM_GDBINIT there.  */
@@ -226,28 +225,30 @@ get_init_files (const char **system_gdbinit,
 	    {
 	      /* Append the part of SYSTEM_GDBINIT that follows GDB_DATADIR
 		 to gdb_datadir.  */
-	      char *tmp_sys_gdbinit = xstrdup (&SYSTEM_GDBINIT[datadir_len]);
-	      char *p;
 
-	      for (p = tmp_sys_gdbinit; IS_DIR_SEPARATOR (*p); ++p)
+	      size_t start = datadir_len;
+	      for (; IS_DIR_SEPARATOR (SYSTEM_GDBINIT[start]); ++start)
 		continue;
-	      relocated_sysgdbinit = concat (gdb_datadir, SLASH_STRING, p,
-					     (char *) NULL);
-	      xfree (tmp_sys_gdbinit);
+	      relocated_sysgdbinit = std::string (gdb_datadir) + SLASH_STRING +
+		&SYSTEM_GDBINIT[start];
 	    }
 	  else
 	    {
-	      relocated_sysgdbinit = relocate_path (gdb_program_name,
-						    SYSTEM_GDBINIT,
-						    SYSTEM_GDBINIT_RELOCATABLE);
+	      char *relocated = relocate_path (gdb_program_name,
+					       SYSTEM_GDBINIT,
+					       SYSTEM_GDBINIT_RELOCATABLE);
+	      if (relocated != nullptr)
+	        {
+		  relocated_sysgdbinit = relocated;
+		  xfree (relocated);
+		}
 	    }
-	  if (relocated_sysgdbinit && stat (relocated_sysgdbinit, &s) == 0)
+	  if (!relocated_sysgdbinit.empty () &&
+	      stat (relocated_sysgdbinit.c_str (), &s) == 0)
 	    sysgdbinit = relocated_sysgdbinit;
-	  else
-	    xfree (relocated_sysgdbinit);
 	}
 
-      homedir = getenv ("HOME");
+      const char *homedir = getenv ("HOME");
 
       /* If the .gdbinit file in the current directory is the same as
 	 the $HOME/.gdbinit file, it should not be sourced.  homebuf
@@ -260,17 +261,16 @@ get_init_files (const char **system_gdbinit,
 
       if (homedir)
 	{
-	  homeinit = xstrprintf ("%s/%s", homedir, GDBINIT);
-	  if (stat (homeinit, &homebuf) != 0)
+	  homeinit = std::string (homedir) + "/" + GDBINIT;
+	  if (stat (homeinit.c_str (), &homebuf) != 0)
 	    {
-	      xfree (homeinit);
-	      homeinit = NULL;
+	      homeinit = "";
 	    }
 	}
 
       if (stat (GDBINIT, &cwdbuf) == 0)
 	{
-	  if (!homeinit
+	  if (homeinit.empty ()
 	      || memcmp ((char *) &homebuf, (char *) &cwdbuf,
 			 sizeof (struct stat)))
 	    localinit = GDBINIT;
@@ -470,11 +470,6 @@ captured_main_1 (struct captured_main_args *context)
   /* All arguments of --directory option.  */
   std::vector<char *> dirarg;
 
-  /* gdb init files.  */
-  const char *system_gdbinit;
-  const char *home_gdbinit;
-  const char *local_gdbinit;
-
   int i;
   int save_auto_load;
   int ret = 1;
@@ -908,6 +903,7 @@ captured_main_1 (struct captured_main_args *context)
   /* Lookup gdbinit files.  Note that the gdbinit file name may be
      overriden during file initialization, so get_init_files should be
      called after gdb_init.  */
+  std::string system_gdbinit, home_gdbinit, local_gdbinit;
   get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
 
   /* Do these (and anything which might call wrap_here or *_filtered)
@@ -984,16 +980,16 @@ captured_main_1 (struct captured_main_args *context)
      This is done *before* all the command line arguments are
      processed; it sets global parameters, which are independent of
      what file you are debugging or what directory you are in.  */
-  if (system_gdbinit && !inhibit_gdbinit)
-    ret = catch_command_errors (source_script, system_gdbinit, 0);
+  if (!system_gdbinit.empty () && !inhibit_gdbinit)
+    ret = catch_command_errors (source_script, system_gdbinit.c_str (), 0);
 
   /* Read and execute $HOME/.gdbinit file, if it exists.  This is done
      *before* all the command line arguments are processed; it sets
      global parameters, which are independent of what file you are
      debugging or what directory you are in.  */
 
-  if (home_gdbinit && !inhibit_gdbinit && !inhibit_home_gdbinit)
-    ret = catch_command_errors (source_script, home_gdbinit, 0);
+  if (!home_gdbinit.empty () && !inhibit_gdbinit && !inhibit_home_gdbinit)
+    ret = catch_command_errors (source_script, home_gdbinit.c_str (), 0);
 
   /* Process '-ix' and '-iex' options early.  */
   for (i = 0; i < cmdarg_vec.size (); i++)
@@ -1096,20 +1092,20 @@ captured_main_1 (struct captured_main_args *context)
 
   /* Read the .gdbinit file in the current directory, *if* it isn't
      the same as the $HOME/.gdbinit file (it should exist, also).  */
-  if (local_gdbinit)
+  if (!local_gdbinit.empty ())
     {
       auto_load_local_gdbinit_pathname
-	= gdb_realpath (local_gdbinit).release ();
+	= gdb_realpath (local_gdbinit.c_str ()).release ();
 
       if (!inhibit_gdbinit && auto_load_local_gdbinit
-	  && file_is_auto_load_safe (local_gdbinit,
+	  && file_is_auto_load_safe (local_gdbinit.c_str (),
 				     _("auto-load: Loading .gdbinit "
 				       "file \"%s\".\n"),
-				     local_gdbinit))
+				     local_gdbinit.c_str ()))
 	{
 	  auto_load_local_gdbinit_loaded = 1;
 
-	  ret = catch_command_errors (source_script, local_gdbinit, 0);
+	  ret = catch_command_errors (source_script, local_gdbinit.c_str (), 0);
 	}
     }
 
@@ -1203,9 +1199,9 @@ gdb_main (struct captured_main_args *args)
 static void
 print_gdb_help (struct ui_file *stream)
 {
-  const char *system_gdbinit;
-  const char *home_gdbinit;
-  const char *local_gdbinit;
+  std::string system_gdbinit;
+  std::string home_gdbinit;
+  std::string local_gdbinit;
 
   get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
 
@@ -1283,18 +1279,18 @@ Other options:\n\n\
   fputs_unfiltered (_("\n\
 At startup, GDB reads the following init files and executes their commands:\n\
 "), stream);
-  if (system_gdbinit)
+  if (!system_gdbinit.empty ())
     fprintf_unfiltered (stream, _("\
    * system-wide init file: %s\n\
-"), system_gdbinit);
-  if (home_gdbinit)
+"), system_gdbinit.c_str ());
+  if (!home_gdbinit.empty ())
     fprintf_unfiltered (stream, _("\
    * user-specific init file: %s\n\
-"), home_gdbinit);
-  if (local_gdbinit)
+"), home_gdbinit.c_str ());
+  if (!local_gdbinit.empty ())
     fprintf_unfiltered (stream, _("\
    * local init file (see also 'set auto-load local-gdbinit'): ./%s\n\
-"), local_gdbinit);
+"), local_gdbinit.c_str ());
   fputs_unfiltered (_("\n\
 For more information, type \"help\" from within GDB, or consult the\n\
 GDB manual (available as on-line info or a printed manual).\n\
-- 
2.23.0.rc1.153.gdeed80330f-goog

Re: [PATCH 1/3] Refactor get_init_files to use std::string

[Sergio Durigan Junior]

On Tuesday, August 20 2019, Christian Biesinger via gdb-patches wrote:

> To avoid manual memory management.

Thanks!  Comments below.

> Tested on buildbot.
>
> gdb/ChangeLog:
>
> 2019-08-20  Christian Biesinger  <cbiesinger@google.com>
>
> 	* main.c (get_init_files): Change to use std::string.
> 	(captured_main_1): Update.
> 	(print_gdb_help): Update.
> ---
>  gdb/main.c | 96 ++++++++++++++++++++++++++----------------------------
>  1 file changed, 46 insertions(+), 50 deletions(-)
>
> diff --git a/gdb/main.c b/gdb/main.c
> index 678c413021..b9e12589ab 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -195,27 +195,26 @@ relocate_gdb_directory (const char *initial, int flag)
>     return them in SYSTEM_GDBINIT, HOME_GDBINIT, LOCAL_GDBINIT.  If
>     there is no system gdbinit (resp. home gdbinit and local gdbinit)
>     to be loaded, then SYSTEM_GDBINIT (resp. HOME_GDBINIT and
> -   LOCAL_GDBINIT) is set to NULL.  */
> +   LOCAL_GDBINIT) is set to the empty string.  */
>  static void
> -get_init_files (const char **system_gdbinit,
> -		const char **home_gdbinit,
> -		const char **local_gdbinit)
> +get_init_files (std::string *system_gdbinit,
> +		std::string *home_gdbinit,
> +		std::string *local_gdbinit)
>  {
> -  static const char *sysgdbinit = NULL;
> -  static char *homeinit = NULL;
> -  static const char *localinit = NULL;
> +  static std::string sysgdbinit;
> +  static std::string homeinit;
> +  static std::string localinit;
>    static int initialized = 0;
>  
>    if (!initialized)
>      {
>        struct stat homebuf, cwdbuf, s;
> -      const char *homedir;
>  
>        if (SYSTEM_GDBINIT[0])
>  	{
>  	  int datadir_len = strlen (GDB_DATADIR);
>  	  int sys_gdbinit_len = strlen (SYSTEM_GDBINIT);

Since you're cleaning things up, I wouldn't mind converting these two
variables to 'size_t' :-).

> -	  char *relocated_sysgdbinit;
> +	  std::string relocated_sysgdbinit;
>  
>  	  /* If SYSTEM_GDBINIT lives in data-directory, and data-directory
>  	     has been provided, search for SYSTEM_GDBINIT there.  */
> @@ -226,28 +225,30 @@ get_init_files (const char **system_gdbinit,
>  	    {
>  	      /* Append the part of SYSTEM_GDBINIT that follows GDB_DATADIR
>  		 to gdb_datadir.  */
> -	      char *tmp_sys_gdbinit = xstrdup (&SYSTEM_GDBINIT[datadir_len]);
> -	      char *p;
>  
> -	      for (p = tmp_sys_gdbinit; IS_DIR_SEPARATOR (*p); ++p)
> +	      size_t start = datadir_len;
> +	      for (; IS_DIR_SEPARATOR (SYSTEM_GDBINIT[start]); ++start)
>  		continue;

This seems wrong; you're starting the iteration from
'SYSTEM_GDBINIT[start]', but 'start' is 'datadir_len'.

> -	      relocated_sysgdbinit = concat (gdb_datadir, SLASH_STRING, p,
> -					     (char *) NULL);
> -	      xfree (tmp_sys_gdbinit);
> +	      relocated_sysgdbinit = std::string (gdb_datadir) + SLASH_STRING +
> +		&SYSTEM_GDBINIT[start];
>  	    }
>  	  else
>  	    {
> -	      relocated_sysgdbinit = relocate_path (gdb_program_name,
> -						    SYSTEM_GDBINIT,
> -						    SYSTEM_GDBINIT_RELOCATABLE);
> +	      char *relocated = relocate_path (gdb_program_name,
> +					       SYSTEM_GDBINIT,
> +					       SYSTEM_GDBINIT_RELOCATABLE);
> +	      if (relocated != nullptr)
> +	        {
> +		  relocated_sysgdbinit = relocated;
> +		  xfree (relocated);
> +		}
>  	    }
> -	  if (relocated_sysgdbinit && stat (relocated_sysgdbinit, &s) == 0)
> +	  if (!relocated_sysgdbinit.empty () &&
> +	      stat (relocated_sysgdbinit.c_str (), &s) == 0)
>  	    sysgdbinit = relocated_sysgdbinit;
> -	  else
> -	    xfree (relocated_sysgdbinit);
>  	}
>  
> -      homedir = getenv ("HOME");
> +      const char *homedir = getenv ("HOME");
>  
>        /* If the .gdbinit file in the current directory is the same as
>  	 the $HOME/.gdbinit file, it should not be sourced.  homebuf
> @@ -260,17 +261,16 @@ get_init_files (const char **system_gdbinit,
>  
>        if (homedir)
>  	{
> -	  homeinit = xstrprintf ("%s/%s", homedir, GDBINIT);
> -	  if (stat (homeinit, &homebuf) != 0)
> +	  homeinit = std::string (homedir) + "/" + GDBINIT;

I know the old code did that already, and I also know that SLASH_STRING
is always defined as "/", but I think we should use it here, instead of
hard coding the "/".

> +	  if (stat (homeinit.c_str (), &homebuf) != 0)
>  	    {
> -	      xfree (homeinit);
> -	      homeinit = NULL;
> +	      homeinit = "";
>  	    }
>  	}
>  
>        if (stat (GDBINIT, &cwdbuf) == 0)
>  	{
> -	  if (!homeinit
> +	  if (homeinit.empty ()
>  	      || memcmp ((char *) &homebuf, (char *) &cwdbuf,
>  			 sizeof (struct stat)))
>  	    localinit = GDBINIT;
> @@ -470,11 +470,6 @@ captured_main_1 (struct captured_main_args *context)
>    /* All arguments of --directory option.  */
>    std::vector<char *> dirarg;
>  
> -  /* gdb init files.  */
> -  const char *system_gdbinit;
> -  const char *home_gdbinit;
> -  const char *local_gdbinit;
> -
>    int i;
>    int save_auto_load;
>    int ret = 1;
> @@ -908,6 +903,7 @@ captured_main_1 (struct captured_main_args *context)
>    /* Lookup gdbinit files.  Note that the gdbinit file name may be
>       overriden during file initialization, so get_init_files should be
>       called after gdb_init.  */
> +  std::string system_gdbinit, home_gdbinit, local_gdbinit;

I'd prefer if you kept each variable declared separately, like it was
before:

  std::string system_gdbinit;
  std::string home_gdbinit;
  std::string local_gdbinit;

IIRC, we discourage declaring many variables in the same line.

>    get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
>  
>    /* Do these (and anything which might call wrap_here or *_filtered)
> @@ -984,16 +980,16 @@ captured_main_1 (struct captured_main_args *context)
>       This is done *before* all the command line arguments are
>       processed; it sets global parameters, which are independent of
>       what file you are debugging or what directory you are in.  */
> -  if (system_gdbinit && !inhibit_gdbinit)
> -    ret = catch_command_errors (source_script, system_gdbinit, 0);
> +  if (!system_gdbinit.empty () && !inhibit_gdbinit)
> +    ret = catch_command_errors (source_script, system_gdbinit.c_str (), 0);
>  
>    /* Read and execute $HOME/.gdbinit file, if it exists.  This is done
>       *before* all the command line arguments are processed; it sets
>       global parameters, which are independent of what file you are
>       debugging or what directory you are in.  */
>  
> -  if (home_gdbinit && !inhibit_gdbinit && !inhibit_home_gdbinit)
> -    ret = catch_command_errors (source_script, home_gdbinit, 0);
> +  if (!home_gdbinit.empty () && !inhibit_gdbinit && !inhibit_home_gdbinit)
> +    ret = catch_command_errors (source_script, home_gdbinit.c_str (), 0);
>  
>    /* Process '-ix' and '-iex' options early.  */
>    for (i = 0; i < cmdarg_vec.size (); i++)
> @@ -1096,20 +1092,20 @@ captured_main_1 (struct captured_main_args *context)
>  
>    /* Read the .gdbinit file in the current directory, *if* it isn't
>       the same as the $HOME/.gdbinit file (it should exist, also).  */
> -  if (local_gdbinit)
> +  if (!local_gdbinit.empty ())
>      {
>        auto_load_local_gdbinit_pathname
> -	= gdb_realpath (local_gdbinit).release ();
> +	= gdb_realpath (local_gdbinit.c_str ()).release ();
>  
>        if (!inhibit_gdbinit && auto_load_local_gdbinit
> -	  && file_is_auto_load_safe (local_gdbinit,
> +	  && file_is_auto_load_safe (local_gdbinit.c_str (),
>  				     _("auto-load: Loading .gdbinit "
>  				       "file \"%s\".\n"),
> -				     local_gdbinit))
> +				     local_gdbinit.c_str ()))
>  	{
>  	  auto_load_local_gdbinit_loaded = 1;
>  
> -	  ret = catch_command_errors (source_script, local_gdbinit, 0);
> +	  ret = catch_command_errors (source_script, local_gdbinit.c_str (), 0);
>  	}
>      }
>  
> @@ -1203,9 +1199,9 @@ gdb_main (struct captured_main_args *args)
>  static void
>  print_gdb_help (struct ui_file *stream)
>  {
> -  const char *system_gdbinit;
> -  const char *home_gdbinit;
> -  const char *local_gdbinit;
> +  std::string system_gdbinit;
> +  std::string home_gdbinit;
> +  std::string local_gdbinit;
>  
>    get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
>  
> @@ -1283,18 +1279,18 @@ Other options:\n\n\
>    fputs_unfiltered (_("\n\
>  At startup, GDB reads the following init files and executes their commands:\n\
>  "), stream);
> -  if (system_gdbinit)
> +  if (!system_gdbinit.empty ())
>      fprintf_unfiltered (stream, _("\
>     * system-wide init file: %s\n\
> -"), system_gdbinit);
> -  if (home_gdbinit)
> +"), system_gdbinit.c_str ());
> +  if (!home_gdbinit.empty ())
>      fprintf_unfiltered (stream, _("\
>     * user-specific init file: %s\n\
> -"), home_gdbinit);
> -  if (local_gdbinit)
> +"), home_gdbinit.c_str ());
> +  if (!local_gdbinit.empty ())
>      fprintf_unfiltered (stream, _("\
>     * local init file (see also 'set auto-load local-gdbinit'): ./%s\n\
> -"), local_gdbinit);
> +"), local_gdbinit.c_str ());
>    fputs_unfiltered (_("\n\
>  For more information, type \"help\" from within GDB, or consult the\n\
>  GDB manual (available as on-line info or a printed manual).\n\
> -- 
> 2.23.0.rc1.153.gdeed80330f-goog

Otherwise, LGTM.  Thanks for doing this!

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH 1/3] Refactor get_init_files to use std::string

[Christian Biesinger via gdb-patches]

I will send an updated version in a moment. More comments below.

On Wed, Aug 21, 2019 at 12:13 PM Sergio Durigan Junior
<sergiodj@redhat.com> wrote:
>
> On Tuesday, August 20 2019, Christian Biesinger via gdb-patches wrote:
>
> > To avoid manual memory management.
>
> Thanks!  Comments below.
>
> > Tested on buildbot.
> >
> > gdb/ChangeLog:
> >
> > 2019-08-20  Christian Biesinger  <cbiesinger@google.com>
> >
> >       * main.c (get_init_files): Change to use std::string.
> >       (captured_main_1): Update.
> >       (print_gdb_help): Update.
> > ---
> >  gdb/main.c | 96 ++++++++++++++++++++++++++----------------------------
> >  1 file changed, 46 insertions(+), 50 deletions(-)
> >
> > diff --git a/gdb/main.c b/gdb/main.c
> > index 678c413021..b9e12589ab 100644
> > --- a/gdb/main.c
> > +++ b/gdb/main.c
> > @@ -195,27 +195,26 @@ relocate_gdb_directory (const char *initial, int flag)
> >     return them in SYSTEM_GDBINIT, HOME_GDBINIT, LOCAL_GDBINIT.  If
> >     there is no system gdbinit (resp. home gdbinit and local gdbinit)
> >     to be loaded, then SYSTEM_GDBINIT (resp. HOME_GDBINIT and
> > -   LOCAL_GDBINIT) is set to NULL.  */
> > +   LOCAL_GDBINIT) is set to the empty string.  */
> >  static void
> > -get_init_files (const char **system_gdbinit,
> > -             const char **home_gdbinit,
> > -             const char **local_gdbinit)
> > +get_init_files (std::string *system_gdbinit,
> > +             std::string *home_gdbinit,
> > +             std::string *local_gdbinit)
> >  {
> > -  static const char *sysgdbinit = NULL;
> > -  static char *homeinit = NULL;
> > -  static const char *localinit = NULL;
> > +  static std::string sysgdbinit;
> > +  static std::string homeinit;
> > +  static std::string localinit;
> >    static int initialized = 0;
> >
> >    if (!initialized)
> >      {
> >        struct stat homebuf, cwdbuf, s;
> > -      const char *homedir;
> >
> >        if (SYSTEM_GDBINIT[0])
> >       {
> >         int datadir_len = strlen (GDB_DATADIR);
> >         int sys_gdbinit_len = strlen (SYSTEM_GDBINIT);
>
> Since you're cleaning things up, I wouldn't mind converting these two
> variables to 'size_t' :-).

Will do.

> > -       char *relocated_sysgdbinit;
> > +       std::string relocated_sysgdbinit;
> >
> >         /* If SYSTEM_GDBINIT lives in data-directory, and data-directory
> >            has been provided, search for SYSTEM_GDBINIT there.  */
> > @@ -226,28 +225,30 @@ get_init_files (const char **system_gdbinit,
> >           {
> >             /* Append the part of SYSTEM_GDBINIT that follows GDB_DATADIR
> >                to gdb_datadir.  */
> > -           char *tmp_sys_gdbinit = xstrdup (&SYSTEM_GDBINIT[datadir_len]);
> > -           char *p;
> >
> > -           for (p = tmp_sys_gdbinit; IS_DIR_SEPARATOR (*p); ++p)
> > +           size_t start = datadir_len;
> > +           for (; IS_DIR_SEPARATOR (SYSTEM_GDBINIT[start]); ++start)
> >               continue;
>
> This seems wrong; you're starting the iteration from
> 'SYSTEM_GDBINIT[start]', but 'start' is 'datadir_len'.

The previous version first initialized tmp_sys_gdbinit to start from
&SYSTEM_GDBINIT[datadir_len], so I think my change is correct (and
simpler)?

> > -           relocated_sysgdbinit = concat (gdb_datadir, SLASH_STRING, p,
> > -                                          (char *) NULL);
> > -           xfree (tmp_sys_gdbinit);
> > +           relocated_sysgdbinit = std::string (gdb_datadir) + SLASH_STRING +
> > +             &SYSTEM_GDBINIT[start];
> >           }
> >         else
> >           {
> > -           relocated_sysgdbinit = relocate_path (gdb_program_name,
> > -                                                 SYSTEM_GDBINIT,
> > -                                                 SYSTEM_GDBINIT_RELOCATABLE);
> > +           char *relocated = relocate_path (gdb_program_name,
> > +                                            SYSTEM_GDBINIT,
> > +                                            SYSTEM_GDBINIT_RELOCATABLE);
> > +           if (relocated != nullptr)
> > +             {
> > +               relocated_sysgdbinit = relocated;
> > +               xfree (relocated);
> > +             }
> >           }
> > -       if (relocated_sysgdbinit && stat (relocated_sysgdbinit, &s) == 0)
> > +       if (!relocated_sysgdbinit.empty () &&
> > +           stat (relocated_sysgdbinit.c_str (), &s) == 0)
> >           sysgdbinit = relocated_sysgdbinit;
> > -       else
> > -         xfree (relocated_sysgdbinit);
> >       }
> >
> > -      homedir = getenv ("HOME");
> > +      const char *homedir = getenv ("HOME");
> >
> >        /* If the .gdbinit file in the current directory is the same as
> >        the $HOME/.gdbinit file, it should not be sourced.  homebuf
> > @@ -260,17 +261,16 @@ get_init_files (const char **system_gdbinit,
> >
> >        if (homedir)
> >       {
> > -       homeinit = xstrprintf ("%s/%s", homedir, GDBINIT);
> > -       if (stat (homeinit, &homebuf) != 0)
> > +       homeinit = std::string (homedir) + "/" + GDBINIT;
>
> I know the old code did that already, and I also know that SLASH_STRING
> is always defined as "/", but I think we should use it here, instead of
> hard coding the "/".

Will do. (I didn't know that SLASH_STRING is always "/", I assumed it
would be "\" on Windows....)

> > +       if (stat (homeinit.c_str (), &homebuf) != 0)
> >           {
> > -           xfree (homeinit);
> > -           homeinit = NULL;
> > +           homeinit = "";
> >           }
> >       }
> >
> >        if (stat (GDBINIT, &cwdbuf) == 0)
> >       {
> > -       if (!homeinit
> > +       if (homeinit.empty ()
> >             || memcmp ((char *) &homebuf, (char *) &cwdbuf,
> >                        sizeof (struct stat)))
> >           localinit = GDBINIT;
> > @@ -470,11 +470,6 @@ captured_main_1 (struct captured_main_args *context)
> >    /* All arguments of --directory option.  */
> >    std::vector<char *> dirarg;
> >
> > -  /* gdb init files.  */
> > -  const char *system_gdbinit;
> > -  const char *home_gdbinit;
> > -  const char *local_gdbinit;
> > -
> >    int i;
> >    int save_auto_load;
> >    int ret = 1;
> > @@ -908,6 +903,7 @@ captured_main_1 (struct captured_main_args *context)
> >    /* Lookup gdbinit files.  Note that the gdbinit file name may be
> >       overriden during file initialization, so get_init_files should be
> >       called after gdb_init.  */
> > +  std::string system_gdbinit, home_gdbinit, local_gdbinit;
>
> I'd prefer if you kept each variable declared separately, like it was
> before:
>
>   std::string system_gdbinit;
>   std::string home_gdbinit;
>   std::string local_gdbinit;
>
> IIRC, we discourage declaring many variables in the same line.

Will do.

> >    get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
> >
> >    /* Do these (and anything which might call wrap_here or *_filtered)
> > @@ -984,16 +980,16 @@ captured_main_1 (struct captured_main_args *context)
> >       This is done *before* all the command line arguments are
> >       processed; it sets global parameters, which are independent of
> >       what file you are debugging or what directory you are in.  */
> > -  if (system_gdbinit && !inhibit_gdbinit)
> > -    ret = catch_command_errors (source_script, system_gdbinit, 0);
> > +  if (!system_gdbinit.empty () && !inhibit_gdbinit)
> > +    ret = catch_command_errors (source_script, system_gdbinit.c_str (), 0);
> >
> >    /* Read and execute $HOME/.gdbinit file, if it exists.  This is done
> >       *before* all the command line arguments are processed; it sets
> >       global parameters, which are independent of what file you are
> >       debugging or what directory you are in.  */
> >
> > -  if (home_gdbinit && !inhibit_gdbinit && !inhibit_home_gdbinit)
> > -    ret = catch_command_errors (source_script, home_gdbinit, 0);
> > +  if (!home_gdbinit.empty () && !inhibit_gdbinit && !inhibit_home_gdbinit)
> > +    ret = catch_command_errors (source_script, home_gdbinit.c_str (), 0);
> >
> >    /* Process '-ix' and '-iex' options early.  */
> >    for (i = 0; i < cmdarg_vec.size (); i++)
> > @@ -1096,20 +1092,20 @@ captured_main_1 (struct captured_main_args *context)
> >
> >    /* Read the .gdbinit file in the current directory, *if* it isn't
> >       the same as the $HOME/.gdbinit file (it should exist, also).  */
> > -  if (local_gdbinit)
> > +  if (!local_gdbinit.empty ())
> >      {
> >        auto_load_local_gdbinit_pathname
> > -     = gdb_realpath (local_gdbinit).release ();
> > +     = gdb_realpath (local_gdbinit.c_str ()).release ();
> >
> >        if (!inhibit_gdbinit && auto_load_local_gdbinit
> > -       && file_is_auto_load_safe (local_gdbinit,
> > +       && file_is_auto_load_safe (local_gdbinit.c_str (),
> >                                    _("auto-load: Loading .gdbinit "
> >                                      "file \"%s\".\n"),
> > -                                  local_gdbinit))
> > +                                  local_gdbinit.c_str ()))
> >       {
> >         auto_load_local_gdbinit_loaded = 1;
> >
> > -       ret = catch_command_errors (source_script, local_gdbinit, 0);
> > +       ret = catch_command_errors (source_script, local_gdbinit.c_str (), 0);
> >       }
> >      }
> >
> > @@ -1203,9 +1199,9 @@ gdb_main (struct captured_main_args *args)
> >  static void
> >  print_gdb_help (struct ui_file *stream)
> >  {
> > -  const char *system_gdbinit;
> > -  const char *home_gdbinit;
> > -  const char *local_gdbinit;
> > +  std::string system_gdbinit;
> > +  std::string home_gdbinit;
> > +  std::string local_gdbinit;
> >
> >    get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
> >
> > @@ -1283,18 +1279,18 @@ Other options:\n\n\
> >    fputs_unfiltered (_("\n\
> >  At startup, GDB reads the following init files and executes their commands:\n\
> >  "), stream);
> > -  if (system_gdbinit)
> > +  if (!system_gdbinit.empty ())
> >      fprintf_unfiltered (stream, _("\
> >     * system-wide init file: %s\n\
> > -"), system_gdbinit);
> > -  if (home_gdbinit)
> > +"), system_gdbinit.c_str ());
> > +  if (!home_gdbinit.empty ())
> >      fprintf_unfiltered (stream, _("\
> >     * user-specific init file: %s\n\
> > -"), home_gdbinit);
> > -  if (local_gdbinit)
> > +"), home_gdbinit.c_str ());
> > +  if (!local_gdbinit.empty ())
> >      fprintf_unfiltered (stream, _("\
> >     * local init file (see also 'set auto-load local-gdbinit'): ./%s\n\
> > -"), local_gdbinit);
> > +"), local_gdbinit.c_str ());
> >    fputs_unfiltered (_("\n\
> >  For more information, type \"help\" from within GDB, or consult the\n\
> >  GDB manual (available as on-line info or a printed manual).\n\
> > --
> > 2.23.0.rc1.153.gdeed80330f-goog
>
> Otherwise, LGTM.  Thanks for doing this!

Thanks for the review!

Christian

[PATCH 1/3 v2] Refactor get_init_files to use std::string

[Christian Biesinger via gdb-patches]

To avoid manual memory management.

Tested on buildbot.

gdb/ChangeLog:

2019-08-20  Christian Biesinger  <cbiesinger@google.com>

	* main.c (get_init_files): Change to use std::string.
	(captured_main_1): Update.
	(print_gdb_help): Update.
---
 gdb/main.c | 102 ++++++++++++++++++++++++++---------------------------
 1 file changed, 50 insertions(+), 52 deletions(-)

diff --git a/gdb/main.c b/gdb/main.c
index 678c413021..724b2928a7 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -195,27 +195,26 @@ relocate_gdb_directory (const char *initial, int flag)
    return them in SYSTEM_GDBINIT, HOME_GDBINIT, LOCAL_GDBINIT.  If
    there is no system gdbinit (resp. home gdbinit and local gdbinit)
    to be loaded, then SYSTEM_GDBINIT (resp. HOME_GDBINIT and
-   LOCAL_GDBINIT) is set to NULL.  */
+   LOCAL_GDBINIT) is set to the empty string.  */
 static void
-get_init_files (const char **system_gdbinit,
-		const char **home_gdbinit,
-		const char **local_gdbinit)
+get_init_files (std::string *system_gdbinit,
+		std::string *home_gdbinit,
+		std::string *local_gdbinit)
 {
-  static const char *sysgdbinit = NULL;
-  static char *homeinit = NULL;
-  static const char *localinit = NULL;
+  static std::string sysgdbinit;
+  static std::string homeinit;
+  static std::string localinit;
   static int initialized = 0;
 
   if (!initialized)
     {
       struct stat homebuf, cwdbuf, s;
-      const char *homedir;
 
       if (SYSTEM_GDBINIT[0])
 	{
-	  int datadir_len = strlen (GDB_DATADIR);
-	  int sys_gdbinit_len = strlen (SYSTEM_GDBINIT);
-	  char *relocated_sysgdbinit;
+	  size_t datadir_len = strlen (GDB_DATADIR);
+	  size_t sys_gdbinit_len = strlen (SYSTEM_GDBINIT);
+	  std::string relocated_sysgdbinit;
 
 	  /* If SYSTEM_GDBINIT lives in data-directory, and data-directory
 	     has been provided, search for SYSTEM_GDBINIT there.  */
@@ -226,28 +225,30 @@ get_init_files (const char **system_gdbinit,
 	    {
 	      /* Append the part of SYSTEM_GDBINIT that follows GDB_DATADIR
 		 to gdb_datadir.  */
-	      char *tmp_sys_gdbinit = xstrdup (&SYSTEM_GDBINIT[datadir_len]);
-	      char *p;
 
-	      for (p = tmp_sys_gdbinit; IS_DIR_SEPARATOR (*p); ++p)
+	      size_t start = datadir_len;
+	      for (; IS_DIR_SEPARATOR (SYSTEM_GDBINIT[start]); ++start)
 		continue;
-	      relocated_sysgdbinit = concat (gdb_datadir, SLASH_STRING, p,
-					     (char *) NULL);
-	      xfree (tmp_sys_gdbinit);
+	      relocated_sysgdbinit = std::string (gdb_datadir) + SLASH_STRING +
+		&SYSTEM_GDBINIT[start];
 	    }
 	  else
 	    {
-	      relocated_sysgdbinit = relocate_path (gdb_program_name,
-						    SYSTEM_GDBINIT,
-						    SYSTEM_GDBINIT_RELOCATABLE);
+	      char *relocated = relocate_path (gdb_program_name,
+					       SYSTEM_GDBINIT,
+					       SYSTEM_GDBINIT_RELOCATABLE);
+	      if (relocated != nullptr)
+	        {
+		  relocated_sysgdbinit = relocated;
+		  xfree (relocated);
+		}
 	    }
-	  if (relocated_sysgdbinit && stat (relocated_sysgdbinit, &s) == 0)
+	  if (!relocated_sysgdbinit.empty () &&
+	      stat (relocated_sysgdbinit.c_str (), &s) == 0)
 	    sysgdbinit = relocated_sysgdbinit;
-	  else
-	    xfree (relocated_sysgdbinit);
 	}
 
-      homedir = getenv ("HOME");
+      const char *homedir = getenv ("HOME");
 
       /* If the .gdbinit file in the current directory is the same as
 	 the $HOME/.gdbinit file, it should not be sourced.  homebuf
@@ -260,17 +261,16 @@ get_init_files (const char **system_gdbinit,
 
       if (homedir)
 	{
-	  homeinit = xstrprintf ("%s/%s", homedir, GDBINIT);
-	  if (stat (homeinit, &homebuf) != 0)
+	  homeinit = std::string (homedir) + SLASH_STRING + GDBINIT;
+	  if (stat (homeinit.c_str (), &homebuf) != 0)
 	    {
-	      xfree (homeinit);
-	      homeinit = NULL;
+	      homeinit = "";
 	    }
 	}
 
       if (stat (GDBINIT, &cwdbuf) == 0)
 	{
-	  if (!homeinit
+	  if (homeinit.empty ()
 	      || memcmp ((char *) &homebuf, (char *) &cwdbuf,
 			 sizeof (struct stat)))
 	    localinit = GDBINIT;
@@ -470,11 +470,6 @@ captured_main_1 (struct captured_main_args *context)
   /* All arguments of --directory option.  */
   std::vector<char *> dirarg;
 
-  /* gdb init files.  */
-  const char *system_gdbinit;
-  const char *home_gdbinit;
-  const char *local_gdbinit;
-
   int i;
   int save_auto_load;
   int ret = 1;
@@ -908,6 +903,9 @@ captured_main_1 (struct captured_main_args *context)
   /* Lookup gdbinit files.  Note that the gdbinit file name may be
      overriden during file initialization, so get_init_files should be
      called after gdb_init.  */
+  std::string system_gdbinit;
+  std::string home_gdbinit;
+  std::string local_gdbinit;
   get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
 
   /* Do these (and anything which might call wrap_here or *_filtered)
@@ -984,16 +982,16 @@ captured_main_1 (struct captured_main_args *context)
      This is done *before* all the command line arguments are
      processed; it sets global parameters, which are independent of
      what file you are debugging or what directory you are in.  */
-  if (system_gdbinit && !inhibit_gdbinit)
-    ret = catch_command_errors (source_script, system_gdbinit, 0);
+  if (!system_gdbinit.empty () && !inhibit_gdbinit)
+    ret = catch_command_errors (source_script, system_gdbinit.c_str (), 0);
 
   /* Read and execute $HOME/.gdbinit file, if it exists.  This is done
      *before* all the command line arguments are processed; it sets
      global parameters, which are independent of what file you are
      debugging or what directory you are in.  */
 
-  if (home_gdbinit && !inhibit_gdbinit && !inhibit_home_gdbinit)
-    ret = catch_command_errors (source_script, home_gdbinit, 0);
+  if (!home_gdbinit.empty () && !inhibit_gdbinit && !inhibit_home_gdbinit)
+    ret = catch_command_errors (source_script, home_gdbinit.c_str (), 0);
 
   /* Process '-ix' and '-iex' options early.  */
   for (i = 0; i < cmdarg_vec.size (); i++)
@@ -1096,20 +1094,20 @@ captured_main_1 (struct captured_main_args *context)
 
   /* Read the .gdbinit file in the current directory, *if* it isn't
      the same as the $HOME/.gdbinit file (it should exist, also).  */
-  if (local_gdbinit)
+  if (!local_gdbinit.empty ())
     {
       auto_load_local_gdbinit_pathname
-	= gdb_realpath (local_gdbinit).release ();
+	= gdb_realpath (local_gdbinit.c_str ()).release ();
 
       if (!inhibit_gdbinit && auto_load_local_gdbinit
-	  && file_is_auto_load_safe (local_gdbinit,
+	  && file_is_auto_load_safe (local_gdbinit.c_str (),
 				     _("auto-load: Loading .gdbinit "
 				       "file \"%s\".\n"),
-				     local_gdbinit))
+				     local_gdbinit.c_str ()))
 	{
 	  auto_load_local_gdbinit_loaded = 1;
 
-	  ret = catch_command_errors (source_script, local_gdbinit, 0);
+	  ret = catch_command_errors (source_script, local_gdbinit.c_str (), 0);
 	}
     }
 
@@ -1203,9 +1201,9 @@ gdb_main (struct captured_main_args *args)
 static void
 print_gdb_help (struct ui_file *stream)
 {
-  const char *system_gdbinit;
-  const char *home_gdbinit;
-  const char *local_gdbinit;
+  std::string system_gdbinit;
+  std::string home_gdbinit;
+  std::string local_gdbinit;
 
   get_init_files (&system_gdbinit, &home_gdbinit, &local_gdbinit);
 
@@ -1283,18 +1281,18 @@ Other options:\n\n\
   fputs_unfiltered (_("\n\
 At startup, GDB reads the following init files and executes their commands:\n\
 "), stream);
-  if (system_gdbinit)
+  if (!system_gdbinit.empty ())
     fprintf_unfiltered (stream, _("\
    * system-wide init file: %s\n\
-"), system_gdbinit);
-  if (home_gdbinit)
+"), system_gdbinit.c_str ());
+  if (!home_gdbinit.empty ())
     fprintf_unfiltered (stream, _("\
    * user-specific init file: %s\n\
-"), home_gdbinit);
-  if (local_gdbinit)
+"), home_gdbinit.c_str ());
+  if (!local_gdbinit.empty ())
     fprintf_unfiltered (stream, _("\
    * local init file (see also 'set auto-load local-gdbinit'): ./%s\n\
-"), local_gdbinit);
+"), local_gdbinit.c_str ());
   fputs_unfiltered (_("\n\
 For more information, type \"help\" from within GDB, or consult the\n\
 GDB manual (available as on-line info or a printed manual).\n\
-- 
2.23.0.rc1.153.gdeed80330f-goog

Re: [PATCH 1/3] Refactor get_init_files to use std::string

[Sergio Durigan Junior]

On Wednesday, August 21 2019, Christian Biesinger wrote:

>> > -       char *relocated_sysgdbinit;
>> > +       std::string relocated_sysgdbinit;
>> >
>> >         /* If SYSTEM_GDBINIT lives in data-directory, and data-directory
>> >            has been provided, search for SYSTEM_GDBINIT there.  */
>> > @@ -226,28 +225,30 @@ get_init_files (const char **system_gdbinit,
>> >           {
>> >             /* Append the part of SYSTEM_GDBINIT that follows GDB_DATADIR
>> >                to gdb_datadir.  */
>> > -           char *tmp_sys_gdbinit = xstrdup (&SYSTEM_GDBINIT[datadir_len]);
>> > -           char *p;
>> >
>> > -           for (p = tmp_sys_gdbinit; IS_DIR_SEPARATOR (*p); ++p)
>> > +           size_t start = datadir_len;
>> > +           for (; IS_DIR_SEPARATOR (SYSTEM_GDBINIT[start]); ++start)
>> >               continue;
>>
>> This seems wrong; you're starting the iteration from
>> 'SYSTEM_GDBINIT[start]', but 'start' is 'datadir_len'.
>
> The previous version first initialized tmp_sys_gdbinit to start from
> &SYSTEM_GDBINIT[datadir_len], so I think my change is correct (and
> simpler)?

Ah, right, now I see the '&SYSTEM_GDBINIT[datadir_len]' there.  That
looks fine, then.

Thanks!

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH 0/3] [RFC] Load gdbinit files from a directory

[Pedro Alves]

On 8/20/19 11:17 PM, Christian Biesinger via gdb-patches wrote:
> This patch series is some refactoring and then a patch to load gdbinit
> files from a directory, instead of only allowing a single file.
> 
> Fedora ships a system gdbinit file that does something similar; this
> does this by default and also works if Python is disabled.

Note that Fedora won't be able to replace the current mechanism with
this, because it also loads Python files from the dir:

 python
 import glob
 # glob.iglob is not available in python-2.4 (RHEL-5).
 for f in glob.glob('/etc/gdbinit.d/*.gdb'):
   gdb.execute('source %s' % f)
 for f in glob.glob('/etc/gdbinit.d/*.py'):
   gdb.execute('source %s' % f)
 end

So we'd need an additional "--with-system-python-scripts-dir"
for Python scripts or some such.

The advantage of Fedora's method IMO is that it's more flexible:
A distro or packager can decide to load gdb scripts from more than
one dir by default, e.g., from "~/gdbinit.d/", or to load gdb scripts and
python scripts from different dirs, etc.  It's similar to
/etc/bashrc loading scripts from /etc/profile.d/, etc. instead of bash
loading the scripts from a dir itself.  Of course the difference
here is that you can't walk directories with gdb's cli scripting.

Speaking of Python scripts, I guess Fedora's script should be
loading Guile scripts as well.

That isn't to say that I object to your patchset, TBC.  I just
see it a bit under the "why do it in C when you can script" light.
Of course the answer can reasonably be "I need this without Python".

Thanks,
Pedro Alves

Re: [PATCH 0/3] [RFC] Load gdbinit files from a directory

[Christian Biesinger via gdb-patches]

On Wed, Aug 21, 2019 at 1:13 PM Pedro Alves <palves@redhat.com> wrote:
> On 8/20/19 11:17 PM, Christian Biesinger via gdb-patches wrote:
> > This patch series is some refactoring and then a patch to load gdbinit
> > files from a directory, instead of only allowing a single file.
> >
> > Fedora ships a system gdbinit file that does something similar; this
> > does this by default and also works if Python is disabled.
>
> Note that Fedora won't be able to replace the current mechanism with
> this, because it also loads Python files from the dir:
>
>  python
>  import glob
>  # glob.iglob is not available in python-2.4 (RHEL-5).
>  for f in glob.glob('/etc/gdbinit.d/*.gdb'):
>    gdb.execute('source %s' % f)
>  for f in glob.glob('/etc/gdbinit.d/*.py'):
>    gdb.execute('source %s' % f)
>  end
>
> So we'd need an additional "--with-system-python-scripts-dir"
> for Python scripts or some such.

That's a good point. I don't think -with-system-python-scripts-dir
would really work for Fedora either since the directory there is
currently the same but I could change the patch to use
get_ext_lang_of_file/ext_lang_script_sourcer (which is effectively
pretty similar to the current Fedora gdbinit)

> The advantage of Fedora's method IMO is that it's more flexible:
> A distro or packager can decide to load gdb scripts from more than
> one dir by default, e.g., from "~/gdbinit.d/", or to load gdb scripts and
> python scripts from different dirs, etc.  It's similar to
> /etc/bashrc loading scripts from /etc/profile.d/, etc. instead of bash
> loading the scripts from a dir itself.  Of course the difference
> here is that you can't walk directories with gdb's cli scripting.

I guess it may be possible to add glob support to the source command.

> Speaking of Python scripts, I guess Fedora's script should be
> loading Guile scripts as well.

I agree. (Certainly if GDB shipped a default gdbinit like that, it
should do that)

> That isn't to say that I object to your patchset, TBC.  I just
> see it a bit under the "why do it in C when you can script" light.
> Of course the answer can reasonably be "I need this without Python".

So there were two reasons why I liked this approach:
- This ships something with GDB, that will work on all Linux
distributions the same way, as long as they agree on a directory
location
- It works without Python enabled

I'd also be happy with shipping a default gdbinit file that does a
similar thing, especially if we were to add glob support to the source
command.

I dunno, anyone else have thoughts on this?

Christian

Re: [PATCH 0/3] [RFC] Load gdbinit files from a directory

[Sergio Durigan Junior]

On Wednesday, August 21 2019, Christian Biesinger via gdb-patches wrote:

> On Wed, Aug 21, 2019 at 1:13 PM Pedro Alves <palves@redhat.com> wrote:
>> On 8/20/19 11:17 PM, Christian Biesinger via gdb-patches wrote:
>> > This patch series is some refactoring and then a patch to load gdbinit
>> > files from a directory, instead of only allowing a single file.
>> >
>> > Fedora ships a system gdbinit file that does something similar; this
>> > does this by default and also works if Python is disabled.
>>
>> Note that Fedora won't be able to replace the current mechanism with
>> this, because it also loads Python files from the dir:
>>
>>  python
>>  import glob
>>  # glob.iglob is not available in python-2.4 (RHEL-5).
>>  for f in glob.glob('/etc/gdbinit.d/*.gdb'):
>>    gdb.execute('source %s' % f)
>>  for f in glob.glob('/etc/gdbinit.d/*.py'):
>>    gdb.execute('source %s' % f)
>>  end
>>
>> So we'd need an additional "--with-system-python-scripts-dir"
>> for Python scripts or some such.
>
> That's a good point. I don't think -with-system-python-scripts-dir
> would really work for Fedora either since the directory there is
> currently the same but I could change the patch to use
> get_ext_lang_of_file/ext_lang_script_sourcer (which is effectively
> pretty similar to the current Fedora gdbinit)

I don't think we need to have separate directories for
Python/Guile/"native" GDB scripts.  I think we should consolidate all of
these scripts inside one directory.

>> Speaking of Python scripts, I guess Fedora's script should be
>> loading Guile scripts as well.

Guile support has been (temporarily?) disabled in Fedora GDB.

> I agree. (Certainly if GDB shipped a default gdbinit like that, it
> should do that)


>> That isn't to say that I object to your patchset, TBC.  I just
>> see it a bit under the "why do it in C when you can script" light.
>> Of course the answer can reasonably be "I need this without Python".
>
> So there were two reasons why I liked this approach:
> - This ships something with GDB, that will work on all Linux
> distributions the same way, as long as they agree on a directory
> location

I think /etc/gdbinit.d/ is a nice location, and I intend to keep using
it for Fedora GDB and to make Debian GDB use it as well.

I agree that supporting other directories, like ~/.gdbinit.d/, would be
great.

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH 0/3] [RFC] Load gdbinit files from a directory

[Christian Biesinger via gdb-patches]

On Wed, Aug 21, 2019 at 2:13 PM Pedro Alves <palves@redhat.com> wrote:
>
> On 8/20/19 11:17 PM, Christian Biesinger via gdb-patches wrote:
> > This patch series is some refactoring and then a patch to load gdbinit
> > files from a directory, instead of only allowing a single file.
> >
> > Fedora ships a system gdbinit file that does something similar; this
> > does this by default and also works if Python is disabled.
>
> Note that Fedora won't be able to replace the current mechanism with
> this, because it also loads Python files from the dir:

Hi Pedro,

I've looked at the code more closely now, and it already uses the
"source" command for loading the system gdbinit file(s). So, Fedora
should be able to use it, I think?

Christian

Re: [PATCH 0/3] [RFC] Load gdbinit files from a directory

[Pedro Alves]

On 8/25/19 11:23 PM, Christian Biesinger via gdb-patches wrote:
> On Wed, Aug 21, 2019 at 2:13 PM Pedro Alves <palves@redhat.com> wrote:
>>
>> On 8/20/19 11:17 PM, Christian Biesinger via gdb-patches wrote:
>>> This patch series is some refactoring and then a patch to load gdbinit
>>> files from a directory, instead of only allowing a single file.
>>>
>>> Fedora ships a system gdbinit file that does something similar; this
>>> does this by default and also works if Python is disabled.
>>
>> Note that Fedora won't be able to replace the current mechanism with
>> this, because it also loads Python files from the dir:
> 
> Hi Pedro,
> 
> I've looked at the code more closely now, and it already uses the
> "source" command for loading the system gdbinit file(s). So, Fedora
> should be able to use it, I think?

Are you sourcing every file in the directory irrespective of
filename / filename extension?  Not sure that's a good idea.

Thanks,
Pedro Alves

Re: [PATCH 0/3] [RFC] Load gdbinit files from a directory

[Christian Biesinger via gdb-patches]

On Mon, Aug 26, 2019 at 8:31 AM Pedro Alves <palves@redhat.com> wrote:
>
> On 8/25/19 11:23 PM, Christian Biesinger via gdb-patches wrote:
> > On Wed, Aug 21, 2019 at 2:13 PM Pedro Alves <palves@redhat.com> wrote:
> >>
> >> On 8/20/19 11:17 PM, Christian Biesinger via gdb-patches wrote:
> >>> This patch series is some refactoring and then a patch to load gdbinit
> >>> files from a directory, instead of only allowing a single file.
> >>>
> >>> Fedora ships a system gdbinit file that does something similar; this
> >>> does this by default and also works if Python is disabled.
> >>
> >> Note that Fedora won't be able to replace the current mechanism with
> >> this, because it also loads Python files from the dir:
> >
> > Hi Pedro,
> >
> > I've looked at the code more closely now, and it already uses the
> > "source" command for loading the system gdbinit file(s). So, Fedora
> > should be able to use it, I think?
>
> Are you sourcing every file in the directory irrespective of
> filename / filename extension?  Not sure that's a good idea.

I sent an updated version of the patch that limits files to *.gdb and
any other supported scripting languages (so .py/.scm right now)

Christian