Re: UNC Pathname handling

2009-12-12 Thread Alexandre Hardy
Hi Paul,

On Fri, Dec 11, 2009 at 3:38 PM, Paul Vriens  wrote:
> How can you be sure about longpath being actually a long path? Maybe should
> do a proper call to pGetLongPathNameW just to be sure you're starting point
> is correct.
Hmm, I assumed that it was. Bad assumption!

>
> If longpath is indeed a long path you could compare it with temppath?
Yes, I can do that!

Updates to follow.

Kind regards
Alexandre


-- 
------
Alexandre Hardy
http://www.ahardy.za.net




Re: UNC Pathname handling

2009-12-12 Thread Alexandre Hardy
Hi Paul,

On Fri, Dec 11, 2009 at 3:20 PM, Paul Vriens  wrote:
>> +    if (!(tmplongpath = (WCHAR *)HeapAlloc( GetProcessHeap(), 0,
>> MAX_LONGPATHNAME_LEN * sizeof(WCHAR) ))) {
>> +        SetLastError( ERROR_OUTOFMEMORY );
>> +        return 0;
>> +    }
>> +
>
> This is not really dynamic, is it? You are still allocating that same
> amount. Now, I'm not sure how to fix this as you need some way of
> calculating the needed length in any case. Is that cast necessary btw?

It is not the same amount of memory allocated as previously. Before it
was somewhere
in the order of 255 WCHARS, now it is 32767 WCHARS. Although I can allocate
this statically, I am a little concerned that there may be too much pressure
on the available stack space. Not sure about this though. The value can't
really be determined dynamically, I need enough memory to store whatever
path may be returned. (Similar to GetLongPathNameA).

>
> Now as I understand the function it can only be a maximum of 32767 when you
> use '\\?\', right? If so then this allocation should only be needed in the
> '\\?\' case. This however is something you have to add tests for as well. So
> create a path > MAX_PATH (if that's possible as I guess the path/file has to
> exist as well) and see what is returned on Windows.
Actually, I'm not certain that the maximum is not always 32767 WCHARS.
The MSDN states that GetLongPathNameA is limited to MAX_PATH characters,
and that you should use "\\?\" to increase the limit to 32767 characters.
I'm not entirely sure what GetLongPathNameW has as a limit.

>
> As you are now covering the '\\?\' case the ERR should go down as well and
> should maybe be a FIXME now?
Share names are still not covered. So it is a slite improvement for UNC
pathnames, but not a complete improvement. I'll follow your
guidelines on this point.

Kind regards
Alexandre


-- 
--
Alexandre Hardy
http://www.ahardy.za.net




Re: UNC Pathname handling

2009-12-11 Thread Alexandre Hardy
Hi Paul,

Here is the patch for GetLongPathNameW. Would you mind
looking at it to see if it makes sense?

(Forgot to bottom post in the last message :-( )

Kind regards
Alexandre

-- 
--
Alexandre Hardy
http://www.ahardy.za.net
From 99b08d8497491d52935335e8dd6f9173f68fbab8 Mon Sep 17 00:00:00 2001
From: Alexandre Hardy 
Date: Fri, 11 Dec 2009 14:53:35 +0200
Subject: Handle non share UNC pathnames

---
 dlls/kernel32/path.c   |   40 +---
 dlls/kernel32/tests/path.c |3 ---
 2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/dlls/kernel32/path.c b/dlls/kernel32/path.c
index e9463bb..168946a 100644
--- a/dlls/kernel32/path.c
+++ b/dlls/kernel32/path.c
@@ -44,6 +44,7 @@
 WINE_DEFAULT_DEBUG_CHANNEL(file);
 
 #define MAX_PATHNAME_LEN1024
+#define MAX_LONGPATHNAME_LEN32767
 
 
 /* check if a file name is for an executable file (.exe or .com) */
@@ -290,7 +291,7 @@ DWORD WINAPI GetFullPathNameA( LPCSTR name, DWORD len, LPSTR buffer,
  */
 DWORD WINAPI GetLongPathNameW( LPCWSTR shortpath, LPWSTR longpath, DWORD longlen )
 {
-WCHAR   tmplongpath[MAX_PATHNAME_LEN];
+LPWSTR  tmplongpath;
 LPCWSTR p;
 DWORD   sp = 0, lp = 0;
 DWORD   tmplen;
@@ -309,13 +310,44 @@ DWORD WINAPI GetLongPathNameW( LPCWSTR shortpath, LPWSTR longpath, DWORD longlen
 return 0;
 }
 
+if (!(tmplongpath = (WCHAR *)HeapAlloc( GetProcessHeap(), 0, MAX_LONGPATHNAME_LEN * sizeof(WCHAR) ))) {
+SetLastError( ERROR_OUTOFMEMORY );
+return 0;
+}
+
 TRACE("%s,%p,%d\n", debugstr_w(shortpath), longpath, longlen);
 
 if (shortpath[0] == '\\' && shortpath[1] == '\\')
 {
 ERR("UNC pathname %s\n", debugstr_w(shortpath));
-lstrcpynW( longpath, shortpath, longlen );
-return strlenW(longpath);
+ 
+/* Handle extended length path */
+if (shortpath[2] == '?' && shortpath[3] == '\\') {
+tmplen = GetLongPathNameW(shortpath + 4, tmplongpath, MAX_LONGPATHNAME_LEN);
+if (tmplen == 0) {
+HeapFree( GetProcessHeap(), 0, tmplongpath );
+return 0;
+}
+if (tmplen + 5 <= longlen) {
+lstrcpynW(longpath, shortpath, 5);
+lstrcatW(longpath, tmplongpath);
+HeapFree( GetProcessHeap(), 0, tmplongpath );
+return tmplen + 4;
+} else 
+HeapFree( GetProcessHeap(), 0, tmplongpath );
+return tmplen + 5;
+}
+
+HeapFree( GetProcessHeap(), 0, tmplongpath );
+
+/* Can't handle shares at the moment, just copy the filename
+ * But only if there is enough space in the buffer */
+tmplen = strlenW(shortpath);
+if (longlen >= tmplen + 1) {
+lstrcpynW( longpath, shortpath, longlen );
+return tmplen;
+}
+return tmplen + 1;
 }
 
 unixabsolute = (shortpath[0] == '/');
@@ -358,6 +390,7 @@ DWORD WINAPI GetLongPathNameW( LPCWSTR shortpath, LPWSTR longpath, DWORD longlen
 {
 TRACE("not found %s!\n", debugstr_w(tmplongpath));
 SetLastError ( ERROR_FILE_NOT_FOUND );
+HeapFree( GetProcessHeap(), 0, tmplongpath );
 return 0;
 }
 FindClose(goit);
@@ -379,6 +412,7 @@ DWORD WINAPI GetLongPathNameW( LPCWSTR shortpath, LPWSTR longpath, DWORD longlen
 tmplen--; /* length without 0 */
 }
 
+HeapFree( GetProcessHeap(), 0, tmplongpath );
 return tmplen;
 }
 
diff --git a/dlls/kernel32/tests/path.c b/dlls/kernel32/tests/path.c
index c7e3dc7..c3b216d 100644
--- a/dlls/kernel32/tests/path.c
+++ b/dlls/kernel32/tests/path.c
@@ -1103,16 +1103,13 @@ static void test_GetLongPathNameW(void)
 lstrcpyW(longpath, uncprefix);
 lstrcatW(longpath, tempfile);
 length = pGetLongPathNameW(longpath,NULL,0);
-todo_wine
 ok(lstrlenW(longpath) + 1==length,"GetLongPathNameW returned %d but expected %d\n",length, lstrlenW(longpath) + 1);
 
 lstrcpyW(shortpath, uncprefix);
 GetShortPathNameW(tempfile, shortpath + 4, MAX_PATH - 4);
 length = pGetLongPathNameW(shortpath,NULL,0);
-todo_wine
 ok(lstrlenW(longpath) + 1==length,"GetLongPathNameW returned %d but expected %d\n",length, lstrlenW(longpath) + 1);
 
-todo_wine
 length = pGetLongPathNameW(shortpath,temppath,MAX_PATH);
 ok(lstrlenW(longpath)==length,"GetLongPathNameW returned %d but expected %d\n",length, lstrlenW(longpath));
 
-- 
1.6.2.1




Re: UNC Pathname handling

2009-12-11 Thread Alexandre Hardy
Hi Paul,

Thanks for the help with the tests. Here is a new set of tests.
I'll send the patch that makes these tests succeed
shortly. I would appreciate it if you could have a look at it.

Kind regards
Alexandre

On Fri, Dec 11, 2009 at 12:25 PM, Alexandre Hardy
 wrote:
> Hi Paul,
>
>> Don't think this memset makes sense as you do lstrcpyW after that (and yes
>> it does not make sense in the A-test I did either ;) ).
>
> Okay, that can be removed...
>
>> You're saying that length should be 0 but the test tests something else.
>
> WIll correct that (copy and paste -> I was in too much of a hurry)
>
>>
>>> +
>>> +    memset(shortpath, 0, MAX_PATH * sizeof(WCHAR));
>>> +    memset(longpath, 0, MAX_PATH * sizeof(WCHAR));
>
> I should remove that as well :)
>
>>
>> You're setting longpath but it's not used anymore after that.
>>
>>> +    lstrcpyW(shortpath, uncprefix);
>>> +    GetShortPathNameW(shortpath + 4, tempfile, MAX_PATH - 4);
>>> +    length = pGetLongPathNameW(shortpath,NULL,0);
>>
>> This doesn't make sense to me. You are fill tempfile but not using it
>> afterwards and then you call pGetLongPathNameW with NULL?
>
> Yes, sorry, that should be
> GetShortPathNameW(tempfile, shortpath + 4, MAX_PATH - 4);
>
>> These are added tests but Wine crashes right now with these?
>
> Yes, wine crashes right now with these.
>
> Kind regards
> Alexandre
>
>
> --
> --
> Alexandre Hardy
> http://www.ahardy.za.net
>



-- 
--
Alexandre Hardy
http://www.ahardy.za.net
From 05e69ddb946cc0de5c71157ea83bcd24f013bbe1 Mon Sep 17 00:00:00 2001
From: Alexandre Hardy 
Date: Fri, 11 Dec 2009 13:21:36 +0200
Subject: Some UNC GetLongPathNameW tests

---
 dlls/kernel32/tests/path.c |   31 +++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/dlls/kernel32/tests/path.c b/dlls/kernel32/tests/path.c
index 104a48c..c7e3dc7 100644
--- a/dlls/kernel32/tests/path.c
+++ b/dlls/kernel32/tests/path.c
@@ -1067,6 +1067,13 @@ static void test_GetLongPathNameW(void)
 {
 DWORD length; 
 WCHAR empty[MAX_PATH];
+WCHAR tempfile[MAX_PATH];
+WCHAR longpath[MAX_PATH];
+WCHAR shortpath[MAX_PATH];
+WCHAR temppath[MAX_PATH];
+WCHAR longfilename[] = {'l', 'o', 'n', 'g', 'f', 'i', 'l', 'e', 'n', 'a', 'm', 'e', '.', 'l', 'o', 'n', 'g', 'e', 'x', 't', 0};
+WCHAR uncprefix[] = {'\\', '\\' , '?', '\\', 0};
+HANDLE file;
 
 /* Not present in all windows versions */
 if(pGetLongPathNameW) 
@@ -1086,6 +1093,30 @@ static void test_GetLongPathNameW(void)
 length = pGetLongPathNameW(empty,NULL,0);
 ok(0==length,"GetLongPathNameW returned %d but expected 0\n",length);
 ok(GetLastError()==ERROR_PATH_NOT_FOUND,"GetLastError returned %d but expected ERROR_PATH_NOT_FOUND\n",GetLastError());
+
+GetTempPathW(MAX_PATH, tempfile);
+lstrcatW(tempfile, longfilename);
+
+file = CreateFileW(tempfile, GENERIC_READ|GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
+CloseHandle(file);
+
+lstrcpyW(longpath, uncprefix);
+lstrcatW(longpath, tempfile);
+length = pGetLongPathNameW(longpath,NULL,0);
+todo_wine
+ok(lstrlenW(longpath) + 1==length,"GetLongPathNameW returned %d but expected %d\n",length, lstrlenW(longpath) + 1);
+
+lstrcpyW(shortpath, uncprefix);
+GetShortPathNameW(tempfile, shortpath + 4, MAX_PATH - 4);
+length = pGetLongPathNameW(shortpath,NULL,0);
+todo_wine
+ok(lstrlenW(longpath) + 1==length,"GetLongPathNameW returned %d but expected %d\n",length, lstrlenW(longpath) + 1);
+
+todo_wine
+length = pGetLongPathNameW(shortpath,temppath,MAX_PATH);
+ok(lstrlenW(longpath)==length,"GetLongPathNameW returned %d but expected %d\n",length, lstrlenW(longpath));
+
+DeleteFileW(tempfile);
 }
 }
 
-- 
1.6.2.1




Re: UNC Pathname handling

2009-12-11 Thread Alexandre Hardy
Hi Paul,

> Don't think this memset makes sense as you do lstrcpyW after that (and yes
> it does not make sense in the A-test I did either ;) ).

Okay, that can be removed...

> You're saying that length should be 0 but the test tests something else.

WIll correct that (copy and paste -> I was in too much of a hurry)

>
>> +
>> +    memset(shortpath, 0, MAX_PATH * sizeof(WCHAR));
>> +    memset(longpath, 0, MAX_PATH * sizeof(WCHAR));

I should remove that as well :)

>
> You're setting longpath but it's not used anymore after that.
>
>> +    lstrcpyW(shortpath, uncprefix);
>> +    GetShortPathNameW(shortpath + 4, tempfile, MAX_PATH - 4);
>> +    length = pGetLongPathNameW(shortpath,NULL,0);
>
> This doesn't make sense to me. You are fill tempfile but not using it
> afterwards and then you call pGetLongPathNameW with NULL?

Yes, sorry, that should be
GetShortPathNameW(tempfile, shortpath + 4, MAX_PATH - 4);

> These are added tests but Wine crashes right now with these?

Yes, wine crashes right now with these.

Kind regards
Alexandre


-- 
--
Alexandre Hardy
http://www.ahardy.za.net




Re: UNC Pathname handling

2009-12-11 Thread Alexandre Hardy
> A proposed patch with such tests is attached.
>

There is a bug, the call to GetShortPathName should be
GetShortPathNameW(tempfile, shortpath + 4, MAX_PATH - 4);

Kind regards
Alexandre




Re: UNC Pathname handling

2009-12-11 Thread Alexandre Hardy
Hi Paul,

On Fri, Dec 11, 2009 at 11:09 AM, Paul Vriens
 wrote:
> Well, the tests show that the A-version also handles this case.
>
Okay, no problem there, but since the edge cases for GetLongPathNameW
are not tested by GetLongPathNameA, how about adding a few of
these tests?

A proposed patch with such tests is attached.

Thanks for the friendly advice!

Kinf regards
Alexandre

-- 
--
Alexandre Hardy
http://www.ahardy.za.net
From 742e937bf945d7769b99782e21e1b5e4f567b6d0 Mon Sep 17 00:00:00 2001
From: Alexandre Hardy 
Date: Fri, 11 Dec 2009 11:50:18 +0200
Subject: Some UNC GetLongPathNameW tests

---
 dlls/kernel32/tests/path.c |   29 +
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/dlls/kernel32/tests/path.c b/dlls/kernel32/tests/path.c
index 104a48c..20b13c5 100644
--- a/dlls/kernel32/tests/path.c
+++ b/dlls/kernel32/tests/path.c
@@ -1067,6 +1067,12 @@ static void test_GetLongPathNameW(void)
 {
 DWORD length; 
 WCHAR empty[MAX_PATH];
+WCHAR tempfile[MAX_PATH];
+WCHAR longpath[MAX_PATH];
+WCHAR shortpath[MAX_PATH];
+WCHAR longfilename[] = {'l', 'o', 'n', 'g', 'f', 'i', 'l', 'e', 'n', 'a', 'm', 'e', '.', 'l', 'o', 'n', 'g', 'e', 'x', 't', 0};
+WCHAR uncprefix[] = {'\\', '\\' , '?', '\\', 0};
+HANDLE file;
 
 /* Not present in all windows versions */
 if(pGetLongPathNameW) 
@@ -1086,6 +1092,29 @@ static void test_GetLongPathNameW(void)
 length = pGetLongPathNameW(empty,NULL,0);
 ok(0==length,"GetLongPathNameW returned %d but expected 0\n",length);
 ok(GetLastError()==ERROR_PATH_NOT_FOUND,"GetLastError returned %d but expected ERROR_PATH_NOT_FOUND\n",GetLastError());
+
+GetTempPathW(MAX_PATH, tempfile);
+lstrcatW(tempfile, longfilename);
+
+file = CreateFileW(tempfile, GENERIC_READ|GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
+CloseHandle(file);
+
+memset(longpath, 0, MAX_PATH * sizeof(WCHAR));
+lstrcpyW(longpath, uncprefix);
+lstrcatW(longpath, tempfile);
+length = pGetLongPathNameW(longpath,NULL,0);
+todo_wine
+ok(lstrlenW(longpath) + 1==length,"GetLongPathNameW returned %d but expected 0\n",length);
+
+memset(shortpath, 0, MAX_PATH * sizeof(WCHAR));
+memset(longpath, 0, MAX_PATH * sizeof(WCHAR));
+lstrcpyW(shortpath, uncprefix);
+GetShortPathNameW(shortpath + 4, tempfile, MAX_PATH - 4);
+length = pGetLongPathNameW(shortpath,NULL,0);
+todo_wine
+ok(lstrlenW(longpath) + 5==length,"GetLongPathNameW returned %d but expected 0\n",length);
+
+DeleteFileW(tempfile);
 }
 }
 
-- 
1.6.2.1




Re: UNC Pathname handling

2009-12-11 Thread Alexandre Hardy
Hi Paul,

Stupid question: What do you mean by bottom post? I have
added wine-devel as a CC to these messages, is that
sufficient?

According to my log it seems to be a GetLongPathNameW.

GetLongPathNameA does not exhibit the same problems
as GetLongPathNameW. So GetLongPathNameA(path, NULL, 0)
succeeds, while GetLongPathNameW(path, NULL, 0) segfaults.
This is because GetLongPathNameA provides its own not NULL
buffer to GetLongPathNameW.

It also seems to me that only GetLongPathNameW is required to
handle UNC pathnames that begin with '\\?\' (at least, that
is how I understand the MSDN).

Kind regards
Alexandre

On Fri, Dec 11, 2009 at 10:56 AM, Paul Vriens
 wrote:
> Hi Alexandre,
>
> On 12/11/2009 09:49 AM, Alexandre Hardy wrote:
>>
>> Hi Paul,
>>
>> I guess it could fall under bug #19807. I was actually testing it with
>> Nokia Ovi Installer.
>>
>
> (Please bottom post on wine-devel).
>
> So to fix 19807 you need to handle the '\\?\' cases only so it seems?
>
> Remember that GetLongPathNameA is forward to GetLongPathNameW and from the
> bug report one can't see which one is actually called. So you need a log
> file with either a +file trace or change the TRACE() in GetLongPathNameA to
> a FIXME().
>
> --
> Cheers,
>
> Paul.
>



-- 
--
Alexandre Hardy
http://www.ahardy.za.net




UNC Pathname handling

2009-12-11 Thread Alexandre Hardy
Hi,

I have written a patch to add basic UNC pathname support which tries
to handle UNC pathnames according to the MSDN spec.

The patch satisfies some of the UNC pathname tests for GetLongPathNameA,
but is actually intended to fix GetLongPathNameW.

I am not entirely satisfied with the patch, and would appreciate some
comments.

Some issues:

1) Paths prefixed by '\\?\' require GetLongPathName to handle
strings of length 32767. I don't like the fact that the patch creates two
such large strings on the local stack, should the memory be dynamically
allocated only when needed?

2) The handling of shares \\hostname\C$ is implemented, but no other
shares are handled. And even in this case, only the host on which wine
is executed is handled, no other hosts are handled. I have read that
Windows 7 disables this share by default. So should wine be handling this
particular URL, or should more general sharing be implemented?

The patch is included as an attachment to this message.

Kind regards
Alexandre


-- 
--
Alexandre Hardy
http://www.ahardy.za.net
From b013419f366c83248704f444cbbb6976df740fc9 Mon Sep 17 00:00:00 2001
From: Alexandre Hardy 
Date: Fri, 11 Dec 2009 09:49:59 +0200
Subject: Add partial UNC pathname support

---
 dlls/kernel32/path.c   |   54 +--
 dlls/kernel32/tests/path.c |4 ---
 2 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/dlls/kernel32/path.c b/dlls/kernel32/path.c
index e9463bb..54d9403 100644
--- a/dlls/kernel32/path.c
+++ b/dlls/kernel32/path.c
@@ -44,6 +44,7 @@
 WINE_DEFAULT_DEBUG_CHANNEL(file);
 
 #define MAX_PATHNAME_LEN1024
+#define MAX_LONGPATHNAME_LEN32767
 
 
 /* check if a file name is for an executable file (.exe or .com) */
@@ -290,19 +291,29 @@ DWORD WINAPI GetFullPathNameA( LPCSTR name, DWORD len, LPSTR buffer,
  */
 DWORD WINAPI GetLongPathNameW( LPCWSTR shortpath, LPWSTR longpath, DWORD longlen )
 {
-WCHAR   tmplongpath[MAX_PATHNAME_LEN];
+WCHAR   tmplongpath[MAX_LONGPATHNAME_LEN];
+WCHAR   sharename[MAX_LONGPATHNAME_LEN];
 LPCWSTR p;
 DWORD   sp = 0, lp = 0;
 DWORD   tmplen;
+DWORD   hostlen;
 BOOLunixabsolute;
 WIN32_FIND_DATAWwfd;
 HANDLE  goit;
+WCHAR   cdollar[4] = { '\\', 'C', '$', 0 };
+WCHAR   c_colon[3] = { 'C', ':', 0 };
+BOOLret;
 
 if (!shortpath)
 {
 SetLastError(ERROR_INVALID_PARAMETER);
 return 0;
 }
+if (!longpath && longlen)
+{
+SetLastError(ERROR_INVALID_PARAMETER);
+return 0;
+}
 if (!shortpath[0])
 {
 SetLastError(ERROR_PATH_NOT_FOUND);
@@ -314,8 +325,45 @@ DWORD WINAPI GetLongPathNameW( LPCWSTR shortpath, LPWSTR longpath, DWORD longlen
 if (shortpath[0] == '\\' && shortpath[1] == '\\')
 {
 ERR("UNC pathname %s\n", debugstr_w(shortpath));
-lstrcpynW( longpath, shortpath, longlen );
-return strlenW(longpath);
+ 
+/* Handle extended length path */
+if (shortpath[2] == '?' && shortpath[3] == '\\') {
+tmplen = GetLongPathNameW(shortpath + 4, tmplongpath, MAX_LONGPATHNAME_LEN);
+if (tmplen + 5 <= longlen) {
+lstrcpynW(longpath, shortpath, 5);
+lstrcatW(longpath, tmplongpath);
+return tmplen + 4;
+} else 
+return tmplen + 5;
+}
+
+/* Handle only our host name\C$ for network shares*/
+hostlen = MAX_COMPUTERNAME_LENGTH + 1;
+ret = GetComputerNameW(sharename, &hostlen);
+if (ret) {
+lstrcatW(sharename, cdollar);
+if (strncmpW(sharename, shortpath + 2, hostlen + 3) == 0) {
+lstrcpyW(sharename, c_colon);
+lstrcatW(sharename, shortpath + 5 + hostlen);
+tmplen = GetLongPathNameW(sharename, tmplongpath, MAX_LONGPATHNAME_LEN);
+if (tmplen + hostlen + 4 <= longlen) {
+lstrcpynW(longpath, shortpath, hostlen + 4);
+lstrcatW(longpath, tmplongpath);
+longpath[hostlen + 4] = '$';
+return tmplen + hostlen + 2;
+} else 
+return tmplen + hostlen + 3;
+}
+}
+
+/* Can't handle shares at the moment, just copy the filename
+ * But only if there is enough space in the buffer */
+tmplen = strlenW(shortpath);
+if (longlen >= tmplen + 1) {
+lstrcpynW( longpath, shortpath, longlen );
+return tmplen;
+}
+return tmplen + 1;
 }
 
 unixabsolute 

Re: UNC Pathname handling

2009-12-11 Thread Alexandre Hardy
Hi Paul,

I guess it could fall under bug #19807. I was actually testing it with
Nokia Ovi Installer.

So, If I take out the \\host\C$ part, then this patch won't change which
tests are passed. So I think we need to add GetLongPathNameW
tests. I'm going to give it a bash...

I would prefer not to work further on UNC pathnames that refer to shares,
I would rather try to solve other problems...

Kind regards
Alexandre

On Fri, Dec 11, 2009 at 10:32 AM, Paul Vriens
 wrote:
> Hi Alexandre,
>
> On 12/11/2009 09:02 AM, Alexandre Hardy wrote:
>>
>> Hi,
>>
>> I have written a patch to add basic UNC pathname support which tries
>> to handle UNC pathnames according to the MSDN spec.
>>
>> The patch satisfies some of the UNC pathname tests for GetLongPathNameA,
>> but is actually intended to fix GetLongPathNameW.
>>
>> I am not entirely satisfied with the patch, and would appreciate some
>> comments.
>>
>> Some issues:
>>
>> 1) Paths prefixed by '\\?\' require GetLongPathName to handle
>> strings of length 32767. I don't like the fact that the patch creates two
>> such large strings on the local stack, should the memory be dynamically
>> allocated only when needed?
>
> Dynamic is preferred. We maybe needs some tests to see what Windows does if
> we pass longer strings (without the '\\?\') to the A- and W-version and
> variants of this (including '\\?\').
>
>>
>> 2) The handling of shares \\hostname\C$ is implemented, but no other
>> shares are handled. And even in this case, only the host on which wine
>
> We need a more general way of handling UNC paths. We can't just have this
> single one (only local host and only C: drive) to satisfy the tests. This
> probably goes beyond GetLongPathName though.
>
>> is executed is handled, no other hosts are handled. I have read that
>> Windows 7 disables this share by default. So should wine be handling this
>> particular URL, or should more general sharing be implemented?
>
> The test results on test.winehq.org tells us a different thing though. It
> looks like this is only the case for the Home versions of Windows.
>
> And (again) I think we need more general sharing to be implemented not just
> one case to have a test succeed.
>
>>
>> The patch is included as an attachment to this message.
>>
>> Kind regards
>> Alexandre
>>
>>
>
> For what bug was this?
>
> --
> Cheers,
>
> Paul.
>



-- 
--
Alexandre Hardy
http://www.ahardy.za.net




Re: dlls/kernel32/path.c: Fix return values for UNC pathnames in GetLongPathNameW

2009-12-01 Thread Alexandre Hardy
Thanks Paul,

Right! Tried to implement it too fast! Other parts of the test code dereference
the function pointer and I took my lead from there without checking properly.

As far as the length goes, I don't really know much about UNC pathnames,
so I'm not sure what exactly the result should be. I will need to read up
to find an example that I can guarantee the length of.

Kind regards
Alexandre

On Tue, Dec 1, 2009 at 12:52 PM, Paul Vriens  wrote:
> On 12/01/2009 08:44 AM, Alexandre Hardy wrote:
>>
>> +    length = (*pGetLongPathNameW)(unc_path,NULL,0);
>
> Why the brackets around *pGetLongPathNameW?
>
>> +    ok(length>0,"GetLongPathNameA: Failed on UNC Pathname
>> (length=%d)\n",length);
>
> You called the W-function so the ok text needs to be adjusted. You also know
> the length so why not check explicitly?
>
> --
> Cheers,
>
> Paul.
>



--
--
Alexandre Hardy
http://www.ahardy.za.net



-- 
--
Alexandre Hardy
http://www.ahardy.za.net




dlls/kernel32/path.c: Fix return values for UNC pathnames in GetLongPathNameW

2009-11-25 Thread Alexandre Hardy
The MSDN:
http://msdn.microsoft.com/en-us/library/aa364980(VS.85).aspx
states that GetLongPathNameW returns the length of the string
required to hold the pathname if the buffer is too small to hold
the path. In the case where the buffer is too small, the filename
should not be stored in the buffer.

This ensures that the buffer is not written to (for UNC pathnames)
if the buffer is not large enought to store the path.

Kind regards
Alexandre

-- 
--
Alexandre Hardy
http://www.ahardy.za.net
From 84f1d7704d3f0d88654ad93ed62651ffc08e3208 Mon Sep 17 00:00:00 2001
From: Alexandre Hardy 
Date: Tue, 24 Nov 2009 11:28:45 +0200
Subject: Fixed handling of UNC path name. Should return the length
 required if the provided string is of insufficient length.

---
 dlls/kernel32/path.c |9 +++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/dlls/kernel32/path.c b/dlls/kernel32/path.c
index e9463bb..5a54613 100644
--- a/dlls/kernel32/path.c
+++ b/dlls/kernel32/path.c
@@ -314,8 +314,13 @@ DWORD WINAPI GetLongPathNameW( LPCWSTR shortpath, LPWSTR longpath, DWORD longlen
 if (shortpath[0] == '\\' && shortpath[1] == '\\')
 {
 ERR("UNC pathname %s\n", debugstr_w(shortpath));
-lstrcpynW( longpath, shortpath, longlen );
-return strlenW(longpath);
+tmplen = strlenW(shortpath);
+if (tmplen <= longlen)
+{
+lstrcpynW( longpath, shortpath, longlen );
+return strlenW(longpath);
+}
+return tmplen + 1;
 }
 
 unixabsolute = (shortpath[0] == '/');
-- 
1.6.2.1