Skip to content
This repository has been archived by the owner on Aug 10, 2021. It is now read-only.

[platformLibs] mingw windows cleanup #3502

Closed
wants to merge 15 commits into from

Conversation

msink
Copy link
Contributor

@msink msink commented Oct 26, 2019

No description provided.

@msink
Copy link
Contributor Author

msink commented Oct 26, 2019

windows.kt size already reduced from 10MB to 5MB

@msink msink force-pushed the mingw_windows_cleanup branch from f435ee6 to 30946e1 Compare October 26, 2019 12:10
@msink msink marked this pull request as ready for review October 26, 2019 12:18
@msink msink force-pushed the mingw_windows_cleanup branch from 30946e1 to 07496ad Compare October 26, 2019 12:26
@msink msink force-pushed the mingw_windows_cleanup branch 3 times, most recently from 6b28bf2 to f88dd8a Compare October 28, 2019 07:39
@@ -0,0 +1,15 @@
package = platform.winscard
depends = windows

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add: noStringConversion = SCardListReaders

This returns a msz (multi-string) which is a list of null terminated strings, then terminated by a null.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then same should be for SCardLocateCards.
Done.

@@ -0,0 +1,15 @@
package = platform.winscard

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These all should be platform.windows.winscard or similar.

Despite the name, winscard.h exists on non-Windows platforms, and it has subtle differences between Linux (which uses pcsclite) and macOS (which is based on a modified version of pcsclite) APIs.

It needs quite significant hand-holding to fix that...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure.
There always can be subtle differnces, even on mingw_x86 (32-bit) and mingw_x64 (64-bit) platforms.

@micolous
Copy link

micolous commented Oct 28, 2019

I think this should be a narrower scope -- only include stuff that is tested and known to work.

The SCardListReaders issue (this returns a multi-string, but Kotlin tries String conversion on it) is one of many such methods in the Win32 API. Including them here prevents other projects from easily "fixing it".

@msink msink force-pushed the mingw_windows_cleanup branch 6 times, most recently from 737a4f1 to 79d8c3d Compare October 28, 2019 13:15
@msink msink force-pushed the mingw_windows_cleanup branch from 79d8c3d to 621e186 Compare October 28, 2019 13:34
@msink msink force-pushed the mingw_windows_cleanup branch from 621e186 to 1f56468 Compare October 29, 2019 13:48
Copy link
Contributor

@olonho olonho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks interesting, are there samples affected by this change?

@msink
Copy link
Contributor Author

msink commented Oct 30, 2019

I don't know any, this is relatively "conservarive" change - 5MB in windows.kt
It can potentially be reduced to 2.2MB if define WIN32_LEAN_AND_MEAN

locale.h math.h memory.h pthread.h sched.h search.h semaphore.h \
setjmp.h signal.h stdint.h stdio.h stdlib.h string.h \
time.h uchar.h unistd.h utime.h wchar.h wctype.h

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We likely need matching compiler options with other headers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean same as in windows.def?

compilerOpts = \
    -DUNICODE \
    -DWINVER=0x0601 \
    -D_WIN32_WINNT=0x0601 \
    -DWINAPI_FAMILY=3 \
    -DOEMRESOURCE \
    -Wno-incompatible-pointer-types \
    -Wno-deprecated-declarations

Tried it and recompile - almost no change in posix.kt

diff -ur a/posix.kt b/posix.kt
--- a/posix.kt	2019-10-30 12:58:39.175066200 +0500
+++ b/posix.kt	2019-10-30 13:48:24.090793700 +0500
@@ -6512,8 +6512,6 @@
 
 const val __MSVCRT_VERSION__: Int = 1792
 
-const val _WIN32_WINNT: Int = 1537
-
 const val MINGW_HAS_SECURE_API: Int = 1
 
 const val __STDC_SECURE_LIB__: Int = 200411
@@ -8062,9 +8060,9 @@
 
 const val CLOCK_THREAD_CPUTIME_ID: Int = 3
 
-const val _INC_CRT_UNICODE_MACROS: Int = 2
+const val _INC_CRT_UNICODE_MACROS: Int = 1
 
-val __MINGW_PROCNAMEEXT_AW: String get() = "A"
+val __MINGW_PROCNAMEEXT_AW: String get() = "W"
 
 const val INVALID_SOCKET: SOCKET = 18446744073709551615u
 
@@ -8314,7 +8312,7 @@
 
 const val SO_PROTOCOL_INFOW: Int = 8197
 
-const val SO_PROTOCOL_INFO: Int = 8196
+const val SO_PROTOCOL_INFO: Int = 8197
 
 const val PVD_CONFIG: Int = 12289
 

@msink
Copy link
Contributor Author

msink commented Oct 30, 2019

Maybe better do a little less "safe" decomposition, but with windows.kt size 2.2 MBytes?
Because in my experience 5 MBytes is still too big for IDEA autocompletion.
#3520

@msink msink closed this Oct 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants