[PATCH] kernel32: Implement GetFinalPathNameByHandle
Sebastian Lackner
sebastian at fds-team.de
Thu Mar 19 13:42:46 CDT 2015
On 19.03.2015 19:32, Nikolay Sivov wrote:
> 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
>>
>>
>
Ah, sure. Was looking at the other code part all the time. ^^ Will fix that.
Any other feedback while we're just discussing this version?
Sebastian
More information about the wine-devel
mailing list