[RFA] xfullpath and new regression test xfullpath.exp - take 2

[RFA] xfullpath and new regression test xfullpath.exp - take 2

[Joel Brobecker]

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

Hello,

based on Eli's comments, I reworked the patch I posted in the
following message:

     http://sources.redhat.com/ml/gdb-patches/2002-04/msg00000.html

I reran the regression test on x86-linux and observed no new regression.
I am not reattaching the xfullpath.exp testcase, since Andrew already
approved it, but it can be retrieved at the URL above. I also took note
that this test will be located in the new gdb.gdb directory.

The patch is attached. I used the isalpha function, which is part of
ANSI C. I hope this is fine?

ChangeLog entry for gdb (unchanged except for the date):

2002-04-05  J. Brobecker  <brobecker@gnat.com>

        * utils.c (xfullpath): New function.
        * defs.h (xfullpath): Add declaration.
        * source.c (openp): Use xfullpath in place of gdb_realpath to
        avoid resolving the basename part of filenames when the
        associated file is a symbolic link. This fixes a potential
        inconsistency between the filenames known to GDB and the
        filenames it prints in the annotations.
        * symtab.c (lookup_symtab): Use the new xfullpath function, in order
        to be able to match a filename with either the real filename, or
        the name of any symbolic link to this file.
        (lookup_partial_symtab): Ditto.

The ChangeLog for gdb/testsuite:

2002-04-05  J. Brobecker  <brobecker@gnat.com>

        * gdb.gdb/xfullpath.exp: New test, to exercise the new
        xfullpath () function.

OK to commit?
-- 
Joel

[-- Attachment #2: xfullpath.diff --]
[-- Type: text/plain, Size: 10377 bytes --]

Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.85
diff -c -3 -p -r1.85 defs.h
*** defs.h	2002/03/23 17:38:12	1.85
--- defs.h	2002/04/05 14:21:03
*************** extern CORE_ADDR host_pointer_to_address
*** 372,377 ****
--- 372,378 ----
  extern void *address_to_host_pointer (CORE_ADDR addr);
  
  extern char *gdb_realpath (const char *);
+ extern char *xfullpath (const char *);
  
  /* From demangle.c */
  
Index: source.c
===================================================================
RCS file: /cvs/src/src/gdb/source.c,v
retrieving revision 1.25
diff -c -3 -p -r1.25 source.c
*** source.c	2002/03/06 06:28:33	1.25
--- source.c	2002/04/05 14:21:03
*************** source_info (char *ignore, int from_tty)
*** 514,520 ****
     get that particular version of foo or an error message).
  
     If FILENAME_OPENED is non-null, set it to a newly allocated string naming
!    the actual file opened (this string will always start with a "/".  We
     have to take special pains to avoid doubling the "/" between the directory
     and the file, sigh!  Emacs gets confuzzed by this when we print the
     source file name!!! 
--- 514,520 ----
     get that particular version of foo or an error message).
  
     If FILENAME_OPENED is non-null, set it to a newly allocated string naming
!    the actual file opened (this string will always start with a "/").  We
     have to take special pains to avoid doubling the "/" between the directory
     and the file, sigh!  Emacs gets confuzzed by this when we print the
     source file name!!! 
*************** openp (const char *path, int try_cwd_fir
*** 609,618 ****
  done:
    if (filename_opened)
      {
        if (fd < 0)
  	*filename_opened = NULL;
        else if (IS_ABSOLUTE_PATH (filename))
! 	*filename_opened = gdb_realpath (filename);
        else
  	{
  	  /* Beware the // my son, the Emacs barfs, the botch that catch... */
--- 609,623 ----
  done:
    if (filename_opened)
      {
+       /* If a file was opened, canonicalize its filename. Use xfullpath
+          rather than gdb_realpath to avoid resolving the basename part
+          of filenames when the associated file is a symbolic link. This
+          fixes a potential inconsistency between the filenames known to
+          GDB and the filenames it prints in the annotations.  */
        if (fd < 0)
  	*filename_opened = NULL;
        else if (IS_ABSOLUTE_PATH (filename))
! 	*filename_opened = xfullpath (filename);
        else
  	{
  	  /* Beware the // my son, the Emacs barfs, the botch that catch... */
*************** done:
*** 621,627 ****
             IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1])
  				     ? "" : SLASH_STRING,
  				     filename, NULL);
! 	  *filename_opened = gdb_realpath (f);
  	  xfree (f);
  	}
      }
--- 626,632 ----
             IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1])
  				     ? "" : SLASH_STRING,
  				     filename, NULL);
! 	  *filename_opened = xfullpath (f);
  	  xfree (f);
  	}
      }
Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.59
diff -c -3 -p -r1.59 symtab.c
*** symtab.c	2002/03/27 23:10:23	1.59
--- symtab.c	2002/04/05 14:21:05
*************** lookup_symtab (const char *name)
*** 144,154 ****
    register struct partial_symtab *ps;
    register struct objfile *objfile;
    char *real_path = NULL;
  
    /* Here we are interested in canonicalizing an absolute path, not
       absolutizing a relative path.  */
    if (IS_ABSOLUTE_PATH (name))
!     real_path = gdb_realpath (name);
  
  got_symtab:
  
--- 144,160 ----
    register struct partial_symtab *ps;
    register struct objfile *objfile;
    char *real_path = NULL;
+   char *full_path = NULL;
  
    /* Here we are interested in canonicalizing an absolute path, not
       absolutizing a relative path.  */
    if (IS_ABSOLUTE_PATH (name))
!     {
!       full_path = xfullpath (name);
!       make_cleanup (xfree, full_path);
!       real_path = gdb_realpath (name);
!       make_cleanup (xfree, real_path);
!     }
  
  got_symtab:
  
*************** got_symtab:
*** 158,181 ****
    {
      if (FILENAME_CMP (name, s->filename) == 0)
        {
- 	xfree (real_path);
  	return s;
        }
      /* If the user gave us an absolute path, try to find the file in
         this symtab and use its absolute path.  */
      if (real_path != NULL)
        {
! 	char *rp = symtab_to_filename (s);
  	if (FILENAME_CMP (real_path, rp) == 0)
  	  {
- 	    xfree (real_path);
  	    return s;
  	  }
        }
    }
  
-   xfree (real_path);
- 
    /* Now, search for a matching tail (only if name doesn't have any dirs) */
  
    if (lbasename (name) == name)
--- 164,195 ----
    {
      if (FILENAME_CMP (name, s->filename) == 0)
        {
  	return s;
        }
+       
      /* If the user gave us an absolute path, try to find the file in
         this symtab and use its absolute path.  */
+     
+     if (full_path != NULL)
+       {
+ 	const char *fp = symtab_to_filename (s);
+ 	if (FILENAME_CMP (full_path, fp) == 0)
+ 	  {
+ 	    return s;
+ 	  }
+       }
+ 
      if (real_path != NULL)
        {
!         const char *rp = gdb_realpath (symtab_to_filename (s));
!         make_cleanup (xfree, rp);
  	if (FILENAME_CMP (real_path, rp) == 0)
  	  {
  	    return s;
  	  }
        }
    }
  
    /* Now, search for a matching tail (only if name doesn't have any dirs) */
  
    if (lbasename (name) == name)
*************** lookup_partial_symtab (const char *name)
*** 221,256 ****
  {
    register struct partial_symtab *pst;
    register struct objfile *objfile;
    char *real_path = NULL;
  
    /* Here we are interested in canonicalizing an absolute path, not
       absolutizing a relative path.  */
    if (IS_ABSOLUTE_PATH (name))
!     real_path = gdb_realpath (name);
  
    ALL_PSYMTABS (objfile, pst)
    {
      if (FILENAME_CMP (name, pst->filename) == 0)
        {
- 	xfree (real_path);
  	return (pst);
        }
      /* If the user gave us an absolute path, try to find the file in
         this symtab and use its absolute path.  */
!     if (real_path != NULL)
        {
  	if (pst->fullname == NULL)
  	  source_full_path_of (pst->filename, &pst->fullname);
  	if (pst->fullname != NULL
! 	    && FILENAME_CMP (real_path, pst->fullname) == 0)
  	  {
- 	    xfree (real_path);
  	    return pst;
  	  }
        }
-   }
  
!   xfree (real_path);
  
    /* Now, search for a matching tail (only if name doesn't have any dirs) */
  
--- 235,289 ----
  {
    register struct partial_symtab *pst;
    register struct objfile *objfile;
+   char *full_path = NULL;
    char *real_path = NULL;
  
    /* Here we are interested in canonicalizing an absolute path, not
       absolutizing a relative path.  */
    if (IS_ABSOLUTE_PATH (name))
!     {
!       full_path = xfullpath (name);
!       make_cleanup (xfree, full_path);
!       real_path = gdb_realpath (name);
!       make_cleanup (xfree, real_path);
!     }
  
    ALL_PSYMTABS (objfile, pst)
    {
      if (FILENAME_CMP (name, pst->filename) == 0)
        {
  	return (pst);
        }
+ 
      /* If the user gave us an absolute path, try to find the file in
         this symtab and use its absolute path.  */
!     if (full_path != NULL)
        {
  	if (pst->fullname == NULL)
  	  source_full_path_of (pst->filename, &pst->fullname);
  	if (pst->fullname != NULL
! 	    && FILENAME_CMP (full_path, pst->fullname) == 0)
  	  {
  	    return pst;
  	  }
        }
  
!     if (real_path != NULL)
!       {
!         char *rp = NULL;
! 	if (pst->fullname == NULL)
! 	  source_full_path_of (pst->filename, &pst->fullname);
!         if (pst->fullname != NULL)
!           {
!             rp = gdb_realpath (pst->fullname);
!             make_cleanup (xfree, rp);
!           }
! 	if (rp != NULL && FILENAME_CMP (real_path, rp) == 0)
! 	  {
! 	    return pst;
! 	  }
!       }
!   }
  
    /* Now, search for a matching tail (only if name doesn't have any dirs) */
  
Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.71
diff -c -3 -p -r1.71 utils.c
*** utils.c	2002/03/28 03:43:37	1.71
--- utils.c	2002/04/05 14:21:06
***************
*** 61,66 ****
--- 61,67 ----
  #include "expression.h"
  #include "language.h"
  #include "annotate.h"
+ #include "filenames.h"
  
  #include "inferior.h" /* for signed_pointer_to_address */
  
*************** gdb_realpath (const char *filename)
*** 2559,2562 ****
--- 2560,2610 ----
  #else
    return xstrdup (filename);
  #endif
+ }
+ 
+ /* Return a copy of FILENAME, with its directory prefix canonicalized
+    by gdb_realpath.  */
+ 
+ char *
+ xfullpath (const char *filename)
+ {
+   const char *base_name = lbasename (filename);
+   char *dir_name;
+   char *real_path;
+   char *result;
+ 
+   /* Extract the basename of filename, and return immediately 
+      a copy of filename if it does not contain any directory prefix. */
+   if (base_name == filename)
+     return xstrdup (filename);
+ 
+   dir_name = alloca ((size_t) (base_name - filename + 2));
+   /* Allocate enough space to store the dir_name + plus one extra
+      character sometimes needed under Windows (see below), and
+      then the closing \000 character */
+   strncpy (dir_name, filename, base_name - filename);
+   dir_name[base_name - filename] = '\000';
+ 
+ #ifdef HAVE_DOS_BASED_FILE_SYSTEM
+   /* We need to be careful when filename is of the form 'd:foo', which
+      is equivalent of d:./foo, which is totally different from d:/foo.  */
+   if (strlen (dir_name) == 2 &&
+       isalpha (dir_name[0]) && dir_name[1] == ':')
+     {
+       dir_name[2] = '.';
+       dir_name[3] = '\000';
+     }
+ #endif
+ 
+   /* Canonicalize the directory prefix, and build the resulting
+      filename. If the dirname realpath already contains an ending
+      directory separator, avoid doubling it.  */
+   real_path = gdb_realpath (dir_name);
+   if (IS_DIR_SEPARATOR (real_path[strlen (real_path) - 1]))
+     result = concat (real_path, base_name, NULL);
+   else
+     result = concat (real_path, SLASH_STRING, base_name, NULL);
+ 
+   xfree (real_path);
+   return result;
  }

Re: [RFA] xfullpath and new regression test xfullpath.exp - take 2

[Eli Zaretskii]

> Date: Fri, 5 Apr 2002 16:43:29 +0200
> From: Joel Brobecker <brobecker@ACT-Europe.FR>
> 
> based on Eli's comments, I reworked the patch I posted in the
> following message:
> 
>      http://sources.redhat.com/ml/gdb-patches/2002-04/msg00000.html
> 
> I reran the regression test on x86-linux and observed no new regression.
> I am not reattaching the xfullpath.exp testcase, since Andrew already
> approved it, but it can be retrieved at the URL above. I also took note
> that this test will be located in the new gdb.gdb directory.
> 
> The patch is attached. I used the isalpha function, which is part of
> ANSI C. I hope this is fine?

Yes, ANSI C is fine.

The patch is approved.  Please commit this.

Re: [RFA] xfullpath and new regression test xfullpath.exp - take 2

[Joel Brobecker]

> The patch is approved.  Please commit this.

Thank you for having reviewed my changes.
Committed.

-- 
Joel

Re: [RFA] xfullpath and new regression test xfullpath.exp - take 2

[Andrew Cagney]

Joel,

The patch contains:

1.60         (brobecke 05-Apr-02):         const char *rp = gdb_realpath 
(symtab_to_filename (s));
1.60         (brobecke 05-Apr-02):         make_cleanup (xfree, rp);

``rp'' shouldn't be const.  -Werror flags this.

Looks like you've fixed heaps of memory leaks!

enjoy,
Andrew

Re: [RFA] xfullpath and new regression test xfullpath.exp - take 2

[Joel Brobecker]

> ``rp'' shouldn't be const.  -Werror flags this.

Ok, I will fix this. I'll make a note of always compiling with -Werror
before sending a patch.

-- 
Joel