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

[Sergio Durigan Junior <sergiodj@redhat.com>]

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/