Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
> Currently we set _WIN32_WINNT at various places in the codebase; this is used > to target a minimum Windows version we want to support. See also for more > detailled information : > https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt > Macros for Conditional Declarations > "Certain functions that depend on a particular version of Windows are > declared using conditional code. This enables you to use the compiler to > detect whether your application uses functions that are not supported on its > target version(s) of Windows." > > However currently we have quite a lot of differing settings of _WIN32_WINNT > in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible > would make sense because we have this setting already at java_props_md.c > (so targeting older Windows versions at other places is most likely not > useful). Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision: adjust API level to Windows 8 for security.cpp and do some cleanup - Changes: - all: https://git.openjdk.java.net/jdk/pull/8428/files - new: https://git.openjdk.java.net/jdk/pull/8428/files/23b63c5b..721528c4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=8428&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=8428&range=02-03 Stats: 31 lines in 7 files changed: 1 ins; 26 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/8428.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8428/head:pull/8428 PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup I'm confused. `src\jdk.crypto.mscapi\windows\native\libsunmscapi\security.cpp` doesn't set _WIN32_WINNT so how is that later API being enabled? Does this mean that not setting _WIN32_WINNT means :any API is allowed" ? - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:34:43 GMT, David Holmes wrote: > I'm confused. > `src\jdk.crypto.mscapi\windows\native\libsunmscapi\security.cpp` doesn't set > _WIN32_WINNT so how is that later API being enabled? Does this mean that not > setting _WIN32_WINNT means :any API is allowed" ? I found this info here https://stackoverflow.com/questions/37668692/visual-studio-setting-winver-win32-winnt-to-windows-8-on-windows-7 "VS 2012 uses the Windows 8.0 SDK which defaults to _WIN32_WINNT=0x0602 (Windows 8). VS 2013 / 2015 uses the Windows 8.1 SDK which defaults to _WIN32_WINNT=0x0603 (Windows 8.1). If you use VS 2015 and the Windows 10 SDK, it defaults to _WIN32_WINNT=0x0A00 (Windows 10)." So it seems you get some more or less recent default when working with a not too old compiler/SDK. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 09:08:28 GMT, Matthias Baesken wrote: > Does this mean that not setting _WIN32_WINNT means :any API is allowed" ? Hi David , I did one more try with my current setup (VS2017 on a Win10 notebook). I did not set _WIN32_WINNT. My little test program #include #include int main() { #ifdef _WIN32_WINNT printf("_WIN32_WINNT is defined .\n"); #if (_WIN32_WINNT == 0x0600) printf("Vista API setting\n"); #endif #if (_WIN32_WINNT == 0x0601) printf("Win 7 API setting\n"); #endif #if (_WIN32_WINNT == 0x0602) printf("Win 8 API setting\n"); #endif #if (_WIN32_WINNT == 0x0603) printf("Win 8.1 API setting\n"); #endif #if (_WIN32_WINNT == 0x0A00) printf("Win 10 API setting\n"); #endif #endif return 0; } shows me _WIN32_WINNT is defined . Win 10 API setting So I think with our current compilers in use like VS2017 / VS2019 we allow Win10 APIs in most of our code except a few places where we set _WIN32_WINNT and go back to some mixture of older APIs. Not sure if this is a good thing, we could break for example Win 8.1/Win2012 compatibility easily this way. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup This seems okay to me in this form. I agree that explicitly setting this is better than unknowingly using API's that might not be available on supported platforms. Thanks. - Marked as reviewed by dholmes (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup Hi David, thanks for the review ! - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup Looks good, thanks for doing this cleanup. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup Marked as reviewed by prr (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 4 May 2022 08:00:08 GMT, Matthias Baesken wrote: >> Currently we set _WIN32_WINNT at various places in the codebase; this is >> used to target a minimum Windows version we want to support. See also for >> more detailled information : >> https://docs.microsoft.com/en-us/windows/win32/winprog/using-the-windows-headers?redirectedfrom=MSDN#setting-winver-or-_win32_winnt >> Macros for Conditional Declarations >> "Certain functions that depend on a particular version of Windows are >> declared using conditional code. This enables you to use the compiler to >> detect whether your application uses functions that are not supported on its >> target version(s) of Windows." >> >> However currently we have quite a lot of differing settings of _WIN32_WINNT >> in the codebase ; setting _WIN32_WINNT to 0x0601 (Windows 7) where possible >> would make sense because we have this setting already at java_props_md.c >> (so targeting older Windows versions at other places is most likely not >> useful). > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust API level to Windows 8 for security.cpp and do some cleanup This change seem to have made this group of declarations obsolete: `src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h:157` (follow the [link]( https://github.com/openjdk/jdk/blob/89de756ffbefac452c7df559e2a4eb50bf71368b/src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h#L157)). Does anyone have plans to drop those? Have any bugs been filed for that? If not, I'll attend to that myself. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 11 May 2022 16:00:32 GMT, Maxim Kartashev wrote: >> Matthias Baesken has updated the pull request incrementally with one >> additional commit since the last revision: >> >> adjust API level to Windows 8 for security.cpp and do some cleanup > > This change seem to have made this group of declarations obsolete: > `src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h:157` (follow > the [link]( > https://github.com/openjdk/jdk/blob/89de756ffbefac452c7df559e2a4eb50bf71368b/src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h#L157)). > Does anyone have plans to drop those? Have any bugs been filed for that? If > not, I'll attend to that myself. @mkartashev all references to WINVER in the AWT code seem obsolete. Possibly most of the IS_WINxxx usages could also be replaced. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Wed, 11 May 2022 16:00:32 GMT, Maxim Kartashev wrote: > This change seem to have made this group of declarations obsolete: > `src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h:157` (follow > the > [link](https://github.com/openjdk/jdk/blob/89de756ffbefac452c7df559e2a4eb50bf71368b/src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h#L157)). > Does anyone have plans to drop those? Have any bugs been filed for that? If > not, I'll attend to that myself. Hi Maxim, no plans from me, so feel free to do further cleanups. - PR: https://git.openjdk.java.net/jdk/pull/8428
Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]
On Thu, 12 May 2022 04:25:24 GMT, David Holmes wrote: >> This change seem to have made this group of declarations obsolete: >> `src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h:157` (follow >> the [link]( >> https://github.com/openjdk/jdk/blob/89de756ffbefac452c7df559e2a4eb50bf71368b/src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h#L157)). >> Does anyone have plans to drop those? Have any bugs been filed for that? If >> not, I'll attend to that myself. > > @mkartashev all references to WINVER in the AWT code seem obsolete. Possibly > most of the IS_WINxxx usages could also be replaced. @dholmes-ora @MBaesken Thank you! Filed [JDK-8286634](https://bugs.openjdk.java.net/browse/JDK-8286634). - PR: https://git.openjdk.java.net/jdk/pull/8428