Re: [PATCH 1/2]: Refactor relocate_path to also check if the relocated file/directory exists

[Khoo Yit Phang <khooyp@cs.umd.edu>]

[-- Attachment #1: Type: text/plain, Size: 443 bytes --]

Hi,

On Sep 21, 2012, at 3:09 PM, Andreas Schwab wrote:

> Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
>> It would have to be written as:
>>  	  if (stat (path, &s) != 0 || !isdir != !S_ISDIR (s.st_mode))
> 
> A common idiom is to use !! when the other side is already boolean.


I like this idiom, it does seem to be used in GDB, and isdir should be a boolean. Here's a patch that uses it.

Yit
September 22, 2012


[-- Attachment #2: refactor-relocate-path.txt --]
[-- Type: text/plain, Size: 2819 bytes --]

gdb/ChangeLog

2012-09-21  Khoo Yit Phang  <khooyp@cs.umd.edu>

	Refactor relocate_path to also check if the relocated
	file/directory exists.
	* 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.

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.  */
 
 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,17 +133,7 @@
 {
   char *dir;
 
-  dir = relocate_path (gdb_program_name, initial, flag);
-  if (dir)
-    {
-      struct stat s;
-
-      if (stat (dir, &s) != 0 || !S_ISDIR (s.st_mode))
-	{
-	  xfree (dir);
-	  dir = NULL;
-	}
-    }
+  dir = relocate_path (gdb_program_name, initial, 1, flag);
   if (!dir)
     dir = xstrdup (initial);
 
@@ -162,15 +169,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);