[PATCH] cygcheck: follow symbolic links

Igor Peshansky pechtcha@cs.nyu.edu
Thu Sep 28 13:06:00 GMT 2006


On Mon, 24 Jul 2006, Corinna Vinschen wrote:

> On Jul 21 20:52, Igor Peshansky wrote:
> > In any case, here's the latest incarnation, with get_word and get_dword
> > folded into path.cc, and display_error returned to cygcheck.cc, where it
> > belongs.  Tested reasonably well (with symlinks pointing to symlinks,
> > etc).  I'll let you judge the neatness of the ChangeLog entry.  If I'm
> > lucky, this might just get into 1.5.21[*].
> > 	Igor
> > [*] Corinna, I'm guessing this is sufficiently different that you can't
> > accept it without "the fax" -- I'll keep pinging the guy who's holding
> > this up, but this message is also supposed to confirm that there is a
> > working patch, and the delay is simply bureaucratic.  Oh, the
> > frustration...  If you judge the changes from the previous incarnation
> > to not be significant, just go ahead and apply this, given the previous
> > approval.
>
> The latest fax was about this change, so I think this should still be
> covered, shouldn't it?  Ping the guy nevertheless.  We should stay on
> the safe side in legal questions.
>
> I'd be happy to apply the patch, but it would be nice if you could tweak
> the formatting somewhat:
>
> > +  if (GetLastError () != NO_ERROR) display_error ("get_dword");
>
> The display_error call should be on its own line, as usual.  This
> happens multiple times in your patch.
>
> > +  if (is_exe (fh))
> > +    dll_info (path, fh, lvl, 1);
> > +  else if (is_symlink (fh))
> > +    display_error ("track_down: Huh?  Got a symlink!");
>
> Is that really the supposed message here?
>
> > +      printf (" - Not a DLL: magic number %x (%d) '%s'\n", magic, magic, (char *)&magic);
>
> Please split the printf so that it's not longer than 80 chars.
>
> > +      /* TODO: check for invalid path contents (see ../cygwin/path.cc:3313 */
>
> Since source code lines are most volatile, I'd not refer to a line number
> in another source code.  Just mention the function name. */
>
> > +      if (got != sizeof (buf) || memcmp (buf, SYMLINK_COOKIE, sizeof (buf)) != 0)
>
> Split the line, please.
>
> > +      if (SetFilePointer (fh, 0x4c + offset + 4, 0, FILE_BEGIN) == INVALID_SET_FILE_POINTER
>
> Same here.
>
> > +	{
> > +	  return false;
> > +	}
>
> I'd rather not have these one liners in curly brackets.  It's a bit
> irritating since sometimes you put them in curly brackets, sometimes you
> don't.
>
> The code looks ok, otherwise.

Looks like I've been remiss in following up on this, though I regenerated
the patch that same day.  Attached is the new version of the patch.  I
believe "the fax" (the new one) has been sent, but I've received no
notification of that, presumably because Corinna is not around...
	Igor
==============================================================================
ChangeLog:
2006-09-28  Igor Peshansky  <pechtcha@cs.nyu.edu>

	* cygcheck.cc (get_word, get_dword): Move to path.cc.
	(LINK_EXTENSION): New macro.
	(check_existence): New static function.
	(find_on_path): Check for symbolic links if asked.
	(dll_info): New error handling.
	(track_down): Only call dll_info() for executables, display
	an error for symlinks, and print magic number for others.
	(find_app_on_path): New static function.
	(cygcheck, dump_sysinfo): Call find_app_on_path() instead of
	find_on_path().
	* path.cc (cmp_shortcut_header): New static function.
	(get_word, get_dword): Moved from cygcheck.cc.
	(EXE_MAGIC, SHORTCUT_MAGIC, SYMLINK_COOKIE, SYMLINK_MAGIC): New
	macros.
	(is_exe, is_symlink, readlink): New functions.
	* path.h (is_exe, is_symlink, readlink): Declare.
	(get_word, get_dword): Ditto.

-- 
				http://cs.nyu.edu/~pechtcha/
      |\      _,,,---,,_	    pechtcha@cs.nyu.edu | igor@watson.ibm.com
ZZZzz /,`.-'`'    -.  ;-;;,_		Igor Peshansky, Ph.D. (name changed!)
     |,4-  ) )-,_. ,\ (  `'-'		old name: Igor Pechtchanski
    '---''(_/--'  `-'\_) fL	a.k.a JaguaR-R-R-r-r-r-.-.-.  Meow!

"Las! je suis sot... -Mais non, tu ne l'es pas, puisque tu t'en rends compte."
"But no -- you are no fool; you call yourself a fool, there's proof enough in
that!" -- Rostand, "Cyrano de Bergerac"
-------------- next part --------------
Index: winsup/utils/cygcheck.cc
===================================================================
RCS file: /cvs/src/src/winsup/utils/cygcheck.cc,v
retrieving revision 1.90
diff -u -p -r1.90 cygcheck.cc
--- winsup/utils/cygcheck.cc	8 Feb 2006 14:19:40 -0000	1.90
+++ winsup/utils/cygcheck.cc	24 Jul 2006 17:52:15 -0000
@@ -249,9 +249,41 @@ init_paths ()
     }
 }
 
+#define LINK_EXTENSION ".lnk"
+
+static bool
+check_existence (char *file, int showall, int foundone, char *first)
+{
+  if (GetFileAttributes (file) != (DWORD) - 1)
+    {
+      char *lastdot = strrchr (file, '.');
+      bool is_link = lastdot && !strcmp (lastdot, LINK_EXTENSION);
+      // If file is a link, fix up the extension before printing
+      if (is_link)
+	*lastdot = '\0';
+      if (showall)
+	printf ("Found: %s\n", file);
+      if (foundone)
+	{
+	  char *flastdot = strrchr (first, '.');
+	  bool f_is_link = flastdot && !strcmp (flastdot, LINK_EXTENSION);
+	  // if first is a link, fix up the extension before printing
+	  if (f_is_link)
+	    *flastdot = '\0';
+	  printf ("Warning: %s hides %s\n", first, file);
+	  if (f_is_link)
+	    *flastdot = '.';
+	}
+      if (is_link)
+	*lastdot = '.';
+      return true;
+    }
+  return false;
+}
+
 static char *
 find_on_path (char *file, char *default_extension,
-	      int showall = 0, int search_sysdirs = 0)
+	      int showall = 0, int search_sysdirs = 0, int checklinks = 0)
 {
   static char rv[4000];
   char tmp[4000], *ptr = rv;
@@ -270,11 +302,21 @@ find_on_path (char *file, char *default_
 
   if (strchr (file, ':') || strchr (file, '\\') || strchr (file, '/'))
     {
+      // FIXME: this will find "foo" before "foo.exe" -- contrary to Windows
       char *fn = cygpath (file, NULL);
       if (access (fn, F_OK) == 0)
 	return fn;
       strcpy (rv, fn);
       strcat (rv, default_extension);
+      if (access (rv, F_OK) == 0)
+	return rv;
+      if (!checklinks)
+	return fn;
+      strcat (rv, LINK_EXTENSION);
+      if (access (rv, F_OK) == 0)
+	return rv;
+      strcpy (rv, fn);
+      strcat (rv, LINK_EXTENSION);
       return access (rv, F_OK) == 0 ? strdup (rv) : fn;
     }
 
@@ -286,14 +328,25 @@ find_on_path (char *file, char *default_
       if (i == 0 || !search_sysdirs || strcasecmp (paths[i], paths[0]))
 	{
 	  sprintf (ptr, "%s\\%s%s", paths[i], file, default_extension);
-	  if (GetFileAttributes (ptr) != (DWORD) - 1)
-	    {
-	      if (showall)
-		printf ("Found: %s\n", ptr);
-	      if (ptr == tmp && verbose)
-		printf ("Warning: %s hides %s\n", rv, ptr);
-	      ptr = tmp;
-	    }
+	  if (check_existence (ptr, showall, ptr == tmp && verbose, rv))
+	    ptr = tmp;
+
+	  if (!checklinks)
+	    continue;
+
+	  sprintf (ptr, "%s\\%s%s%s", paths[i], file, default_extension, LINK_EXTENSION);
+	  if (check_existence (ptr, showall, ptr == tmp && verbose, rv))
+	    ptr = tmp;
+
+	  if (!*default_extension)
+	    continue;
+
+	  sprintf (ptr, "%s\\%s", paths[i], file);
+	  if (check_existence (ptr, showall, ptr == tmp && verbose, rv))
+	    ptr = tmp;
+	  sprintf (ptr, "%s\\%s%s", paths[i], file, LINK_EXTENSION);
+	  if (check_existence (ptr, showall, ptr == tmp && verbose, rv))
+	    ptr = tmp;
 	}
     }
 
@@ -330,38 +383,6 @@ already_did (char *file)
   return d;
 }
 
-static int
-get_word (HANDLE fh, int offset)
-{
-  short rv;
-  unsigned r;
-
-  if (SetFilePointer (fh, offset, 0, FILE_BEGIN) == INVALID_SET_FILE_POINTER
-      && GetLastError () != NO_ERROR)
-    display_error ("get_word: SetFilePointer()");
-
-  if (!ReadFile (fh, &rv, 2, (DWORD *) &r, 0))
-    display_error ("get_word: Readfile()");
-
-  return rv;
-}
-
-static int
-get_dword (HANDLE fh, int offset)
-{
-  int rv;
-  unsigned r;
-
-  if (SetFilePointer (fh, offset, 0, FILE_BEGIN) == INVALID_SET_FILE_POINTER
-      && GetLastError () != NO_ERROR)
-    display_error ("get_dword: SetFilePointer()");
-
-  if (!ReadFile (fh, &rv, 4, (DWORD *) &r, 0))
-    display_error ("get_dword: Readfile()");
-
-  return rv;
-}
-
 struct Section
 {
   char name[8];
@@ -505,6 +526,8 @@ dll_info (const char *path, HANDLE fh, i
   DWORD junk;
   int i;
   int pe_header_offset = get_dword (fh, 0x3c);
+  if (GetLastError () != NO_ERROR)
+    display_error ("get_dword");
   int opthdr_ofs = pe_header_offset + 4 + 20;
   unsigned short v[6];
 
@@ -528,12 +551,24 @@ dll_info (const char *path, HANDLE fh, i
     printf ("\n");
 
   int num_entries = get_dword (fh, opthdr_ofs + 92);
+  if (GetLastError () != NO_ERROR)
+    display_error ("get_dword");
   int export_rva = get_dword (fh, opthdr_ofs + 96);
+  if (GetLastError () != NO_ERROR)
+    display_error ("get_dword");
   int export_size = get_dword (fh, opthdr_ofs + 100);
+  if (GetLastError () != NO_ERROR)
+    display_error ("get_dword");
   int import_rva = get_dword (fh, opthdr_ofs + 104);
+  if (GetLastError () != NO_ERROR)
+    display_error ("get_dword");
   int import_size = get_dword (fh, opthdr_ofs + 108);
+  if (GetLastError () != NO_ERROR)
+    display_error ("get_dword");
 
   int nsections = get_word (fh, pe_header_offset + 4 + 2);
+  if (nsections == -1)
+    display_error ("get_word");
   char *sections = (char *) malloc (nsections * 40);
 
   if (SetFilePointer (fh, pe_header_offset + 4 + 20 +
@@ -682,7 +717,20 @@ track_down (char *file, char *suffix, in
 
   d->state = DID_ACTIVE;
 
-  dll_info (path, fh, lvl, 1);
+  if (is_exe (fh))
+    dll_info (path, fh, lvl, 1);
+  else if (is_symlink (fh))
+    printf (" - Found a symlink instead of a DLL\n");
+  else
+    {
+      int magic = get_word (fh, 0x0);
+      if (magic == -1)
+        display_error ("get_word");
+      magic &= 0x00FFFFFF;
+      printf (" - Not a DLL: magic number %x (%d) '%s'\n",
+              magic, magic, (char *)&magic);
+    }
+
   d->state = DID_INACTIVE;
   if (!CloseHandle (fh))
     display_error ("track_down: CloseHandle()");
@@ -711,11 +759,56 @@ ls (char *f)
     display_error ("ls: CloseHandle()");
 }
 
+// Find a real application on the path (possibly following symlinks)
+static char *
+find_app_on_path (char *app, int showall = 0)
+{
+  char *papp = find_on_path (app, (char *) ".exe", showall, 0, 1);
+
+  HANDLE fh =
+    CreateFile (papp, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE,
+		NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
+  if (fh == INVALID_HANDLE_VALUE)
+    {
+      printf (" - Cannot open\n");
+      return NULL;
+    }
+
+  if (is_symlink (fh))
+    {
+      static char tmp[4000] = "";
+      char *ptr;
+      if (!readlink (fh, tmp, 3999))
+	display_error("readlink failed");
+      ptr = cygpath (tmp, NULL);
+      for (char *p = ptr; (p = strchr (p, '/')); p++)
+	*p = '\\';
+      printf (" -> %s\n", ptr);
+      if (!strchr (ptr, '\\'))
+	{
+	  char *lastsep;
+	  strncpy (tmp, cygpath (papp, NULL), 3999);
+	  for (char *p = tmp; (p = strchr (p, '/')); p++)
+	    *p = '\\';
+	  lastsep = strrchr (tmp, '\\');
+	  strncpy (lastsep+1, ptr, 3999-(lastsep-tmp));
+	  ptr = tmp;
+	}
+      if (!CloseHandle (fh))
+	display_error ("find_app_on_path: CloseHandle()");
+      return find_app_on_path (ptr, showall);
+    }
+
+  if (!CloseHandle (fh))
+    display_error ("find_app_on_path: CloseHandle()");
+  return papp;
+}
+
 // Return true on success, false if error printed
 static bool
 cygcheck (char *app)
 {
-  char *papp = find_on_path (app, (char *) ".exe", 1, 0);
+  char *papp = find_app_on_path (app, 1);
   if (!papp)
     {
       printf ("Error: could not find %s\n", app);
@@ -1441,7 +1534,7 @@ dump_sysinfo ()
     printf
       ("Looking to see where common programs can be found, if at all...\n");
   for (i = 0; common_apps[i].name; i++)
-    if (!find_on_path ((char *) common_apps[i].name, (char *) ".exe", 1, 0))
+    if (!find_app_on_path ((char *) common_apps[i].name, 1))
       {
 	if (common_apps[i].missing_is_good)
 	  printf ("Not Found: %s (good!)\n", common_apps[i].name);
Index: winsup/utils/path.cc
===================================================================
RCS file: /cvs/src/src/winsup/utils/path.cc,v
retrieving revision 1.9
diff -u -p -r1.9 path.cc
--- winsup/utils/path.cc	29 Apr 2005 16:39:34 -0000	1.9
+++ winsup/utils/path.cc	24 Jul 2006 17:52:15 -0000
@@ -9,14 +9,16 @@ Cygwin license.  Please consult the file
 details. */
 
 /* The purpose of this file is to hide all the details about accessing
-   Cygwin's mount table.  If the format or location of the mount table
-   changes, this is the file to change to match it. */
+   Cygwin's mount table, shortcuts, etc.  If the format or location of
+   the mount table, or the shortcut format changes, this is the file to
+   change to match it. */
 
 #define str(a) #a
 #define scat(a,b) str(a##b)
 #include <windows.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include "path.h"
 #include "cygwin/include/cygwin/version.h"
 #include "cygwin/include/sys/mount.h"
 #include "cygwin/include/mntent.h"
@@ -29,6 +31,202 @@ details. */
    })
 
 
+static const GUID GUID_shortcut
+			= { 0x00021401L, 0, 0, 0xc0, 0, 0, 0, 0, 0, 0, 0x46 };
+
+enum {
+  WSH_FLAG_IDLIST = 0x01,	/* Contains an ITEMIDLIST. */
+  WSH_FLAG_FILE = 0x02,		/* Contains a file locator element. */
+  WSH_FLAG_DESC = 0x04,		/* Contains a description. */
+  WSH_FLAG_RELPATH = 0x08,	/* Contains a relative path. */
+  WSH_FLAG_WD = 0x10,		/* Contains a working dir. */
+  WSH_FLAG_CMDLINE = 0x20,	/* Contains command line args. */
+  WSH_FLAG_ICON = 0x40		/* Contains a custom icon. */
+};
+
+struct win_shortcut_hdr
+  {
+    DWORD size;		/* Header size in bytes.  Must contain 0x4c. */
+    GUID magic;		/* GUID of shortcut files. */
+    DWORD flags;	/* Content flags.  See above. */
+
+    /* The next fields from attr to icon_no are always set to 0 in Cygwin
+       and U/Win shortcuts. */
+    DWORD attr;	/* Target file attributes. */
+    FILETIME ctime;	/* These filetime items are never touched by the */
+    FILETIME mtime;	/* system, apparently. Values don't matter. */
+    FILETIME atime;
+    DWORD filesize;	/* Target filesize. */
+    DWORD icon_no;	/* Icon number. */
+
+    DWORD run;		/* Values defined in winuser.h. Use SW_NORMAL. */
+    DWORD hotkey;	/* Hotkey value. Set to 0.  */
+    DWORD dummy[2];	/* Future extension probably. Always 0. */
+  };
+
+static bool
+cmp_shortcut_header (win_shortcut_hdr *file_header)
+{
+  /* A Cygwin or U/Win shortcut only contains a description and a relpath.
+     Cygwin shortcuts also might contain an ITEMIDLIST. The run type is
+     always set to SW_NORMAL. */
+  return file_header->size == sizeof (win_shortcut_hdr)
+      && !memcmp (&file_header->magic, &GUID_shortcut, sizeof GUID_shortcut)
+      && (file_header->flags & ~WSH_FLAG_IDLIST)
+	 == (WSH_FLAG_DESC | WSH_FLAG_RELPATH)
+      && file_header->run == SW_NORMAL;
+}
+
+int
+get_word (HANDLE fh, int offset)
+{
+  unsigned short rv;
+  unsigned r;
+
+  SetLastError(NO_ERROR);
+  if (SetFilePointer (fh, offset, 0, FILE_BEGIN) == INVALID_SET_FILE_POINTER
+      && GetLastError () != NO_ERROR)
+    return -1;
+
+  if (!ReadFile (fh, &rv, 2, (DWORD *) &r, 0))
+    return -1;
+
+  return rv;
+}
+
+/*
+ * Check the value of GetLastError() to find out whether there was an error.
+ */
+int
+get_dword (HANDLE fh, int offset)
+{
+  int rv;
+  unsigned r;
+
+  SetLastError(NO_ERROR);
+  if (SetFilePointer (fh, offset, 0, FILE_BEGIN) == INVALID_SET_FILE_POINTER
+      && GetLastError () != NO_ERROR)
+    return -1;
+
+  if (!ReadFile (fh, &rv, 4, (DWORD *) &r, 0))
+    return -1;
+
+  return rv;
+}
+
+#define EXE_MAGIC ((int)*(unsigned short *)"MZ")
+#define SHORTCUT_MAGIC ((int)*(unsigned short *)"L\0")
+#define SYMLINK_COOKIE "!<symlink>"
+#define SYMLINK_MAGIC ((int)*(unsigned short *)SYMLINK_COOKIE)
+
+bool
+is_exe (HANDLE fh)
+{
+  int magic = get_word (fh, 0x0);
+  return magic == EXE_MAGIC;
+}
+
+bool
+is_symlink (HANDLE fh)
+{
+  int magic = get_word (fh, 0x0);
+  if (magic != SHORTCUT_MAGIC && magic != SYMLINK_MAGIC)
+    return false;
+  DWORD got;
+  BY_HANDLE_FILE_INFORMATION local;
+  if (!GetFileInformationByHandle (fh, &local))
+    return false;
+  if (magic == SHORTCUT_MAGIC)
+    {
+      DWORD size;
+      if (!local.dwFileAttributes & FILE_ATTRIBUTE_READONLY)
+	return false; /* Not a Cygwin symlink. */
+      if ((size = GetFileSize (fh, NULL)) > 8192)
+	return false; /* Not a Cygwin symlink. */
+      char buf[size];
+      SetFilePointer (fh, 0, 0, FILE_BEGIN);
+      if (!ReadFile (fh, buf, size, &got, 0))
+	return false;
+      if (got != size || !cmp_shortcut_header ((win_shortcut_hdr *) buf))
+	return false; /* Not a Cygwin symlink. */
+      /* TODO: check for invalid path contents
+         (see symlink_info::check() in ../cygwin/path.cc) */
+    }
+  else /* magic == SYMLINK_MAGIC */
+    {
+      if (!local.dwFileAttributes & FILE_ATTRIBUTE_SYSTEM)
+	return false; /* Not a Cygwin symlink. */
+      char buf[sizeof (SYMLINK_COOKIE) - 1];
+      SetFilePointer (fh, 0, 0, FILE_BEGIN);
+      if (!ReadFile (fh, buf, sizeof (buf), &got, 0))
+	return false;
+      if (got != sizeof (buf) ||
+          memcmp (buf, SYMLINK_COOKIE, sizeof (buf)) != 0)
+	return false; /* Not a Cygwin symlink. */
+    }
+  return true;
+}
+
+/* Assumes is_symlink(fh) is true */
+bool
+readlink (HANDLE fh, char *path, int maxlen)
+{
+  int got;
+  int magic = get_word (fh, 0x0);
+
+  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;
+
+      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)
+    {
+      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';
+	}
+    }
+  else
+    return false;
+  return true;
+}
+
 static struct mnt
   {
     const char *native;
Index: winsup/utils/path.h
===================================================================
RCS file: /cvs/src/src/winsup/utils/path.h,v
retrieving revision 1.2
diff -u -p -r1.2 path.h
--- winsup/utils/path.h	4 Mar 2003 05:30:50 -0000	1.2
+++ winsup/utils/path.h	24 Jul 2006 17:52:15 -0000
@@ -9,3 +9,9 @@ Cygwin license.  Please consult the file
 details. */
 
 char *cygpath (const char *s, ...);
+bool is_exe (HANDLE);
+bool is_symlink (HANDLE);
+bool readlink (HANDLE, char *, int);
+int get_word (HANDLE, int);
+int get_dword (HANDLE, int);
+


More information about the Cygwin-patches mailing list