[OpenSmalltalk/opensmalltalk-vm] minheadless sqWin32Common.c (#345)

 

Questions arising from general review and familiarization with minheadless...

platforms/minheadless/windows/sqWin32Common.c defines functions...

fromSqueakInto()
fromSqueak()
fromSqueak2()
sqWin32PrintLastError()

The first three don't seem to be used from Windows...

$ grep -RIn fromSqueak * | grep -v sqWin32Common.c
platforms/Mac OS/plugins/SecurityPlugin/sqMacSecurity.c:21:#define fromSqueak(string,length) string
platforms/Mac OS/plugins/SecurityPlugin/sqMacSecurity.c:72:  return isAccessiblePathName(fromSqueak(pathString, pathStringLength));
platforms/Mac OS/plugins/SecurityPlugin/sqMacSecurity.c:78:  return isAccessiblePathName(fromSqueak(pathString, pathStringLength));
platforms/Mac OS/plugins/SecurityPlugin/sqMacSecurity.c:84:  return isAccessiblePathName(fromSqueak(pathString, pathStringLength));
platforms/Mac OS/plugins/SecurityPlugin/sqMacSecurity.c:91:  return isAccessibleFileName(fromSqueak(pathString, pathStringLength));
platforms/Mac OS/plugins/SecurityPlugin/sqMacSecurity.c:100:  return isAccessibleFileName(fromSqueak(pathString, pathStringLength));
platforms/Mac OS/plugins/SecurityPlugin/sqMacSecurity.c:106:  return isAccessibleFileName(fromSqueak(pathString, pathStringLength));
platforms/minheadless/windows/sqWin32.h:32:TCHAR *fromSqueak(const char *sqPtr, int sqSize);
platforms/minheadless/windows/sqWin32.h:33:TCHAR *fromSqueak2(const char *sqPtr, int sqSize);
platforms/unix/plugins/SecurityPlugin/sqUnixSecurity.c:24:static char* fromSqueak(char* string, int len)
platforms/unix/plugins/SecurityPlugin/sqUnixSecurity.c:76:  return isAccessiblePathName(fromSqueak(pathString, pathStringLength));
platforms/unix/plugins/SecurityPlugin/sqUnixSecurity.c:84:  return isAccessiblePathName(fromSqueak(pathString, pathStringLength));
platforms/unix/plugins/SecurityPlugin/sqUnixSecurity.c:92:  return isAccessiblePathName(fromSqueak(pathString, pathStringLength));
platforms/unix/plugins/SecurityPlugin/sqUnixSecurity.c:103:  return isAccessibleFileName(fromSqueak(pathString, pathStringLength));
platforms/unix/plugins/SecurityPlugin/sqUnixSecurity.c:118:  return isAccessibleFileName(fromSqueak(pathString, pathStringLength));
platforms/unix/plugins/SecurityPlugin/sqUnixSecurity.c:125:  return isAccessibleFileName(fromSqueak(pathString, pathStringLength));

So what are the pros/cons of the other two definitions? Can these be consolidated to a higher-commonality? Is there a security benefit to defining the function statically local just to that file.

#define fromSqueak(string,length) string

static char* fromSqueak(char* string, int len)
{
  static char buf[MAXPATHLEN];
  strncpy(buf, string, len);
  buf[len] = '\0';
  return buf;
}

For the last, what is the benefit versus printLastError(TEXT())...

$ grep -RIn sqWin32PrintLastError * | grep -v sqWin32Common.c
platforms/minheadless/windows/sqWin32.h:30:void sqWin32PrintLastError(const char *message);
platforms/minheadless/windows/sqWin32Alloc.c:192:               sqWin32PrintLastError("VirtualProtect(x,y,PAGE_EXECUTE_READWRITE)");
platforms/minheadless/windows/sqWin32Alloc.c:204:               sqWin32PrintLastError("VirtualProtect(x,y,PAGE_EXECUTE_READWRITE)");
platforms/minheadless/windows/sqWin32SpurAlloc.c:210:           sqWin32PrintLastError("VirtualProtect(x,y,PAGE_EXECUTE_READWRITE)");
platforms/minheadless/windows/sqWin32SpurAlloc.c:224:           sqWin32PrintLastError("VirtualProtect(x,y,PAGE_EXECUTE_READWRITE)");

@nicolas-cellier-aka-nice some of the above seems related to your recent comments on the mail list

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://github.githubassets.com/images/email/message_cards/header.png","avatar_image_url":"https://github.githubassets.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"DESCRIPTION","message":"minheadless sqWin32Common.c (#345)"}],"action":{"name":"View Issue","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/345"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/345", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/345", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>

 

I already cleaned that in platforms/win32 and now I've WIP in platforms/minheadless/windows.

• https://github.com/OpenSmalltalk/opensmalltalk-vm/search?q=fromSqueak&type=Commits
• 545ec0a

Thanks for the reminder!

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://github.githubassets.com/images/email/message_cards/header.png","avatar_image_url":"https://github.githubassets.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"PERSON","message":"@nicolas-cellier-aka-nice in #345: I already cleaned that in platforms/win32 and now I've WIP in platforms/minheadless/windows.\r\n\r\n- https://github.com/OpenSmalltalk/opensmalltalk-vm/search?q=fromSqueak\u0026type=Commits\r\n- https://github.com/OpenSmalltalk/opensmalltalk-vm/commit/545ec0a931166a8374683940f3344e27a110f42c\r\n\r\nThanks for the reminder!"}],"action":{"name":"View Issue","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/345#issuecomment-451639131"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/345#issuecomment-451639131", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/345#issuecomment-451639131", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>

 

Concerning sqWin32PrintLastError, I don't think that we need two functions indeed.
Either Ronie did not see the existing printLastError definition, or he wanted a better naming scheme where our own functions are prefixed sq or sqUnix sqMqc sqWin32 ... when they exist only on specific platform.

Note that printLastError function is not declared in several call site.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://github.githubassets.com/images/email/message_cards/header.png","avatar_image_url":"https://github.githubassets.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"PERSON","message":"@nicolas-cellier-aka-nice in #345: Concerning `sqWin32PrintLastError`, I don't think that we need two functions indeed.\r\nEither Ronie did not see the existing `printLastError` definition, or he wanted a better naming scheme where our own functions are prefixed sq or sqUnix sqMqc sqWin32 ... when they exist only on specific platform.\r\n\r\nNote that printLastError function is not declared in several call site."}],"action":{"name":"View Issue","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/345#issuecomment-451640429"}}}</script> <script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/345#issuecomment-451640429", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/345#issuecomment-451640429", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>