Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]

2022-05-12 Thread Maxim Kartashev
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


Re: RFR: JDK-8285730: unify _WIN32_WINNT settings [v4]

2022-05-12 Thread Matthias Baesken
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]

2022-05-11 Thread David Holmes
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]

2022-05-11 Thread Maxim Kartashev
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]

2022-05-09 Thread Alan Bateman
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]

2022-05-09 Thread Phil Race
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]

2022-05-06 Thread Magnus Ihse Bursie
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]

2022-05-06 Thread Erik Joelsson
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]

2022-05-06 Thread Matthias Baesken
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]

2022-05-06 Thread David Holmes
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]

2022-05-06 Thread Matthias Baesken
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]

2022-05-04 Thread Matthias Baesken
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]

2022-05-04 Thread David Holmes
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]

2022-05-04 Thread Matthias Baesken
> 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=8428=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8428=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