Re: [PATCH] kernel32: Implement GetFinalPathNameByHandle

[Nikolay Sivov]

On 19.03.2015 18:22, Andrew Eikum wrote:
> +/*************************************************************************
> -
> -
>       GetFinalPathNameByHandleA   (KERNEL32.@)
> - */
>
> +DWORD WINAPI GetFinalPathNameByHandleA(HANDLE handle, char *outstr, DWORD len,
> -
>    DWORD flags)
>
> +{
> - WCHAR *wstr;
> - DWORD dw;
> -
> - if (len > 0)
> - {
> -
>    wstr = HeapAlloc( GetProcessHeap(), 0, len * sizeof(WCHAR) );
> -
>    if (!wstr)
> -
>    {
> -
>        SetLastError( ERROR_OUTOFMEMORY );
> -
>        return 0;
> -
>    }
> - }
> - else
> -
>    wstr = NULL;
> -
> - dw = GetFinalPathNameByHandleW( handle, wstr, len, flags );
> -
> - if (dw > 0 && dw < len)
> - {
> -
>    /* success */
> -
> -
>    dw = WideCharToMultiByte( CP_ACP, 0, wstr, -1, NULL, 0, NULL, NULL );
> -
>    if (dw > len)
> -
>    {
> -
>        /* buffer too small */
> -
>        HeapFree( GetProcessHeap(), 0, wstr );
> -
>        return dw;
> -
>    }
> -
> -
>    dw = WideCharToMultiByte( CP_ACP, 0, wstr, -1, outstr, len, NULL, NULL );
> -
>    if (!dw)
> -
>    {
> -
>        HeapFree( GetProcessHeap(), 0, wstr );
> -
>        return 0;
> -
>    }
> -
> -
>    /* don't report NUL */
> -
>    dw = dw - 1;
> - }
> -
> - HeapFree( GetProcessHeap(), 0, wstr );
> -
> - return dw;
>
> +}
> -

This doesn't look right. Proper way would be:

- call GetFinalPathNameByHandleW() with NULL buffer to get required length;
- allocated WCHAR buffer and get a path in it;
- call WideCharToMultiByte() with NULL output buffer to get required 
length for CP_ACP string;
- do what you need to do if A buffer length is too small.

Right now you're assuming that WCHAR representation has same number of 
characters as CP_ACP one, I don't think it's always a case.

[PATCH] kernel32: Implement GetFinalPathNameByHandle

[Andrew Eikum]

On Thu, Mar 19, 2015 at 06:29:50PM +0300, Nikolay Sivov wrote:
> This doesn't look right. Proper way would be:
> - call GetFinalPathNameByHandleW() with NULL buffer to get required length;
> - allocated WCHAR buffer and get a path in it;
> - call WideCharToMultiByte() with NULL output buffer to get required
>
> length for CP_ACP string;
> - do what you need to do if A buffer length is too small.
>
> Right now you're assuming that WCHAR representation has same number
> of characters as CP_ACP one, I don't think it's always a case.

Yeah, good point. I'll fix and resubmit.

Thanks!
Andrew