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

Jini George jini.george at oracle.com
Thu May 10 19:34:12 UTC 2018


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

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


More information about the serviceability-dev mailing list