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

[Coleen Phillimore]

Mikael,

I do like your new version better but because you changed so much, it 
should be verified on all versions of windows.   Which you might not 
want to do.   My suggestion to make this code nicer (fix case stmt and 
outline common code) was really simple and I think can be verified by 
code inspection, which might be less work and easier to review.

Coleen

On 8/7/2013 12:28 PM, 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
>>>
>>
>