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

Nikolay Sivov bunglehead at gmail.com
Sun Feb 8 06:46:17 CST 2015


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?



More information about the wine-devel mailing list