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

Message ID 20190820221745.147370-2-cbiesinger@google.com
State New, archived
Headers

Commit Message

Terekhov, Mikhail via Gdb-patches Aug. 20, 2019, 10:17 p.m. UTC
  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(-)
  

Comments

Sergio Durigan Junior Aug. 21, 2019, 5:13 p.m. UTC | #1
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!
  
Terekhov, Mikhail via Gdb-patches Aug. 21, 2019, 5:28 p.m. UTC | #2
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
  
Sergio Durigan Junior Aug. 21, 2019, 5:33 p.m. UTC | #3
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!
  

Patch

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\