[patchv2 3/11] Code cleanup: Drop IS_ABSOLUTE_PATH checks

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

formerly:
	[patch 2/9] Code cleanup: Drop IS_ABSOLUTE_PATH checks
	http://sourceware.org/ml/gdb-patches/2013-01/msg00384.html


Thanks,
Jan


gdb/
2013-01-25  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Code cleanup.
	* breakpoint.c (clear_command): Remove variable is_abs, unify the
	call of filename_cmp with compare_filenames_for_search.
	* dwarf2read.c (dw2_map_symtabs_matching_filename): Remove variable
	is_abs, unify the call of FILENAME_CMP with
	compare_filenames_for_search.  New gdb_asserts for real_path and name.
	Unify the call of compare_filenames_for_search with FILENAME_CMP.
	* psymtab.c (partial_map_symtabs_matching_filename): Likewise.
	* symfile.h (struct quick_symbol_functions): Extend the comment for
	map_symtabs_matching_filename.
	* symtab.c (compare_filenames_for_search): Remove the function comment
	relative path requirement.  Handle absolute filenames, with a comment.
	(iterate_over_some_symtabs): Remove variable is_abs, unify the call of
	FILENAME_CMP with compare_filenames_for_search.  New gdb_asserts for
	real_path and name.  Unify the call of compare_filenames_for_search
	with FILENAME_CMP.
	(iterate_over_symtabs): New gdb_assert on REAL_PATH.

gdb/testsuite/
2013-01-25  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.mi/mi-fullname-deleted.exp: Use double last slash for $srcfileabs.
	(compare_filenames_for_search does not match)
	(compare_filenames_for_search does match): New tests.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -11941,8 +11941,6 @@ clear_command (char *arg, int from_tty)
   make_cleanup (VEC_cleanup (breakpoint_p), &found);
   for (i = 0; i < sals.nelts; i++)
     {
-      int is_abs;
-
       /* If exact pc given, clear bpts at that pc.
          If line given (pc == 0), clear all bpts on specified line.
          If defaulting, clear all bpts on default line
@@ -11956,7 +11954,6 @@ clear_command (char *arg, int from_tty)
          1              0             <can't happen> */
 
       sal = sals.sals[i];
-      is_abs = sal.symtab == NULL ? 1 : IS_ABSOLUTE_PATH (sal.symtab->filename);
 
       /* Find all matching breakpoints and add them to 'found'.  */
       ALL_BREAKPOINTS (b)
@@ -11984,12 +11981,8 @@ clear_command (char *arg, int from_tty)
 		      && sal.pspace == loc->pspace
 		      && loc->line_number == sal.line)
 		    {
-		      if (filename_cmp (loc->symtab->filename,
-					sal.symtab->filename) == 0)
-			line_match = 1;
-		      else if (!IS_ABSOLUTE_PATH (sal.symtab->filename)
-			       && compare_filenames_for_search (loc->symtab->filename,
-								sal.symtab->filename))
+		      if (compare_filenames_for_search (loc->symtab->filename,
+							sal.symtab->filename))
 			line_match = 1;
 		    }
 
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3048,7 +3048,6 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const char *name,
 {
   int i;
   const char *name_basename = lbasename (name);
-  int is_abs = IS_ABSOLUTE_PATH (name);
 
   dw2_setup (objfile);
 
@@ -3073,8 +3072,7 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const char *name,
 	{
 	  const char *this_name = file_data->file_names[j];
 
-	  if (FILENAME_CMP (name, this_name) == 0
-	      || (!is_abs && compare_filenames_for_search (this_name, name)))
+	  if (compare_filenames_for_search (this_name, name))
 	    {
 	      if (dw2_map_expand_apply (objfile, per_cu, name, real_path,
 					callback, data))
@@ -3092,11 +3090,10 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const char *name,
 	      const char *this_real_name = dw2_get_real_path (objfile,
 							      file_data, j);
 
+	      gdb_assert (IS_ABSOLUTE_PATH (real_path));
+	      gdb_assert (IS_ABSOLUTE_PATH (name));
 	      if (this_real_name != NULL
-		  && (FILENAME_CMP (real_path, this_real_name) == 0
-		      || (!is_abs
-			  && compare_filenames_for_search (this_real_name,
-							   name))))
+		  && FILENAME_CMP (real_path, this_real_name) == 0)
 		{
 		  if (dw2_map_expand_apply (objfile, per_cu, name, real_path,
 					    callback, data))
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -166,7 +166,6 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
 {
   struct partial_symtab *pst;
   const char *name_basename = lbasename (name);
-  int is_abs = IS_ABSOLUTE_PATH (name);
 
   ALL_OBJFILE_PSYMTABS_REQUIRED (objfile, pst)
   {
@@ -179,8 +178,7 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
     if (pst->anonymous)
       continue;
 
-    if (FILENAME_CMP (name, pst->filename) == 0
-	|| (!is_abs && compare_filenames_for_search (pst->filename, name)))
+    if (compare_filenames_for_search (pst->filename, name))
       {
 	if (partial_map_expand_apply (objfile, name, real_path,
 				      pst, callback, data))
@@ -197,10 +195,11 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
        this symtab and use its absolute path.  */
     if (real_path != NULL)
       {
+	gdb_assert (IS_ABSOLUTE_PATH (real_path));
+	gdb_assert (IS_ABSOLUTE_PATH (name));
 	psymtab_to_fullname (pst);
 	if (pst->fullname != NULL
-	    && (FILENAME_CMP (real_path, pst->fullname) == 0
-		|| (!is_abs && compare_filenames_for_search (real_path, name))))
+	    && FILENAME_CMP (real_path, pst->fullname) == 0)
 	  {
 	    if (partial_map_expand_apply (objfile, name, real_path,
 					  pst, callback, data))
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -159,9 +159,10 @@ struct quick_symbol_functions
   /* Expand and iterate over each "partial" symbol table in OBJFILE
      where the source file is named NAME.
 
-     If NAME is not absolute, a match after a '/' in the symbol
-     table's file name will also work.  REAL_PATH is the absolute file
-     name run through gdb_realpath.
+     If NAME is not absolute, a match after a '/' in the symbol table's
+     file name will also work, REAL_PATH is NULL then.  If NAME is
+     absolute then REAL_PATH is non-NULL absolute file name as resolved
+     via gdb_realpath from NAME.
 
      If a match is found, the "partial" symbol table is expanded.
      Then, this calls iterate_over_some_symtabs (or equivalent) over
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -146,8 +146,8 @@ const struct block *block_found;
 
 /* See whether FILENAME matches SEARCH_NAME using the rule that we
    advertise to the user.  (The manual's description of linespecs
-   describes what we advertise).  We assume that SEARCH_NAME is
-   a relative path.  Returns true if they match, false otherwise.  */
+   describes what we advertise).  Returns true if they match, false
+   otherwise.  */
 
 int
 compare_filenames_for_search (const char *filename, const char *search_name)
@@ -166,12 +166,18 @@ compare_filenames_for_search (const char *filename, const char *search_name)
      preceding the trailing SEARCH_NAME segment of FILENAME must be a
      directory separator.
 
+     The check !IS_ABSOLUTE_PATH ensures SEARCH_NAME "/dir/file.c"
+     cannot match FILENAME "/path//dir/file.c" - as user has requested
+     absolute path.  The sama applies for "c:\file.c" possibly
+     incorrectly hypothetically matching "d:\dir\c:\file.c".
+
      The HAS_DRIVE_SPEC purpose is to make FILENAME "c:file.c"
      compatible with SEARCH_NAME "file.c".  In such case a compiler had
      to put the "c:file.c" name into debug info.  Such compatibility
      works only on GDB built for DOS host.  */
   return (len == search_len
-	  || IS_DIR_SEPARATOR (filename[len - search_len - 1])
+	  || (!IS_ABSOLUTE_PATH (search_name)
+	      && IS_DIR_SEPARATOR (filename[len - search_len - 1]))
 	  || (HAS_DRIVE_SPEC (filename)
 	      && STRIP_DRIVE_SPEC (filename) == &filename[len - search_len]));
 }
@@ -198,18 +204,10 @@ iterate_over_some_symtabs (const char *name,
 {
   struct symtab *s = NULL;
   const char* base_name = lbasename (name);
-  int is_abs = IS_ABSOLUTE_PATH (name);
 
   for (s = first; s != NULL && s != after_last; s = s->next)
     {
-      /* Exact match is always ok.  */
-      if (FILENAME_CMP (name, s->filename) == 0)
-	{
-	  if (callback (s, data))
-	    return 1;
-	}
-
-      if (!is_abs && compare_filenames_for_search (s->filename, name))
+      if (compare_filenames_for_search (s->filename, name))
 	{
 	  if (callback (s, data))
 	    return 1;
@@ -228,17 +226,13 @@ iterate_over_some_symtabs (const char *name,
       {
         const char *fullname = symtab_to_fullname (s);
 
+	gdb_assert (IS_ABSOLUTE_PATH (real_path));
+	gdb_assert (IS_ABSOLUTE_PATH (name));
 	if (FILENAME_CMP (real_path, fullname) == 0)
 	  {
 	    if (callback (s, data))
 	      return 1;
 	  }
-
-	if (!is_abs && compare_filenames_for_search (fullname, name))
-	  {
-	    if (callback (s, data))
-	      return 1;
-	  }
       }
     }
 
@@ -269,6 +263,7 @@ iterate_over_symtabs (const char *name,
     {
       real_path = gdb_realpath (name);
       make_cleanup (xfree, real_path);
+      gdb_assert (IS_ABSOLUTE_PATH (real_path));
     }
 
   ALL_OBJFILES (objfile)
--- a/gdb/testsuite/gdb.mi/mi-fullname-deleted.exp
+++ b/gdb/testsuite/gdb.mi/mi-fullname-deleted.exp
@@ -24,6 +24,12 @@ if [mi_gdb_start] {
 standard_testfile
 set srcfileabs [standard_output_file $srcfile]
 
+# "//$srcfile" It is used for the test of compare_filenames_for_search.
+if { [regsub {/[^/]+$} $srcfileabs {/\0} srcfileabs] != 1 } {
+    xfail "Cannot double the last slash separator"
+    return -1
+}
+
 if { [regsub {^(/[^/]+)/} $srcfileabs {\1subst/} srcfileabssubst] != 1
      || [regsub {^(/[^/]+)/.*$} $srcfileabs {\1} initdir] != 1 } {
     xfail "Missing root subdirectory"
@@ -49,3 +55,12 @@ mi_gdb_test "-interpreter-exec console \"set substitute-path ${initdir} ${initdi
 mi_gdb_test "-file-list-exec-source-file" ".*\",fullname=\".*\".*" "fullname present"
 
 mi_gdb_test "-file-list-exec-source-file" ".*\",fullname=\"[string_to_regexp $srcfileabssubst]\".*" "substituted fullname"
+
+# Test compare_filenames_for_search does not falsely use absolute filename as
+# a relative one.
+mi_gdb_test "-break-insert -t /$srcfile:main" \
+	    "\\^error,msg=\"No source file named /[string_to_regexp $srcfile]\\.\"" \
+	    "compare_filenames_for_search does not match"
+mi_gdb_test "-break-insert -t $srcfile:main" \
+	    {\^done,bkpt=.*} \
+	    "compare_filenames_for_search does match"