Re: [PATCH] Try to initialize data-directory by first searching for "data-directory" in the same directory as the gdb binary

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Tue, 18 Sep 2012 22:32:40 +0200, Khoo Yit Phang wrote:
> As discussed previously, here a patch that tries to initialize
> data-directory by first searching for "data-directory" in the same directory
> as the gdb binary.

I do not see how is it related to the 'isdir' addition, IMO 'isdir' is an
unrelated patch which should be posted separately.


> Additionally, I've also reverted my previous change to cc-with-tweaks.sh
> that added the -data-directory flag, since it is no longer necessary.

It can be kept there, it does not harm anything and it then does not depend on
a bit magic relocate_gdb_directory.



Thanks,
Jan


> gdb/ChangeLog
> 
> 2012-09-18  Khoo Yit Phang  <khooyp@cs.umd.edu>
> 
> 	Try to initialize data-directory by first searching for
> 	"data-directory" in the same directory as the gdb binary.
> 	* contrib/cc-with-tweaks.sh (GDB): Revert -data-directory
> 	additions that is now unnecessary.
> 	* main.c (relocate_path): Add an isdir argument, and check that
> 	the relocated file/directory exists.
> 	(relocate_gdb_directory): Remove the directory check that is
> 	subsumed by relocate_path.
> 	(get_init_files): Remove the file check that is subsumed by
> 	relocate_path.
> 	(relocate_gdb_data_directory): New function similar to
> 	relocate_gdb_directory, but specifically for data-directory.
> 	(captured_main): Call relocate_gdb_data_directory to initialize
> 	gdb_datadir.
> 
> diff --git a/gdb/contrib/cc-with-tweaks.sh b/gdb/contrib/cc-with-tweaks.sh
> --- a/gdb/contrib/cc-with-tweaks.sh
> +++ b/gdb/contrib/cc-with-tweaks.sh
> @@ -19,8 +19,7 @@
>  # This program requires gdb and objcopy in addition to gcc.
>  # The default values are gdb from the build tree and objcopy from $PATH.
>  # They may be overridden by setting environment variables GDB and OBJCOPY
> -# respectively.  Note that GDB should contain the gdb binary as well as the
> -# -data-directory flag, e.g., "foo/gdb -data-directory foo/data-directory".
> +# respectively.
>  # We assume the current directory is either $obj/gdb or $obj/gdb/testsuite.
>  #
>  # Example usage:
> @@ -47,13 +46,13 @@
>  then
>      if [ -f ./gdb ]
>      then
> -	GDB="./gdb -data-directory data-directory"
> +	GDB="./gdb"
>      elif [ -f ../gdb ]
>      then
> -	GDB="../gdb -data-directory ../data-directory"
> +	GDB="../gdb"
>      elif [ -f ../../gdb ]
>      then
> -	GDB="../../gdb -data-directory ../../data-directory"
> +	GDB="../../gdb"
>      else
>  	echo "$myname: unable to find usable gdb" >&2
>  	exit 1
> diff --git a/gdb/main.c b/gdb/main.c
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -91,17 +91,34 @@
>  
>  static void print_gdb_help (struct ui_file *);
>  
> -/* Relocate a file or directory.  PROGNAME is the name by which gdb
> -   was invoked (i.e., argv[0]).  INITIAL is the default value for the
> -   file or directory.  FLAG is true if the value is relocatable, false
> -   otherwise.  Returns a newly allocated string; this may return NULL
> -   under the same conditions as make_relative_prefix.  */
> +/* Relocate a file or directory, checking if it exists.  PROGNAME is the
> +   name by which gdb was invoked (i.e., argv[0]).  INITIAL is the default
> +   value for the file or directory.  ISDIR is true if INITIAL is a
> +   directory.  FLAG is true if the value is relocatable, false otherwise.
> +   Returns a newly allocated string; this may return NULL under the same
> +   conditions as make_relative_prefix, or if the relocated path does not
> +   exist.  */

GDB comments are formatted to textwidth=72 which may explain this excessive
reformatting.  Try to keep textwidth=72 instead of changes back and forth.


>  
>  static char *
> -relocate_path (const char *progname, const char *initial, int flag)
> +relocate_path (const char *progname, const char *initial, int isdir,
> +	       int flag)
>  {
>    if (flag)
> -    return make_relative_prefix (progname, BINDIR, initial);
> +    {
> +      char *path;
> +      path = make_relative_prefix (progname, BINDIR, initial);
> +      if (path)
> +	{
> +	  struct stat s;
> +
> +	  if (stat (path, &s) != 0 || (isdir && !S_ISDIR (s.st_mode)))
> +	    {
> +	      xfree (path);
> +	      path = NULL;
> +	    }
> +	}
> +      return path;
> +    }
>    return xstrdup (initial);
>  }
>  
> @@ -116,19 +133,52 @@
>  {
>    char *dir;
>  
> -  dir = relocate_path (gdb_program_name, initial, flag);
> -  if (dir)
> +  dir = relocate_path (gdb_program_name, initial, 1, flag);
> +  if (!dir)
> +    dir = xstrdup (initial);
> +
> +  /* Canonicalize the directory.  */
> +  if (*dir)
>      {
> -      struct stat s;
> +      char *canon_sysroot = lrealpath (dir);
>  
> -      if (stat (dir, &s) != 0 || !S_ISDIR (s.st_mode))
> +      if (canon_sysroot)
>  	{
>  	  xfree (dir);
> -	  dir = NULL;
> +	  dir = canon_sysroot;
>  	}
>      }
> +
> +  return dir;
> +}
> +
> +/* Like relocate_gdb_path, but specifically for data-directory. */
> +
> +static char *
> +relocate_gdb_data_directory (void)
> +{
> +  char *dir;
> +
> +  /* First try to find "data-directory" in the same directory as gdb.
> +
> +    Use relocate_path only to resolve the parent directory of
> +    gdb_program_name (i.e., based on PATH if necessary); relocate_path
> +    (gdb_program_name, BINDIR "/data-directory") cannot be used to resolve
> +    data-directory as it returns a path relative to the _grandparent
> +    directory_ of gdb_program_name (munging the parent directory).  */
> +
> +  dir = relocate_path (gdb_program_name, BINDIR, 1, 1);
> +  if (dir)
> +    dir = reconcat (dir, dir, SLASH_STRING, "data-directory", NULL);
> +
> +  /* Then try to find GDB_DATADIR relocated relative to gdb. */
>    if (!dir)

It can be more clear by replacing this 'if (!dir)' by 'else'.


> -    dir = xstrdup (initial);
> +    dir = relocate_path (gdb_program_name, GDB_DATADIR, 1,
> +			 GDB_DATADIR_RELOCATABLE);

Why isn't here the original statement?
	dir = relocate_gdb_directory (GDB_DATADIR, GDB_DATADIR_RELOCATABLE);



> +
> +  /* Otherwise use GDB_DATADIR as is. */
> +  if (!dir)
> +    dir = xstrdup (GDB_DATADIR);

This can be moved to the 'else' block suggested above to simplify it.


>  
>    /* Canonicalize the directory.  */
>    if (*dir)
> @@ -162,15 +212,15 @@
>  
>    if (!initialized)
>      {
> -      struct stat homebuf, cwdbuf, s;
> +      struct stat homebuf, cwdbuf;
>        char *homedir, *relocated_sysgdbinit;
>  
>        if (SYSTEM_GDBINIT[0])
>  	{
>  	  relocated_sysgdbinit = relocate_path (gdb_program_name,
> -						SYSTEM_GDBINIT,
> +						SYSTEM_GDBINIT, 0,
>  						SYSTEM_GDBINIT_RELOCATABLE);
> -	  if (relocated_sysgdbinit && stat (relocated_sysgdbinit, &s) == 0)
> +	  if (relocated_sysgdbinit)
>  	    sysgdbinit = relocated_sysgdbinit;
>  	  else
>  	    xfree (relocated_sysgdbinit);
> @@ -363,8 +413,7 @@
>    debug_file_directory = relocate_gdb_directory (DEBUGDIR,
>  						 DEBUGDIR_RELOCATABLE);
>  
> -  gdb_datadir = relocate_gdb_directory (GDB_DATADIR,
> -					GDB_DATADIR_RELOCATABLE);
> +  gdb_datadir = relocate_gdb_data_directory ();
>  
>  #ifdef WITH_PYTHON_PATH
>    {