RFR 9/8: 8066504: GetVersionEx in java.base/windows/native/libjava/java_props_md.c might not get correct Windows version

[Roger Riggs]

I updated the webrev to remove the Windows 3.1 case and
will wait 24hrs to see if there are more comments.

Thanks for the review, Roger


On 6/17/2015 9:58 AM, Alan Bateman wrote:
> On 17/06/2015 14:49, Roger Riggs wrote:
>> Hi Alan,
>>
>> Would there be any concerns about backporting to JDK 8?
> I don't think so but it will require the jdk8u maintainers to approve.
>>
>> The new helper APIs 
>> <https://msdn.microsoft.com/en-us/library/windows/desktop/ms724451%28v=vs.85%29.aspx> 
>> only verify that the OS is at least the version requested.
>> It does not reveal what version the OS is actually running on.
>> Adding a manifest attribute just declares the minimum version required
>> and is reported by GetVersionEx.
>> In the current case, the code uses GetVersionEx as a fallback in
>> case the version on kernel32.dll is not available.
>> That fallback could be removed since kernel32.dll must exist on all 
>> supported versions.
> Okay, I thought the helper APIs had better support than this. Maybe 
> the Microsoft folks that are contributing to OpenJDK might have 
> suggestions. In general I don't have any objections to the approach, 
> just thought it was a big odd that the OS forces us to look at the 
> version of kernel32.dll.
>
>
>> :
>>
>>>
>>> Also, is there any update needed in src/windows/resource/java.manifest?
>> If using the version of kernel32.dll then the manifest version does 
>> not affect the value of os.name.
>> But yes, I added the Windows 10 supportedOS uid.
> I don't think this was in the first webrev that I saw but I see it is 
> there now, looks fine.
>
>
>>>
>>> In passing, GetNativeSystemInfo has been in Windows since Windows XP 
>>> so I don't think we need to use GetModuleHandle to get the address now.
>> ok.
>>
>> Similarly, can the case for Windows 3.1 be removed?   (its not in 
>> theJDK 8 supported matrix 
>> <http://www.oracle.com/technetwork/java/javase/certconfig-2095354.html>)
>> caseVER_PLATFORM_WIN32s:
> I think that would be a fine cleanup.
>
> -Alan.