RFR (XS): 8003879: Duplicate definitions in vmStructs

David Holmes david.holmes at oracle.com
Wed Nov 21 20:17:27 PST 2012


Hi Mikael,

On 22/11/2012 11:42 AM, Mikael Vidstedt wrote:
>
> Please review the below change.
>
> The change for 7045397 introduced a couple of duplicate entries in the
> vmStructs::localHotSpotVMTypes array. This shows up when using the jmap
> tool in a rather ugly way:
<snip>

Other than an indentation problem (which I don't think you introduced) 
this fixup seems fine.

> In addition to removing the two duplicated entries I also added a
> simple, naive runtime test to walk through and make sure no type is
> repeated. The VMStructs::init only called in debug_only so there's no
> startup overhead in product, but it may be better to turn the test into
> a unit test and only running it as part of ExecuteInternalVMTests.
> Feedback appreciated!

I have a few comments on this part:

- Array indices should be int's not size_t (ie signed not unsigned)
- I can't clearly see how the localHotSpotVMTypes array is declared or 
filled but I assume there is guaranteed to be a sentinel entry at the 
end so that we don't index past the end?
- assert(0, ...) should be assert(false, ...)  (as per style guide [1] ;-) )

That all said I'm not sure this test belongs there, but I don't feel 
strongly either way.

Cheers,
David

[1] https://wikis.oracle.com/display/HotSpotInternals/StyleGuide

>
> http://cr.openjdk.java.net/~mikael/8003879/webrev.00/
>
> Cheers,
> Mikael
>


More information about the serviceability-dev mailing list