Re: Bug in builtin function abspath

[Eli Zaretskii]

> Date: Fri, 26 May 2006 23:39:33 +0200
> From: Andreas =?iso-8859-1?Q?B=FCning?= <address@hidden>
> CC: address@hidden
> 
> > (Yes, I see in the source code that it will not DTRT with "d:/foo" and
> > will not add a drive letter to "/foo", but I wonder whether there are
> > other examples.)
> 
> Exactly. But these are real examples which can happen if some Makefile
> uses commands like pwd or some other make builtins. In the first case
> you will get something like "c:/bar/d:/foo/file.c" which can't work,
> obviously.

Here's the patch I suggest to fix this.  It leaves most of the
original code untouched, except where comparisons with a literal '/'
were used (I use the IS_PATHSEP macro instead).  To handle the drive
letters, I modified the code that copies the root or the current
directory into the destination buffer, and also introduced a variable
that holds the place past which we cannot back up with "..".

Apart of simpler code (IMHO), this patch also has 2 other advantages:

 . It handles the d:foo case by converting it to d:./foo
 . It returns the result with all backslashes converted to forward
   slashes, so $abspath can be used on an already absolute file name
   to convert it to forward slashes, where we previously had to use
   $patsubst.

Please test.  The Makefile I used to test it is this:

FILES1 = foo/bar\\baz /foo/bar/baz \\foo\\bar/baz e:/foo\\bar\\baz e:foo/bar
FILES2 = foo/..\\.\bar ///foo/bar/../baz/frob/../../xyz e:/foo/bar/../../../baz 
e:\\foo\\bar\\baz\\frob

all:
        @echo $(abspath $(FILES1))
        @echo $(abspath $(FILES2))

Here's the patch:

2006-05-27  Eli Zaretskii  <address@hidden>

        * function.c (IS_ABSOLUTE, ROOT_LEN): New macros.
        (abspath): Support systems that define HAVE_DOS_PATHS (have
        drive letters in their file names).  Use IS_PATHSEP instead of a
        literal '/' comparison.


--- function.c~1        2006-05-27 15:58:26.984375000 +0300
+++ function.c  2006-05-27 17:37:47.453125000 +0300
@@ -1890,6 +1890,14 @@ func_not (char *o, char **argv, char *fu
 #endif
 
 
+#ifdef HAVE_DOS_PATHS
+#define IS_ABSOLUTE(n) (n[0] && n[1] == ':')
+#define ROOT_LEN 3
+#else
+#define IS_ABSOLUTE(n) (n[0] == '/')
+#define ROOT_LEN 1
+#endif
+
 /* Return the absolute name of file NAME which does not contain any `.',
    `..' components nor any repeated path separators ('/').   */
 
@@ -1898,13 +1906,14 @@ abspath (const char *name, char *apath)
 {
   char *dest;
   const char *start, *end, *apath_limit;
+  unsigned long root_len = ROOT_LEN;
 
   if (name[0] == '\0' || apath == NULL)
     return NULL;
 
   apath_limit = apath + GET_PATH_MAX;
 
-  if (name[0] != '/')
+  if (!IS_ABSOLUTE(name))
     {
       /* It is unlikely we would make it until here but just to make sure. */
       if (!starting_directory)
@@ -1912,12 +1921,40 @@ abspath (const char *name, char *apath)
 
       strcpy (apath, starting_directory);
 
+#ifdef HAVE_DOS_PATHS
+      if (IS_PATHSEP(name[0]))
+       {
+         /* We have /foo, an absolute file name except for the drive
+            letter.  Assume the missing drive letter is the current
+            drive, which we can get if we remove from starting_directory
+            everything past the root directory.  */
+         apath[root_len] = '\0';
+       }
+#endif
+
       dest = strchr (apath, '\0');
     }
   else
     {
-      apath[0] = '/';
-      dest = apath + 1;
+      strncpy (apath, name, root_len);
+      apath[root_len] = '\0';
+      dest = apath + root_len;
+      /* Get past the root in name, since we already copied it.  */
+      name += root_len;
+#ifdef HAVE_DOS_PATHS
+      if (!IS_PATHSEP(apath[2]))
+       {
+         /* Convert d:foo into d:./foo and increase root_len.  */
+         apath[2] = '.';
+         apath[3] = '/';
+         dest++;
+         root_len++;
+         /* strncpy above copied one character too many.  */
+         name--;
+       }
+      else
+       apath[2] = '/'; /* make sure it's a forward slash */
+#endif
     }
 
   for (start = end = name; *start != '\0'; start = end)
@@ -1925,11 +1962,11 @@ abspath (const char *name, char *apath)
       unsigned long len;
 
       /* Skip sequence of multiple path-separators.  */
-      while (*start == '/')
+      while (IS_PATHSEP(*start))
        ++start;
 
       /* Find end of path component.  */
-      for (end = start; *end != '\0' && *end != '/'; ++end)
+      for (end = start; *end != '\0' && !IS_PATHSEP(*end); ++end)
         ;
 
       len = end - start;
@@ -1941,12 +1978,12 @@ abspath (const char *name, char *apath)
       else if (len == 2 && start[0] == '.' && start[1] == '.')
        {
          /* Back up to previous component, ignore if at root already.  */
-         if (dest > apath + 1)
-           while ((--dest)[-1] != '/');
+         if (dest > apath + root_len)
+           for (--dest; !IS_PATHSEP(dest[-1]); --dest);
        }
       else
        {
-         if (dest[-1] != '/')
+         if (!IS_PATHSEP(dest[-1]))
             *dest++ = '/';
 
          if (dest + len >= apath_limit)
@@ -1959,7 +1996,7 @@ abspath (const char *name, char *apath)
     }
 
   /* Unless it is root strip trailing separator.  */
-  if (dest > apath + 1 && dest[-1] == '/')
+  if (dest > apath + root_len && IS_PATHSEP(dest[-1]))
     --dest;
 
   *dest = '\0';