Re: [patch] recognise when an exec()d process terminates due to unhandled exception

2008-03-15 Thread Brian Dessent
Christopher Faylor wrote:

 That was going to be my first observation, actually.  I'm still trying
 to digest the patch but it seems like it wouldn't work well with the
 fork retry code.

The patch doesn't change any behavior though: in current Cygwin if the
thing we're exec()ing returns a Win32 exit code of C135 (or
whatever) then we retry creating it 5 times.  With the patch, we still
retry starting it 5 times but after the last one fails we recognise the
situation and print a message.

Brian


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

2008-03-15 Thread Brian Dessent
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.

Brian2008-03-15  Brian Dessent  [EMAIL PROTECTED]

* 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 -  1.14
+++ path.cc 15 Mar 2008 13:12:45 -
@@ -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