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