[RFC] xfullpath and new regression test xfullpath.exp

[RFC] xfullpath and new regression test xfullpath.exp

[Joel Brobecker]

[-- 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
}

Re: [RFC] xfullpath and new regression test xfullpath.exp

[Eli Zaretskii]

On Mon, 1 Apr 2002, Joel Brobecker wrote:

> 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.

Thanks.  However, this patch will not DTRT on DOS and Windows systems, 
I'm afraid.  Details below.

>         * 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.

This kind of detailed description should go into the source, IMHO.  It's 
okay to have it in the ChangeLog as well, but having it _only_ in the 
ChangeLog is not a good idea: people who read the code don't expect to 
find the associated comments in the logs.

+ /*
+  * xfullpath
+  *
+  * Return a copy of FILENAME, with its directory prefix canonicalized
+  * by gdb_realpath.
+  */

I believe this is not the comment style the GNU project uses.

Here are specific comments about the code part of your patch:

+ 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';

This doesn't work correctly when the input name is something like 
"d:foo".  lbasename returns a pointer to `f', so you end up with the 
directory being "d:", and the rest of the code will generate "d:/foo", 
which is something very different in semantics.  The Right Thing to do
in this case is to return either the unmodified "d:foo" or "d:./foo".

+   /* 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);

This doesn't work if the input is "\\", which is also a root directory.
You should use the more portable IS_DIR_SEPARATOR macro (defined on 
include/filenames.h) instead.

Finally, I don't understand why do you avoid duplicate slashes when
dir_name is "/", but not if it is "/foo/bar/".  Why not handle the 
other case as well?  (This issue is a general one, not something specific 
to MS-Windows, although the case of "d:/", the root directory on drive d:, 
was what initially brought that to my attention.)

Re: [RFC] xfullpath and new regression test xfullpath.exp

[Joel Brobecker]

> Thanks.  However, this patch will not DTRT on DOS and Windows systems, 
> I'm afraid.  Details below.

I'm quite ignorant of the DOS and Windows platforms, so I'm not too
surprised, somehow. I thought that since only cygwin-gdb was supported,
I did not have to deal with DOS-like formats. Thanks for correcting me.

> This kind of detailed description should go into the source, IMHO.  It's 
> okay to have it in the ChangeLog as well, but having it _only_ in the 
> ChangeLog is not a good idea: people who read the code don't expect to 
> find the associated comments in the logs.

Right, I will put this in the code as well.

> 
> + /*
> +  * xfullpath
> +  *
> +  * Return a copy of FILENAME, with its directory prefix canonicalized
> +  * by gdb_realpath.
> +  */
> 
> I believe this is not the comment style the GNU project uses.

Is the following style more in line with the GNU project style?

  /* Return a copy of FILENAME, with its directory prefix canonicalized
     by gdb_realpath.  */

> This doesn't work correctly when the input name is something like 
> "d:foo".  lbasename returns a pointer to `f', so you end up with the 
> directory being "d:", and the rest of the code will generate "d:/foo", 
> which is something very different in semantics.  The Right Thing to do
> in this case is to return either the unmodified "d:foo" or "d:./foo".

Ah, Ok, I did not know this, I thought that d:foo and d:/foo were
equivalent. Shouldn't xfullpath return d:<cwd>/foo instead? What do you
think of the following pseudo-code?

   [...]
   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';

   #if defined(__CYGWIN__) || defined(__MINGW32__)
   if (strlen (dir_name) == 2 &&
       is_letter (dir_name[0]) && dir_name[1] == ':')
     {
       dir_name[2] = '.';
       dir_name[3] = '\000';
     }
   #endif

   real_path = gdb_realpath (dir_name);
   [...]

> This doesn't work if the input is "\\", which is also a root directory.
> You should use the more portable IS_DIR_SEPARATOR macro (defined on 
> include/filenames.h) instead.

Cool macro.

> Finally, I don't understand why do you avoid duplicate slashes when
> dir_name is "/", but not if it is "/foo/bar/".  Why not handle the 
> other case as well?  (This issue is a general one, not something specific 
> to MS-Windows, although the case of "d:/", the root directory on drive d:, 
> was what initially brought that to my attention.)

Since I thought that only cygwin-style filenames were supported, I
thought that the the double-slash issue would only happen with the root
directory.

Is this better if I modify the last part of the function to be
(can I consider that SLASH_STRING will always be one character long?):

   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);

Thanks,
-- 
Joel

Re: [RFC] xfullpath and new regression test xfullpath.exp

[Eli Zaretskii]

> Is the following style more in line with the GNU project style?
> 
>   /* Return a copy of FILENAME, with its directory prefix canonicalized
>      by gdb_realpath.  */

Yes.

> Ah, Ok, I did not know this, I thought that d:foo and d:/foo were
> equivalent. Shouldn't xfullpath return d:<cwd>/foo instead? What do you
> think of the following pseudo-code?
> 
>    [...]
>    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';
> 
>    #if defined(__CYGWIN__) || defined(__MINGW32__)
>    if (strlen (dir_name) == 2 &&
>        is_letter (dir_name[0]) && dir_name[1] == ':')
>      {
>        dir_name[2] = '.';
>        dir_name[3] = '\000';
>      }
>    #endif
> 
>    real_path = gdb_realpath (dir_name);

Yes, this is okay, but please don't use system-dependent symbols like
__CYGWIN__ etc. if you can avoid that: we want to avoid
system-dependent preprocessor symbols as much as we can.  In this
case, include/filename.h already defines a non-zero value for the
macro HAVE_DOS_BASED_FILE_SYSTEM on systems that need that special
code.  So please use HAVE_DOS_BASED_FILE_SYSTEM instead.

> Is this better if I modify the last part of the function to be
> 
>    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);

Yes, I think this is better, thanks.

> (can I consider that SLASH_STRING will always be one character long?):

Where does the code assume it's a single character?  `concat' conses
up a new string, right?  If so, the length of SLASH_STRING shouldn't
matter, I think.  Or am I missing something?

Re: [RFC] xfullpath and new regression test xfullpath.exp

[Joel Brobecker]

> So please use HAVE_DOS_BASED_FILE_SYSTEM instead.

Sure, thanks for the tip.

> >    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);
> 
> Yes, I think this is better, thanks.
> 
> > (can I consider that SLASH_STRING will always be one character long?):
> 
> Where does the code assume it's a single character?  `concat' conses
> up a new string, right?  If so, the length of SLASH_STRING shouldn't
> matter, I think.  Or am I missing something?

I implicitely make this assumption when I use the IS_DIR_SEPARATOR
macro to check the value of the last character in the real_path string,
and then use SLASH_STRING during the concat.

If all is fine, I'll resubmit a new patch shortly.

-- 
Joel

Re: [RFC] xfullpath and new regression test xfullpath.exp

[Andrew Cagney]

> 2002-03-30  J. Brobecker  <brobecker@gnat.com>
> 
> * gdb.base/xfullpath.exp: New test, to exercise the new
>         xfullpath () function.
> 

Yes! This looks good (but in the *NEW* directory) gdb.gdb!

I think I'll let Eli approve xfullpath() proper.

thanks!
Andrew

Re: [RFC] xfullpath and new regression test xfullpath.exp

[Eli Zaretskii]

> Date: Wed, 3 Apr 2002 15:19:09 +0200
> From: Joel Brobecker <brobecker@ACT-Europe.FR>
> 
> > >    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);
> > 
> > Yes, I think this is better, thanks.
> > 
> > > (can I consider that SLASH_STRING will always be one character long?):
> > 
> > Where does the code assume it's a single character?  `concat' conses
> > up a new string, right?  If so, the length of SLASH_STRING shouldn't
> > matter, I think.  Or am I missing something?
> 
> I implicitely make this assumption when I use the IS_DIR_SEPARATOR
> macro to check the value of the last character in the real_path string,
> and then use SLASH_STRING during the concat.

You don't need to worry about that: it's IS_DIR_SEPARATOR's job to
DTRT for each platform, even if the directory separator is not a
single character.

> If all is fine, I'll resubmit a new patch shortly.

Please do.  Thanks.