RFR (S): 8022452: Hotspot needs to know about Windows 8.1 and Windows Server 2012 R2

[Ioi Lam]

Just a minor nit. I think it's better to keep the old variable name 
(osvi) so that the number of lines changed would be minimized.

For testability, good luck finding a Windows 3.1 :-)

How about splitting the function so that you have a

void os::win32::print_windows_version(outputStream* st, OSVERSIONINFOEX 
osvi)

Then, we can have a stand-alone test.exe that links to os_windows.obj 
and test it with all combinarions of osvi.

- Ioi

On 08/07/2013 09:28 AM, Mikael Vidstedt wrote:
>
> Coleen,
>
> Funny you should mention that, and since I fully agree with you I 
> actually did prepare another version of the patch which is heavily 
> inspired by the java_props_md.c implementation:
>
> http://cr.openjdk.java.net/~mikael/webrevs/winvercleanup2/webrev/
>
> I personally like this version better, but the one thing that held me 
> back was the fact that I will basically want to dig up machines with 
> all the different (supported) Windows versions and verify that the 
> code is still correct in all cases. If you think this is the way to go 
> I more than willing to will do so. Let me know what you think about 
> the new version of the patch!
>
> There's no real urgency here - I'd rather do it correct once than...
>
> Cheers,
> Mikael
>
>
> On 2013-08-07 06:39, Coleen Phillimore wrote:
>>
>> It looks like the code under the case for all these releases, should 
>> be different case statements for each, rather than these cascading 
>> 'if' statements.  And make 1668-1673 a new function returning si.   
>> So if we have a new release, in general it will work because the 
>> default will print out something.   But a better default would be 
>> lines 1712-1717 which are dead code now.
>>
>> Sorry, I know it's simple and urgent but this seems to be getting 
>> increasingly ugly.
>>
>> Thanks,
>> Coleen
>>
>> On 8/6/2013 7:36 PM, Mikael Vidstedt wrote:
>>>
>>> Please review the following change:
>>>
>>> http://cr.openjdk.java.net/~mikael/webrevs/8022452/webrev
>>>
>>> The patch adds support to os_windows.cpp for recognizing the 
>>> upcoming Windows versions: Windows 8.1 and Windows Server 2012 R2.
>>>
>>> The changed is based on the corresponding code on the JDK 
>>> side[1][2][3].
>>>
>>> Thanks,
>>> Mikael
>>>
>>> [1] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8020191
>>> [2] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/40221b09812f
>>> [3] 
>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-July/019463.html
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130807/9aa93ece/attachment-0001.html