Re: [PATCH 3 of 3] shell32: implement SHPathPrepareForWrite

[Dmitry Timoshkov]

"Vincent Povirk"  wrote:
> fixes bug 9523, Knytt Stories installer fails
> +/***********************************************************************
> -
> -
>          SHPathPrepareForWriteA (SHELL32.@)
> - */
>
> +HRESULT WINAPI SHPathPrepareForWriteA(HWND hwnd, IUnknown *modless, LPCSTR path, DWORD flags)
> +{
> - WCHAR wpath[MAX_PATH];
> - MultiByteToWideChar( CP_ACP, 0, path, -1, wpath, MAX_PATH);
> - return SHPathPrepareForWriteW(hwnd, modless, wpath, flags);

You are still using a buffer of a fixed size.
> - /* cut off filename if necessary */
> - if (flags & SHPPFW_IGNOREFILENAME)
> - {
> -
>    last_slash = StrRChrW(path, NULL, '\\');

StrRChrW is a shlwapi export. Since shlwapi.dll is just mostly a wrapper/helper,
and it imports shell32, that would create a circular dependency. I already
suggested to use strrchrW here.
> -
>    temppath = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
> -
>    if (!temppath)
> -
>        return E_OUTOFMEMORY;
> -
>    StrCpyNW(temppath, path, len);

Again StrCpyNW is a shlwapi export, and since you just allocated the buffer
of correct length memcpy + adding an explicit '\0' terminator look more
naturally here (if not kernel32.lstrcpynW).
> - /* try to create the directory if asked to */
> - if (flags & (SHPPFW_DIRCREATE|SHPPFW_ASKDIRCREATE))
> - {
> -
>    if (flags & SHPPFW_ASKDIRCREATE)
> -
>        FIXME("treating SHPPFW_ASKDIRCREATE as SHPPFW_DIRCREATE\n");
> -
> -
>    SHCreateDirectoryExW(0, realpath, NULL);
> - }

You are still not checking SHCreateDirectoryExW return value.
> - if (temppath)
> -
>    HeapFree(GetProcessHeap(), 0, temppath);

There is no need to check temppath for NULL before HeapFree call.