[PATCH] kernel32: Implement GetFinalPathNameByHandle

Nikolay Sivov bunglehead at gmail.com
Thu Mar 19 13:32:59 CDT 2015


On 19.03.2015 21:03, Sebastian Lackner wrote:
> On 19.03.2015 18:45, Andrew Eikum wrote:
>> On Thu, Mar 19, 2015 at 06:30:34PM +0100, Sebastian Lackner wrote:
>>> On 19.03.2015 16:22, Andrew Eikum wrote:
>>>> ---
>>>>   dlls/kernel32/kernel32.spec |  4 +-
>>>>   dlls/kernel32/path.c        | 97 +++++++++++++++++++++++++++++++++++++++++++++
>>>>   dlls/kernel32/tests/file.c  | 88 ++++++++++++++++++++++++++++++++++++++++
>>>>   include/winbase.h           |  2 +
>>>>   4 files changed, 189 insertions(+), 2 deletions(-)
>>>>
>>>
>>> Why do you do the work twice? It would have been much easier if you would have just
>>> helped us to review and improve our Staging patchset which is there since about half
>>> a year ago, and supports a lot more flags.
>>>
>>> https://github.com/wine-compholio/wine-staging/blob/master/patches/kernel32-GetFinalPathNameByHandle/0001-kernel32-Implement-GetFinalPathNameByHandle.patch
>>> https://github.com/wine-compholio/wine-staging/blob/master/patches/kernel32-GetFinalPathNameByHandle/0002-kernel32-tests-Add-tests-for-GetFinalPathNameByHandl.patch
>>>
>>> Why does noone bother anymore to test those patches, the author Michael Müller asked
>>> for feedback there long time ago: https://bugs.winehq.org/show_bug.cgi?id=36073#c4
>>>
>>> I'll submit the patches in a few minutes so that you can decide if its better/worse
>>> than the current approach.
>>>
>>
>> The patch you linked certainly looks better, though note that the A
>> version seems to have the same problem that Nikolay pointed out.
>>
>> Andrew
>>
>
> Yes, this part should probably be changed a bit to make the code a bit more clean. Nevertheless
> I am wondering if this is a real problem. Under which circumstances would the WCHAR representation
> be longer than the ASCII one?

WCHAR representation probably can't be longer in characters than ansii 
one. But that's not an overallocation that I was talking about. From 
your repo:

---
+DWORD WINAPI GetFinalPathNameByHandleA(HANDLE file, LPSTR path, DWORD 
charcount, DWORD flags)
+{
+    WCHAR *str;
+    DWORD result;
+
+    TRACE( "(%p,%p,%d,%x)\n", file, path, charcount, flags );
+
+    if (!path || !charcount)
+        return GetFinalPathNameByHandleW(file, (LPWSTR)path, charcount, 
flags);
---

Here you return W-length in characters as A-length in characters, and 
that doesn't look right to me, because you can't know A-length until you 
run WideCharToMultiByte(). At least that's my understanding of how DBCS 
behave.

  Can anyone given an example? Wine uses the same assumption also in
> a couple of other functions, for example:
>
> - GetEnvironmentVariableA
> - ExpandEnvironmentStringsA
> - ...
>
> Are these all wrong?
>
> Sebastian
>
>




More information about the wine-devel mailing list