RFR: (S): JDK-8195613: [SA] HotSpotTypeDataBase.readVMLongConstants truncates values to int

[Jini George]

Thank you, David. Could I get another pair of eyes to take a look at:

http://cr.openjdk.java.net/~jgeorge/8195613/webrev.02/index.html

Thanks,
Jini.

On 5/11/2018 11:58 AM, David Holmes wrote:
> Forgot to add - no need to see an updated webrev.
> 
> Thanks,
> David
> 
> On 11/05/2018 4:22 PM, David Holmes wrote:
>> Hi Jini,
>>
>> On 11/05/2018 5:34 AM, Jini George wrote:
>>> Hi David,
>>>
>>> Thank you very much for the review and for the clarifications 
>>> offline! I have addressed your comments and have a new webrev at:
>>>
>>> http://cr.openjdk.java.net/~jgeorge/8195613/webrev.01/index.html
>>
>> Looks fine.
>>
>> A couple of minor comments on the test:
>>
>> You don't need:
>>
>>    79                 LingeredApp.stopApp(theApp);
>>
>> as it is done in the finally block.
>>
>> Can you add comments in checkForTruncation stating where the expected 
>> values were obtained from.
>>
>> Thanks,
>> David
>>
>>> Thanks,
>>> Jini.
>>>
>>>
>>> On 5/4/2018 12:23 PM, David Holmes wrote:
>>>> Hi Jini,
>>>>
>>>> On 4/05/2018 2:17 AM, Jini George wrote:
>>>>> Hello!
>>>>>
>>>>> Requesting reviews for:
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8195613
>>>>>
>>>>> ([SA] HotSpotTypeDataBase.readVMLongConstants truncates values to int)
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~jgeorge/8195613/webrev.00/
>>>>
>>>> Actual fix seems fine.
>>>>
>>>> I have a few comments on the test ...
>>>>
>>>> - Why are you only testing on 64-bit? The truncation would happen on 
>>>> 32-bit as well.
>>>>
>>>> - CheckForTruncation seems rather complicated, not sure why we need 
>>>> to look for two different things with one being amd64 specific??
>>>>
>>>> - We've had discussions in the past about splitting strings on \n or 
>>>> \r\n depending on whether it's Windows or not. Better to use 
>>>> String.split("\R") regex function.
>>>>
>>>> - Exception message strings should not contain newline characters. 
>>>> Also you can simplify them:
>>>>
>>>> Reading XXX: got value NNN but expected MMM
>>>>
>>>> And you don't need to define a local variable "String message" you 
>>>> can just:
>>>>
>>>> throw new Exception("Reading XXX: got value " + value + " but 
>>>> expected " + expected);
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>> Testing: The SA tests passed on Mach5.
>>>>>
>>>>> Thanks,
>>>>> Jini.
>>>>>
>>>>>