RFR (XS): 8007257: metaspace.cpp: Incorrect arguments in calls to err_msg

[Vitaly Davidovich]

Looks good to me (I'm not an official reviewer though).

Thanks

Sent from my phone
On Jan 31, 2013 12:11 PM, "Mikael Vidstedt" <mikael.vidstedt at oracle.com>
wrote:

>
> Vitaly,
>
> Thanks for the review, and good catch! An updated webrev can be found here:
>
> http://cr.openjdk.java.net/~mikael/8007257/webrev.01/webrev
>
> Cheers,
> Mikael
>
> On 2013-01-30 19:18, Vitaly Davidovich wrote:
>
> Hi Mikael,
>
> Shouldn't this assert:
>
> assert(chunk_word_size != 0 && class_chunk_word_size != 0,
> 1741     err_msg("Initial chunks sizes bad: data  " SIZE_FORMAT
> 1742             " class " SIZE_FORMAT,
> 1743             *chunk_word_size, *class_chunk_word_size));
>
> be (deref the values in the pointers):
>
> assert(*chunk_word_size != 0 && *class_chunk_word_size != 0,
> 1741     err_msg("Initial chunks sizes bad: data  " SIZE_FORMAT
> 1742             " class " SIZE_FORMAT,
> 1743             *chunk_word_size, *class_chunk_word_size));
>
> Otherwise it's testing something other than what I think it's trying to
> test.
>
> Thanks
>
> Sent from my phone
> On Jan 30, 2013 7:53 PM, "Mikael Vidstedt" <mikael.vidstedt at oracle.com>
> wrote:
>
>>
>> Please review the following webrev:
>>
>> http://cr.openjdk.java.net/~mikael/8007257/webrev.00/
>>
>>
>> The key part of the fix is the change in
>> SpaceManager::get_initial_chunk_sizes(), where the intention is to print
>> out the actual sizes, but accidentally the arguments to err_msg are
>> pointers to the values.
>>
>> I also found three other mismatching format/arguments which I fixed while
>> at it:
>>
>> HumongousChunkGranularity is an enum, which is an int, and therefore
>> should be printed using %d. The others fixes are for size_t and uintx
>> variables and should be printed using those respective formats.
>>
>> Cheers,
>> Mikael
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130131/b50a9bb9/attachment.html