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?