Re: setupapi: Implement SetupOpenLog(), SetupLogErrorA(), SetupLogErrorW(), SetupCloseLog() (try 3)

[Nikolay Sivov]

This is better, but still not quire right.
> +CRITICAL_SECTION setuplog_critical;

Take a look at how critical section data is initialized in wine code, 
name needs to be adjusted to let's say 'setupapi_cs'.
> - if (MessageString)
> - {
> -
>    len = lstrlenA(MessageString) + 1;
> -
>    msg = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
> -
>    if (msg == NULL)
> -
>    {
> -
>        SetLastError(ERROR_NOT_ENOUGH_MEMORY);
> -
>        return FALSE;
> -
>    }
> -
>    MultiByteToWideChar(CP_ACP, 0, MessageString, -1, msg, len);
> - }

That's not how A->W conversion works, we have tons of examples for that.
>      OsVersionInfo.dwOSVersionInfoSize = sizeof(OSVERSIONINFOW);
>      if (!GetVersionExW(&OsVersionInfo))
>          return FALSE;
> -
>    InitializeCriticalSection(&setuplog_critical);
>    SETUPAPI_hInstance = hinstDLL;
>    break;
> case DLL_PROCESS_DETACH:
>        if (lpvReserved) break;
> -
>    DeleteCriticalSection(&setuplog_critical);
>    if (CABINET_hInstance) FreeLibrary(CABINET_hInstance);
>    break;

You won't need these calls once you init it like the rest of Wine code 
does. Also what about closing files on DLL_PROCESS_DETACH?
> - SetLastError(ERROR_ALREADY_EXISTS);
> - return TRUE;

Is it really important for some application to set this error on 
success? In general you only need to set it if call failed.
> - setupact = CreateFileW(path, GENERIC_WRITE, FILE_SHARE_WRITE | FILE_SHARE_READ,
> -
>                       NULL, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);

Did you check what happens if multiple processes are logging?