diff mbox

[1/2] Try both make_relative_prefix{,ignore_links} for relocatability

Message ID 047d7b15fd01af6a62051d214793@google.com
State New
Headers show

Commit Message

Doug Evans Aug. 12, 2015, 6:20 p.m. UTC
Hi.

I have a use-case where I need relocatability in a tree of symlinks.

The fix is straightforward enough.
The testcase may need some thought.
It does a "make install" into the testsuite subdir,
and then sets up the necessary conditions to properly test
make_relative_prefix{,ignore_links}.

The test runs fast enough, that's not the issue.
Just dunno how people will feel about doing a "make install"
in a testcase.
OTOH, this is what we need to test: the behaviour of the installed gdb.

The test is contingent on whether the user configured the tree
in a relocatable way. The test won't fail, but I'm guessing
re-running configure/make for this test is a bit too much,
whereas what's here is a reasonable compromise.

1/2 is the patch
2/2 is the testcase

2015-08-12  Doug Evans  <dje@google.com>

	* completer.c (gdb_path_isdir): Move to ...
	* utils.c (gdb_path_isdir): ... here.
	(gdb_path_isfile): New function.
	* utils.h (gdb_path_isdir): Declare.
	(gdb_path_isfile): Declare.
	* main.c (relocate_gdb_file): Renamed from relocate_path.
	Try both make_relative_prefix and make_relative_prefix_ignore_symlinks.
	All callers updated.
	(maybe_lrealpath): New function.
	(relocate_gdb_directory: Try both make_relative_prefix and
	make_relative_prefix_ignore_symlinks.

Comments

Pedro Alves Sept. 29, 2015, 11:01 a.m. UTC | #1
On 08/12/2015 07:20 PM, Doug Evans wrote:
> -    }
> +/* Relocate a directory.
> +   INITIAL is the default value of the directory.
> +   FLAG is true if the value is relocatable, false otherwise.

Nit: why not s/FLAG/RELOCATABLE/ then?

> +   If the result is a directory, it is used; otherwise, INITIAL is used.
> +   The chosen directory is then canonicalized using lrealpath.  This
> +   function always returns a newly-allocated string.  */
> +
> +char *
> +relocate_gdb_directory (const char *initial, int flag)
> +{
> +  char *dir;
> 
> -  return dir;
> +  /* Early exit if there's nothing we can do.  */
> +  if (initial[0] == '\0')
> +    return xstrdup ("");
> +  if (!flag)
> +    return maybe_lrealpath (xstrdup (initial));
> +

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/completer.c b/gdb/completer.c
index 7fc27b1..870e70f 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -1461,17 +1461,6 @@  gdb_display_match_list_pager (int lines,
      return 0;
  }

-/* Return non-zero if FILENAME is a directory.
-   Based on readline/complete.c:path_isdir.  */
-
-static int
-gdb_path_isdir (const char *filename)
-{
-  struct stat finfo;
-
-  return (stat (filename, &finfo) == 0 && S_ISDIR (finfo.st_mode));
-}
-
  /* Return the portion of PATHNAME that should be output when listing
     possible completions.  If we are hacking filename completion, we
     are only interested in the basename, the portion following the
diff --git a/gdb/main.c b/gdb/main.c
index aecd60a..34dd850 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -137,58 +137,97 @@  set_gdb_data_directory (const char *new_datadir)
      }
  }

-/* 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.
+   INITIAL is the default value of the file.
+   FLAG is true if the value is relocatable, false otherwise.
+   Returns a newly allocated string; this may return NULL
+   under the same conditions as gdb_make_relative_prefix.  */

  static char *
-relocate_path (const char *progname, const char *initial, int flag)
+relocate_gdb_file (const char *initial, int flag)
  {
-  if (flag)
-    return make_relative_prefix (progname, BINDIR, initial);
+  char *path;
+
+  /* Early exit if there's nothing we can do.  */
+  if (!flag)
+    return xstrdup (initial);
+
+  /* We have to try both make_relative_prefix and
+     make_relative_prefix_ignore_links. The first handles a symlink to
+     gdb where it is installed.  The second handles the case where the
+     installation tree is itself a collection of symlinks to random places.
+     Alas we can't distinguish a NULL return from make_relative_prefix*
+     due to lack of memory or due to failure to find a common prefix.
+     make_relative_prefix is tried first for backward compatibility.   
Ugh.  */
+
+  path = make_relative_prefix (gdb_program_name, BINDIR, initial);
+  if (path != NULL && gdb_path_isfile (path))
+    return path;
+  xfree (path);
+  path = make_relative_prefix_ignore_links (gdb_program_name, BINDIR,  
initial);
+  if (path != NULL && gdb_path_isfile (path))
+    return path;
+  xfree (path);
    return xstrdup (initial);
  }

-/* Like relocate_path, but specifically checks for a directory.
-   INITIAL is relocated according to the rules of relocate_path.  If
-   the result is a directory, it is used; otherwise, INITIAL is used.
-   The chosen directory is then canonicalized using lrealpath.  This
-   function always returns a newly-allocated string.  */
+/* Return the lrealpath form of PATH.
+   Space for PATH must have been malloc'd.
+   PATH is freed if the lrealpath'd form is different.  */

-char *
-relocate_gdb_directory (const char *initial, int flag)
+static char *
+maybe_lrealpath (char *path)
  {
-  char *dir;
+  char *canon;

-  dir = relocate_path (gdb_program_name, initial, flag);
-  if (dir)
-    {
-      struct stat s;
+  if (*path == '\0')
+    return path;

-      if (*dir == '\0' || stat (dir, &s) != 0 || !S_ISDIR (s.st_mode))
-	{
-	  xfree (dir);
-	  dir = NULL;
-	}
+  canon = lrealpath (path);
+  if (canon != NULL)
+    {
+      xfree (path);
+      path = canon;
      }
-  if (!dir)
-    dir = xstrdup (initial);

-  /* Canonicalize the directory.  */
-  if (*dir)
-    {
-      char *canon_sysroot = lrealpath (dir);
+  return path;
+}

-      if (canon_sysroot)
-	{
-	  xfree (dir);
-	  dir = canon_sysroot;
-	}
-    }
+/* Relocate a directory.
+   INITIAL is the default value of the directory.
+   FLAG is true if the value is relocatable, false otherwise.
+   If the result is a directory, it is used; otherwise, INITIAL is used.
+   The chosen directory is then canonicalized using lrealpath.  This
+   function always returns a newly-allocated string.  */
+
+char *
+relocate_gdb_directory (const char *initial, int flag)
+{
+  char *dir;

-  return dir;
+  /* Early exit if there's nothing we can do.  */
+  if (initial[0] == '\0')
+    return xstrdup ("");
+  if (!flag)
+    return maybe_lrealpath (xstrdup (initial));
+
+  /* We have to try both make_relative_prefix and
+     make_relative_prefix_ignore_links. The first handles a symlink to
+     gdb where it is installed.  The second handles the case where the
+     installation tree is itself a collection of symlinks to random places.
+     Alas we can't distinguish a NULL return from make_relative_prefix*
+     due to lack of memory or due to failure to find a common prefix.  Ugh.
+     make_relative_prefix is tried first for backward compatibility.  */
+
+  dir = make_relative_prefix (gdb_program_name, BINDIR, initial);
+  if (dir != NULL && gdb_path_isdir (dir))
+    return maybe_lrealpath (dir);
+  xfree (dir);
+  dir = make_relative_prefix_ignore_links (gdb_program_name, BINDIR,  
initial);
+  if (gdb_path_isdir (dir))
+    return maybe_lrealpath (dir);
+  xfree (dir);
+  return maybe_lrealpath (xstrdup (initial));
  }

  /* Compute the locations of init files that GDB should source and
@@ -237,9 +276,9 @@  get_init_files (const char **system_gdbinit,
  	    }
  	  else
  	    {
-	      relocated_sysgdbinit = relocate_path (gdb_program_name,
-						    SYSTEM_GDBINIT,
-						    SYSTEM_GDBINIT_RELOCATABLE);
+	      relocated_sysgdbinit
+		= relocate_gdb_file (SYSTEM_GDBINIT,
+				     SYSTEM_GDBINIT_RELOCATABLE);
  	    }
  	  if (relocated_sysgdbinit && stat (relocated_sysgdbinit, &s) == 0)
  	    sysgdbinit = relocated_sysgdbinit;
diff --git a/gdb/utils.c b/gdb/utils.c
index e5ad195..dfdd04d 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3015,6 +3015,28 @@  ldirname (const char *filename)
    return dirname;
  }

+/* See utils.h.
+   Based on readline/complete.c:path_isdir.  */
+
+int
+gdb_path_isdir (const char *path)
+{
+  struct stat s;
+
+  return stat (path, &s) == 0 && S_ISDIR (s.st_mode);
+}
+
+/* See utils.h.
+   Based on readline/complete.c:path_isdir.  */
+
+int
+gdb_path_isfile (const char *path)
+{
+  struct stat s;
+
+  return stat (path, &s) == 0 && S_ISREG (s.st_mode);
+}
+
  /* Call libiberty's buildargv, and return the result.
     If buildargv fails due to out-of-memory, call nomem.
     Therefore, the returned value is guaranteed to be non-NULL,
diff --git a/gdb/utils.h b/gdb/utils.h
index 995a1cf..0adb097 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -135,6 +135,14 @@  extern void substitute_path_component (char **stringp,  
const char *from,
  				       const char *to);

  char *ldirname (const char *filename);
+
+/* Return non-zero if PATH is a directory.  */
+
+extern int gdb_path_isdir (const char *path);
+
+/* Return non-zero if PATH is a plain file.  */
+
+extern int gdb_path_isfile (const char *path);
  
  /* GDB output, ui_file utilities.  */