This is essentially sanitizing and cleanups of existing win32 platform code. It is also very important to reduce the number of compiler warnings. 9 times out of 10, these warnings are false positive, but the tenth is indicating a true problem that we'd better not ignore. If we let the benign warnings overflow us, then we are depriving ourself of a useful tool. You can view, comment on, or merge this pull request online at:https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/329 Commit Summary
File Changes
Patch Links:
— |
@krono commented on this pull request. In platforms/win32/plugins/CroquetPlugin/sqWin32CroquetPlugin.c: > @@ -8,7 +8,7 @@ static BOOLEAN (__stdcall *RtlGenRandom)(PVOID, ULONG) = NULL; int ioGatherEntropy(char *bufPtr, int bufSize) { if(!loaded) { loaded = 1; - hAdvApi32 = LoadLibrary("advapi32.dll"); + hAdvApi32 = LoadLibraryA("advapi32.dll"); What about ⬇️ Suggested change- hAdvApi32 = LoadLibraryA("advapi32.dll"); + hAdvApi32 = LoadLibrary(TEXT("advapi32.dll")); instead? — |
In reply to this post by David T Lewis
I really like this PR. There's one more "fundamental" comment tho:
PS: I just read the last Windows version not being UNICODE was Windows ME. Everything after that internally is — |
In reply to this post by David T Lewis
@krono commented on this pull request. In platforms/win32/vm/sqWin32.h: > @@ -263,7 +263,7 @@ extern char imageName[]; /* full path and name to image */ extern TCHAR imagePath[]; /* full path to image */ extern TCHAR vmPath[]; /* full path to interpreter's directory */ extern TCHAR vmName[]; /* name of the interpreter's executable */ -extern TCHAR windowTitle[]; /* window title string */ +extern char windowTitle[]; /* window title string */ Why? — |
In reply to this post by David T Lewis
@krono commented on this pull request. In platforms/win32/vm/sqWin32Backtrace.c: > @@ -261,14 +261,14 @@ get_modules(void) { DWORD moduleCount2, i; HANDLE me = GetCurrentProcess(); - HANDLE hPsApi = LoadLibrary("psapi.dll"); + HANDLE hPsApi = LoadLibraryA("psapi.dll"); See above — |
In reply to this post by David T Lewis
@krono commented on this pull request. In platforms/win32/vm/sqWin32Heartbeat.c: > @@ -65,6 +65,11 @@ sqLong ioHighResClock(void) { : "=a" (value) : : "rdx"); +#elif defined(_MSC_VER) — |
In reply to this post by David T Lewis
@krono commented on this pull request. In platforms/win32/vm/sqWin32Main.c: > @@ -540,7 +540,7 @@ void gatherSystemInfo(void) { { - HANDLE hUser = LoadLibrary( "user32.dll" ); + HANDLE hUser = LoadLibraryA( "user32.dll" ); see above — |
In reply to this post by David T Lewis
@krono commented on this pull request. In platforms/win32/vm/sqWin32Prefs.c: > @@ -207,8 +207,17 @@ void LoadPreferences() } /* get window title from ini file */ +#ifdef UNICODE + { + TCHAR windowTitleT[MAX_PATH]; + GetPrivateProfileString(U_GLOBAL, TEXT("WindowTitle"), + TEXT(""), windowTitleT, MAX_PATH, squeakIniName); + WideCharToMultiByte(CP_UTF8, 0, windowTitleT, -1, windowTitle, MAX_PATH, NULL, NULL); This needs a check — |
In reply to this post by David T Lewis
Hi Tobias, about LoadLibraryA, I see nothing in MSDN suggesting that these ASCII versions will be deprecated. The purpose of using W variants is to enable usage of internationalized strings, like in window titles, file names, etc... But when we have a simple explicit ASCII string like "advapi32.dll", using the W variant brings nothing. About windowTitle, I can explain more: if you look at https://github.com/OpenSmalltalk/opensmalltalk-vm/blob/476f70605a0352dd7528d251f7403e9233716cdb/platforms/win32/vm/sqWin32Window.c you'll see this: Then
This was before I change anything. Also we have this function:
I think it is for the purpose of communicating the title back to the image. For communicating with the image, we wish using UTF8, so the minimal and simplest change was to just declare In fact, most of the time, we have to communicate the strings with the image, and for that, we will need to perform UTF8-UTF16 conversions, so compiling with — |
In reply to this post by David T Lewis
@nicolas-cellier-aka-nice commented on this pull request. In platforms/win32/vm/sqWin32Prefs.c: > @@ -207,8 +207,17 @@ void LoadPreferences() } /* get window title from ini file */ +#ifdef UNICODE + { + TCHAR windowTitleT[MAX_PATH]; + GetPrivateProfileString(U_GLOBAL, TEXT("WindowTitle"), + TEXT(""), windowTitleT, MAX_PATH, squeakIniName); + WideCharToMultiByte(CP_UTF8, 0, windowTitleT, -1, windowTitle, MAX_PATH, NULL, NULL); Definitely! this can only work with -DUNICODE... I will emit a new commit. — |
In reply to this post by David T Lewis
@nicolas-cellier-aka-nice commented on this pull request. In platforms/win32/vm/sqWin32Prefs.c: > @@ -207,8 +207,17 @@ void LoadPreferences() } /* get window title from ini file */ +#ifdef UNICODE + { + TCHAR windowTitleT[MAX_PATH]; + GetPrivateProfileString(U_GLOBAL, TEXT("WindowTitle"), + TEXT(""), windowTitleT, MAX_PATH, squeakIniName); + WideCharToMultiByte(CP_UTF8, 0, windowTitleT, -1, windowTitle, MAX_PATH, NULL, NULL); Wait, I have been abused by the too-short context: this piece of code is precisely protected with a We may switch to using the W version unconditionnally, but for that, — |
In reply to this post by David T Lewis
@krono commented on this pull request. In platforms/win32/vm/sqWin32Prefs.c: > @@ -207,8 +207,17 @@ void LoadPreferences() } /* get window title from ini file */ +#ifdef UNICODE + { + TCHAR windowTitleT[MAX_PATH]; + GetPrivateProfileString(U_GLOBAL, TEXT("WindowTitle"), + TEXT(""), windowTitleT, MAX_PATH, squeakIniName); + WideCharToMultiByte(CP_UTF8, 0, windowTitleT, -1, windowTitle, MAX_PATH, NULL, NULL); hm, I just meant that — |
In reply to this post by David T Lewis
I agree with most of what you said. Apart from that, good work! — |
In reply to this post by David T Lewis
@nicolas-cellier-aka-nice commented on this pull request. In platforms/win32/vm/sqWin32Prefs.c: > @@ -207,8 +207,17 @@ void LoadPreferences() } /* get window title from ini file */ +#ifdef UNICODE + { + TCHAR windowTitleT[MAX_PATH]; + GetPrivateProfileString(U_GLOBAL, TEXT("WindowTitle"), + TEXT(""), windowTitleT, MAX_PATH, squeakIniName); + WideCharToMultiByte(CP_UTF8, 0, windowTitleT, -1, windowTitle, MAX_PATH, NULL, NULL); OK! — |
In reply to this post by David T Lewis
@nicolas-cellier-aka-nice pushed 1 commit.
— |
In reply to this post by David T Lewis
Merged #329 into Cog. — |
Free forum by Nabble | Edit this page |