Hi, Pierre. Some comments for your patch.
--- a/dlls/setupapi/Makefile.in +++ b/dlls/setupapi/Makefile.in @@ -11,6 +11,7 @@ C_SRCS = \ diskspace.c \ fakedll.c \ install.c \
- log.c \ misc.c \ parser.c \ query.c \
I'm not sure you need a new file for that. It depends on how much will be possibly added in the future, but general rule I think is to try to find a room in existing files.
+#include "wine/debug.h" +#include "windef.h" +#include "winbase.h" +#include "winuser.h" +#include "winreg.h" +#include "setupapi.h" +#include "winnls.h"
Looks like you don't need all of those.
+HANDLE setupact = INVALID_HANDLE_VALUE; +HANDLE setuperr = INVALID_HANDLE_VALUE;
Should be static.
- LPWSTR msg = (LPWSTR)MessageString;
I see what you want to do, but it's a bit ugly imho.
- if (MessageString)
- {
len = lstrlenA(MessageString) + 1;
msg = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
MultiByteToWideChar(CP_ACP, 0, MessageString, -1, msg, len);
- }
Please check for memory allocation failure.
- ret = WriteFile(setupact, MessageString, lstrlenW(MessageString) * sizeof(WCHAR), NULL, NULL);
ret = WriteFile(setuperr, MessageString, lstrlenW(MessageString) * sizeof(WCHAR), NULL, NULL);
You can probably do strlenW() once.
- GetWindowsDirectoryW(path, MAX_PATH);
Same, you need to get this path once.
- if (setupact != INVALID_HANDLE_VALUE)
- {
CloseHandle(setupact);
setupact = INVALID_HANDLE_VALUE;
- }
- if (setuperr != INVALID_HANDLE_VALUE)
- {
CloseHandle(setuperr);
setuperr = INVALID_HANDLE_VALUE;
- }
You can just CloseHandle() unconditionally.
I think it also makes sense to SetupCloseLog() on PROCESS_DETACH, but that's easy to test. Another thing to test is to see what happens in multithreaded case, if it works correctly on windows you need to protect those handles in SetupOpenLog/SetupCloseLog, maybe somewhere else too.