Re: gdb_realpath: dealing with ./ and ../

["Doug Evans" <dje@google.com>]

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

[fwiw ...]

I wonder if things would be cleaner if the path argument to
start_subfile (name, dirname) was processed the same as each subfile's
path (subfile->name, subfile->dirname).  Otherwise it seems like a
potential bug source (not necessarily now, but down the road).  There
(apparently) have been edits to this area of the code in the past that
have introduced bugs.

i.e. how about writing a utility to process name,dirname and using
that utility for both the argument to start_subfile and each subfile
in the comparison loop?  This patch is untested, I'm just providing a
definitive example of the suggestion.

Another question: In the case where dirname is prepended to name,
should the whole thing be passed to rewrite_source_path, instead of
just dirname?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: rewrite-path-2.patch --]
[-- Type: text/x-patch; name=rewrite-path-2.patch, Size: 3610 bytes --]

Index: buildsym.c
===================================================================
RCS file: /cvs/src/src/gdb/buildsym.c,v
retrieving revision 1.59
diff -u -p -c -r1.59 buildsym.c
*** buildsym.c	1 Jan 2008 22:53:09 -0000	1.59
--- buildsym.c	4 Jan 2008 20:22:00 -0000
*************** static struct obstack pending_addrmap_ob
*** 86,91 ****
--- 87,93 ----
  static int pending_addrmap_interesting;
  
  \f
+ static char *rewrite_subfile_path (char *name, char* dirname);
  static int compare_line_numbers (const void *ln1p, const void *ln2p);
  \f
  
*************** void
*** 583,617 ****
  start_subfile (char *name, char *dirname)
  {
    struct subfile *subfile;
  
    /* See if this subfile is already known as a subfile of the current
       main source file.  */
  
    for (subfile = subfiles; subfile; subfile = subfile->next)
      {
!       char *subfile_name;
! 
!       /* If NAME is an absolute path, and this subfile is not, then
! 	 attempt to create an absolute path to compare.  */
!       if (IS_ABSOLUTE_PATH (name)
! 	  && !IS_ABSOLUTE_PATH (subfile->name)
! 	  && subfile->dirname != NULL)
! 	subfile_name = concat (subfile->dirname, SLASH_STRING,
! 			       subfile->name, NULL);
!       else
! 	subfile_name = subfile->name;
  
!       if (FILENAME_CMP (subfile_name, name) == 0)
  	{
  	  current_subfile = subfile;
! 	  if (subfile_name != subfile->name)
! 	    xfree (subfile_name);
  	  return;
  	}
!       if (subfile_name != subfile->name)
! 	xfree (subfile_name);
      }
  
    /* This subfile is not known.  Add an entry for it. Make an entry
       for this subfile in the list of all subfiles of the current main
       source file.  */
--- 599,630 ----
  start_subfile (char *name, char *dirname)
  {
    struct subfile *subfile;
+   /* Rewritten NAME, typically an absolute path, that is used to detect
+      identical subfiles.  */
+   char *comparable_name;
+ 
+   comparable_name = rewrite_subfile_path (name, dirname);
  
    /* See if this subfile is already known as a subfile of the current
       main source file.  */
  
    for (subfile = subfiles; subfile; subfile = subfile->next)
      {
!       char *subfile_name = rewrite_subfile_path (subfile->name,
! 						 subfile->dirname);
  
!       if (FILENAME_CMP (subfile_name, comparable_name) == 0)
  	{
  	  current_subfile = subfile;
! 	  xfree (comparable_name);
! 	  xfree (subfile_name);
  	  return;
  	}
!       xfree (subfile_name);
      }
  
+   xfree (comparable_name);
+ 
    /* This subfile is not known.  Add an entry for it. Make an entry
       for this subfile in the list of all subfiles of the current main
       source file.  */
*************** start_subfile (char *name, char *dirname
*** 681,686 ****
--- 694,727 ----
      }
  }
  
+ /* Subroutine of start_subfile to simplify it.
+    Convert NAME, DIRNAME to a form that can be used to watch for
+    identical subfiles.
+    Space for the result is malloc'd, caller must free.  */
+ 
+ static char *
+ rewrite_subfile_path (char *name, char *dirname)
+ {
+   char *p = name;
+   char *rwname;
+ 
+   if (! IS_ABSOLUTE_PATH (name)
+       && dirname != NULL)
+     p = concat (dirname, SLASH_STRING, name, NULL);
+ 
+   rwname = rewrite_source_path (p);
+ 
+   if (rwname != NULL)
+     {
+       if (p != name)
+ 	xfree (p);
+       return rwname;
+     }
+   if (p != name)
+     return p;
+   return xstrdup (name);
+ }
+ 
  /* For stabs readers, the first N_SO symbol is assumed to be the
     source file name, and the subfile struct is initialized using that
     assumption.  If another N_SO symbol is later seen, immediately