[PATCH] normalize_posix_path and c:/foo/bar

2008-03-16 Thread Brian Dessent

There's a small buglet in normalize_posix_path in that it doesn't see
c:/foo/bar type paths as being win32 and so it treats them as a relative
path and prepends cwd.  Things go downhill from there.  The testcase
that exposed this was insight failing to load because some of the tcl
parts use win32 paths, but really a reduced testcase is simply
open(c:/cygwin/bin/ls.exe, O_RDONLY) = ENOENT.

Brian2008-03-16  Brian Dessent  [EMAIL PROTECTED]

* path.cc (normalize_posix_path): Correctly identify a win32
path beginning with a drive letter and forward slashes.

 path.cc |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: path.cc
===
RCS file: /cvs/src/src/winsup/cygwin/path.cc,v
retrieving revision 1.479
diff -u -p -r1.479 path.cc
--- path.cc 12 Mar 2008 16:07:04 -  1.479
+++ path.cc 16 Mar 2008 07:18:41 -
@@ -253,7 +253,7 @@ normalize_posix_path (const char *src, c
   char *dst_start = dst;
   syscall_printf (src %s, src);
 
-  if ((isdrive (src)  src[2] == '\\') || *src == '\\')
+  if ((isdrive (src)  (src[2] == '\\' || isslash (src[2]))) || *src == '\\')
 goto win32_path;
 
   tail = dst;


Re: [PATCH] QueryDosDevice in handle_to_fn

2008-03-16 Thread Christopher Faylor
On Sun, Mar 16, 2008 at 04:22:13PM +0100, Corinna Vinschen wrote:
On Mar 16 03:14, Brian Dessent wrote:
   I debugged this and found the
 strangest thing, when you call QueryDosDevice (NULL, fnbuf, len) to get
 the list of all DOS devices and len = 65536, Win32 always returns 0
 with GetLastError set to ERROR_MORE_DATA.  Since len was being set as
 sizeof (OBJECT_NAME_INFORMATION) + NT_MAX_PATH * sizeof (WCHAR) this
 always happened, causing handle_to_fn() to simply give up and copy the
 Win32 name into the POSIX name and return.  The attached patch fixes the
 problem by just clamping the size of the buffer to under 64k.

insert lament here

 Another observation that I had while debugging this is that calling
 strncasematch() in this function is probably wrong -- it expands to
 cygwin_strncasecmp(), which is a wrapper that first converts both
 arguments to temporary UNICODE strings and then calls
 RtlCompareUnicodeString() -- we're doing this on strings that we had
 just converted *out* of UNICODE.  I think ascii_strncasematch() is
 probably what we want here instead, either that or try to stay in
 unicode throughout.

Using ascii_strncasematch here is right because the DEVICE_PREFIX is
plain ascii anyway.  But, yes, the function should be converted to
do everything in WCHAR/UNICODE_STRING and only convert to char *
when creating the final posix_fn.

 +  /* For some reason QueryDosDevice will fail with Win32 errno 234
 + (ERROR_MORE_DATA) if you try to pass a buffer larger than 64k  */
 +  size_t qddlen = len  65536 ? len : 65535;

len is a const value.  Checking len for being  65536 is a constant
expression which always results in qddlen being 65535 so the ?: is
a noop, more or less.

Did you test if QueryDosDeviceW has the same problem as QueryDosDeviceA?
If not, we should use that function.

This is basically my function.  I'll try to convert it to use Unicode
today.

cgf


Re: [PATCH] QueryDosDevice in handle_to_fn

2008-03-16 Thread Brian Dessent
Corinna Vinschen wrote:

 len is a const value.  Checking len for being  65536 is a constant
 expression which always results in qddlen being 65535 so the ?: is
 a noop, more or less.

Yeah, I realized that, and the compiler should optimize it away
completely.  I put it explicitly as a test in the source just for safety
so that if in the future somebody changes the definition of NT_MAX_PATH
or len for some reason and forgets to touch this part that things work
correctly without any overflows.

 Did you test if QueryDosDeviceW has the same problem as QueryDosDeviceA?
 If not, we should use that function.

No, but that's a good idea.  I'll check that and if it doesn't have this
quirk I'll see about moving to do the bulk of the function with WCHARs.

Brian


Re: [PATCH] normalize_posix_path and c:/foo/bar

2008-03-16 Thread Brian Dessent
Corinna Vinschen wrote:

 Actually that was intended, but unfortunately the current path handling
 deliberately creates DOS paths with slashes (in find_exec) right now,
 so that doesn't work ATM.

I guess what I don't understand is how it's both possible for
open(c:/foo/bar.exe) to succeed and for this code to treat it as a
relative posix path instead of absolute win32.  Or is that the point,
that forward-slash win32 paths are intended not to work?  That I think
is going to be quite a lot of affected code unfortunately... as I said
the only real reason I went looking here is I updated my tree to current
CVS and insight stopped functioning.

Brian