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

[David Holmes]

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.
>>>>
>>>>