This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] cygcheck.cc update for cygpath()


Corinna Vinschen wrote:

> Yuk.  I guess it would help a lot to reproduce path.cc:check_shortcut(*)
> in utils as close as possible.  Afaics it doesn't use any code which
> would be restricted to Cygwin, except for the call to
> mount_table->conv_to_posix_path in the posixify method.

I started down that road, trying to come up with a slick method for
sharing the code so that we don't need to maintain two copies.  After
poking at it for a while I came to the realization that the two are just
plumbed too differently to share code directly.  In the Cygwin code, the
"check if this is a valid symlink" and the "read its contents" are kind
of lumped together as the symlink_info class provides a convenient
location to store the data.  But in the cygcheck implementation we have
only is_symlink() and readlink(), with both take just a HANDLE and share
nothing, and no pflags to worry about, etc.

And of course there's the obvious that cygcheck has no Win32->POSIX
conversion capability which makes it hard to support reading reparse
points because readlink() is supposed to return the POSIX link target.

Anyway, the attached is the result of what happened when I started with
the Cygwin code and whittled it down.  It fixes the bug in the
grandparent of this email where it was reading the Win32 path out of a
shortcut that didn't have an ITEMIDLIST, and it supports the new-style
shortcut where the target > MAX_PATH gets stored at the end.  It does
not attempt to do anything with reparse points however.

Another factor was that the file IO in symlink_info::check_shortcut was
using the native API, which I rewrote to use the plain Win32 flavor in
case we want to keep cygcheck working on 9x/ME.  I don't think this will
matter if we want to make cygcheck support long paths though, since it's
just ReadFile, SetFilePointer, and GetFileInformationByHandle -- the
HANDLE is already open so this should require no change to support
WCHAR.

Brian
2008-03-15  Brian Dessent  <brian@dessent.net>

	* path.cc: Include malloc.h for alloca.
	(is_symlink): Rewrite.  Just read the whole file in memory rather
	than by parts.  Account for an ITEMIDLIST if present, as well as
	the new style of Cygwin shortcut supporting targets > MAX_PATH.

 path.cc |   96 +++++++++++++++++++++++++++++++---------------------------------
 1 file changed, 47 insertions(+), 49 deletions(-)

Index: path.cc
===================================================================
RCS file: /cvs/src/src/winsup/utils/path.cc,v
retrieving revision 1.14
diff -u -p -r1.14 path.cc
--- path.cc	11 Mar 2008 17:20:02 -0000	1.14
+++ path.cc	15 Mar 2008 13:12:45 -0000
@@ -18,6 +18,7 @@ details. */
 #include <windows.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <malloc.h>
 #include "path.h"
 #include "cygwin/include/cygwin/version.h"
 #include "cygwin/include/sys/mount.h"
@@ -172,60 +173,57 @@ is_symlink (HANDLE fh)
 bool
 readlink (HANDLE fh, char *path, int maxlen)
 {
-  int got;
-  int magic = get_word (fh, 0x0);
+  DWORD rv;
+  char *buf, *cp;
+  unsigned short len;
+  win_shortcut_hdr *file_header;
+  BY_HANDLE_FILE_INFORMATION fi;
+
+  if (!GetFileInformationByHandle (fh, &fi)
+      || fi.nFileSizeHigh != 0
+      || fi.nFileSizeLow > 8192)
+    return false;
 
-  if (magic == SHORTCUT_MAGIC)
-    {
-      int offset = get_word (fh, 0x4c);
-      int slen = get_word (fh, 0x4c + offset + 2);
-      if (slen >= maxlen)
-	{
-	  SetLastError (ERROR_FILENAME_EXCED_RANGE);
-	  return false;
-	}
-      if (SetFilePointer (fh, 0x4c + offset + 4, 0, FILE_BEGIN) ==
-	  INVALID_SET_FILE_POINTER && GetLastError () != NO_ERROR)
-	return false;
+  buf = (char *) alloca (fi.nFileSizeLow + 1);
+  file_header = (win_shortcut_hdr *) buf;
 
-      if (!ReadFile (fh, path, slen, (DWORD *) &got, 0))
-	return false;
-      else if (got < slen)
-	{
-	  SetLastError (ERROR_READ_FAULT);
-	  return false;
-	}
-      else
-	path[got] = '\0';
-    }
-  else if (magic == SYMLINK_MAGIC)
+  if (SetFilePointer (fh, 0L, NULL, FILE_BEGIN) == INVALID_SET_FILE_POINTER
+      || !ReadFile (fh, buf, fi.nFileSizeLow, &rv, NULL)
+      || rv != fi.nFileSizeLow)
+    return false;
+  
+  if (fi.nFileSizeLow > sizeof (file_header)
+      && cmp_shortcut_header (file_header))
     {
-      char cookie_buf[sizeof (SYMLINK_COOKIE) - 1];
-
-      if (SetFilePointer (fh, 0, 0, FILE_BEGIN) == INVALID_SET_FILE_POINTER
-	  && GetLastError () != NO_ERROR)
-	return false;
-
-      if (!ReadFile (fh, cookie_buf, sizeof (cookie_buf), (DWORD *) &got, 0))
-	return false;
-      else if (got == sizeof (cookie_buf)
-	       && memcmp (cookie_buf, SYMLINK_COOKIE, sizeof (cookie_buf)) == 0)
-	{
-	  if (!ReadFile (fh, path, maxlen, (DWORD *) &got, 0))
-	    return false;
-	  else if (got >= maxlen)
-	    {
-	      SetLastError (ERROR_FILENAME_EXCED_RANGE);
-	      path[0] = '\0';
-	      return false;
-	    }
-	  else
-	    path[got] = '\0';
-	}
-    }
+      cp = buf + sizeof (win_shortcut_hdr);
+      if (file_header->flags & WSH_FLAG_IDLIST) /* Skip ITEMIDLIST */
+        cp += *(unsigned short *) cp + 2;
+      if (!(len = *(unsigned short *) cp))
+        return false;
+      cp += 2;
+      /* Has appended full path?  If so, use it instead of description. */
+      unsigned short relpath_len = *(unsigned short *) (cp + len);
+      if (cp + len + 2 + relpath_len < buf + fi.nFileSizeLow)
+        {
+          cp += len + 2 + relpath_len;
+          len = *(unsigned short *) cp;
+          cp += 2;
+        }
+      if (len + 1 > maxlen)
+        return false;
+      memcpy (path, cp, len);
+      path[len] = '\0';
+      return true;
+    }
+  else if (strncmp (buf, SYMLINK_COOKIE, strlen (SYMLINK_COOKIE)) == 0
+           && fi.nFileSizeLow - strlen (SYMLINK_COOKIE) <= (unsigned) maxlen
+           && buf[fi.nFileSizeLow - 1] == '\0')
+    {
+      strcpy (path, &buf[strlen (SYMLINK_COOKIE)]);
+      return true;
+    }      
   else
     return false;
-  return true;
 }
 
 typedef struct mnt

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]