[RFC] xfullpath and new regression test xfullpath.exp

[Joel Brobecker <brobecker@ACT-Europe.FR>]

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

Hello,

I implemented the new xfullpath function based on Andrew's comments, and
made some adjustements to fix the problem reported in

    http://sources.redhat.com/ml/gdb-patches/2002-03/msg00345.html

while being careful not to break Tom's setup. The patch is attached.
I also implemented a new test for it (see attached: xfullpath.exp).

I verified that it does not introduce any regression on x86-linux.

I have a question: I replaced some calls to xfree by a call to
make_cleanup, because there were several exit paths in the procedure.
Using make_cleanup is probably less efficient, but it makes the code
more robust (less likely to have a memory leak), and easier to read. Is
this fine?

ChangeLog entry for gdb:

2002-03-30  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-03-30  J. Brobecker  <brobecker@gnat.com>

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

-- 
Joel

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

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/03/31 23:00:08
*************** gdb_realpath (const char *filename)
*** 2560,2562 ****
--- 2560,2599 ----
    return xstrdup (filename);
  #endif
  }
+ 
+ /*
+  * xfullpath
+  *
+  * 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 + 1));
+   strncpy (dir_name, filename, base_name - filename);
+   dir_name[base_name - filename] = '\000';
+ 
+   /* Canonicalize the directory prefix, and build the resulting
+      filename. Avoid returning a path which starts with 2 SLASH_CHARS
+      if the canonicalized path is the root path */
+   real_path = gdb_realpath (dir_name);
+   if (strcmp (real_path, SLASH_STRING) == 0)
+     result = concat (SLASH_STRING, base_name, NULL);
+   else
+     result = concat (real_path, SLASH_STRING, base_name, NULL);
+ 
+   xfree (real_path);
+   return result;
+ }
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/03/31 22:59:53
*************** 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/03/31 22:59:57
*************** done:
*** 612,618 ****
        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... */
--- 612,618 ----
        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);
  	}
      }
--- 621,627 ----
             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/03/31 23:00:04
*************** 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) */

[-- Attachment #3: xfullpath.exp --]
[-- Type: text/plain, Size: 6246 bytes --]

#   Copyright 2002
#   Free Software Foundation, Inc.

# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 2 of the License, or
# (at your option) any later version.
# 
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
# 
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  

# Please email any bugs, comments, and/or additions to this file to:
# bug-gdb@prep.ai.mit.edu

# This file was written by Joel Brobecker. (brobecker@gnat.com), derived
# from selftest.exp, written by Rob Savoye.

if $tracelevel then {
    strace $tracelevel
}

set prms_id 0
set bug_id 0

# are we on a target board
if [is_remote target] {
    return
}

if [istarget "m68k*-*-hpux*"] then {
    # The top-level makefile passes CFLAGS= (no -g) for hp300.  This probably
    # should be fixed (it is only needed for gcc bootstrapping, not gdb),
    # but until then.....
    setup_xfail "*-*-*"
    fail "cannot test self if compiled without debug info"
    return -1
}

proc setup_test { executable } {
    global gdb_prompt
    global timeout

    # load yourself into the debugger
    # This can take a relatively long time, particularly for testing where
    # the executable is being accessed over a network, or where gdb does not
    # support partial symbols for a particular target and has to load the
    # entire symbol table.  Set the timeout to 10 minutes, which should be
    # adequate for most environments (it *has* timed out with 5 min on a
    # SPARCstation SLC under moderate load, so this isn't unreasonable).
    # After gdb is started, set the timeout to 30 seconds for the duration
    # of this test, and then back to the original value.

    set oldtimeout $timeout
    set timeout 600
    verbose "Timeout is now $timeout seconds" 2
    if {[gdb_load $executable] <0} then {
	set timeout $oldtimeout
	verbose "Timeout is now $timeout seconds" 2
	return -1
    }
    set timeout $oldtimeout
    verbose "Timeout is now $timeout seconds" 2

    # Set a breakpoint at main
    gdb_test "break captured_main" \
            "Breakpoint.*at.* file.*, line.*" \
            "breakpoint in captured_main"

    # run yourself
    # It may take a very long time for the inferior gdb to start (lynx),
    # so we bump it back up for the duration of this command.
    set timeout 600

    set description "run until breakpoint at captured_main"
    send_gdb "run -nw\n"
    gdb_expect {
        -re "Starting program.*Breakpoint \[0-9\]+,.*captured_main .data.* at .*main.c:.*$gdb_prompt $" {
            pass "$description"
        }
        -re "Starting program.*Breakpoint \[0-9\]+,.*captured_main .data.*$gdb_prompt $" {
            xfail "$description (line numbers scrambled?)"
        }
        -re "vfork: No more processes.*$gdb_prompt $" {
            fail "$description (out of virtual memory)"
            set timeout $oldtimeout
            verbose "Timeout is now $timeout seconds" 2
            return -1
        }
        -re ".*$gdb_prompt $" {
            fail "$description"
            set timeout $oldtimeout
            verbose "Timeout is now $timeout seconds" 2
            return -1
        }
        timeout {
            fail "$description (timeout)"
        }
    }

    set timeout $oldtimeout
    verbose "Timeout is now $timeout seconds" 2

    return 0
}

proc test_with_self { executable } {

    set setup_result [setup_test $executable]
    if {$setup_result <0} then {
        return -1
    }

    # A file which contains a directory prefix
    gdb_test "print xfullpath (\"./xfullpath.exp\")" \
             ".\[0-9\]+ =.*\".*/xfullpath.exp\"" \
             "A filename with ./ as the directory prefix"

    # A file which contains a directory prefix
    gdb_test "print xfullpath (\"../../defs.h\")" \
             ".\[0-9\]+ =.*\".*/defs.h\"" \
             "A filename with ../ in the directory prefix"

    # A one-character filename
    gdb_test "print xfullpath (\"./a\")" \
             ".\[0-9\]+ =.*\".*/a\"" \
             "A one-char filename in the current directory"
          
    # A file in the root directory
    gdb_test "print xfullpath (\"/root_file_which_should_exist\")" \
             ".\[0-9\]+ =.*\"/root_file_which_should_exist\"" \
             "A filename in the root directory"
          
    # A file which does not have a directory prefix
    gdb_test "print xfullpath (\"xfullpath.exp\")" \
             ".\[0-9\]+ =.*\"xfullpath.exp\"" \
             "A filename without any directory prefix"

    # A one-char filename without any directory prefix
    gdb_test "print xfullpath (\"a\")" \
             ".\[0-9\]+ =.*\"a\"" \
             "A one-char filename without any directory prefix"

    # An empty filename
    gdb_test "print xfullpath (\"\")" \
             ".\[0-9\]+ =.*\"\"" \
             "An empty filename"

    return 0
}

# Find a pathname to a file that we would execute if the shell was asked
# to run $arg using the current PATH.

proc find_gdb { arg } {

    # If the arg directly specifies an existing executable file, then
    # simply use it.

    if [file executable $arg] then {
	return $arg
    }

    set result [which $arg]
    if [string match "/" [ string range $result 0 0 ]] then {
	return $result
    }

    # If everything fails, just return the unqualified pathname as default
    # and hope for best.

    return $arg
}

# Run the test with self.
# Copy the file executable file in case this OS doesn't like to edit its own
# text space.

set GDB_FULLPATH [find_gdb $GDB]

# Remove any old copy lying around.
remote_file host delete x$tool

gdb_start
set file [remote_download host $GDB_FULLPATH x$tool]
set result [test_with_self $file];
gdb_exit;
catch "remote_file host delete $file";

if {$result <0} then {
    warning "Couldn't test self"
    return -1
}