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

[serguei.spitsyn at oracle.com]

Yes, it is better to have the same view on it.

Thanks!
Serguei


On 1/31/13 11:27 PM, Bengt Rutisson wrote:
>
> Serguei,
>
> On 1/31/13 8:20 PM, serguei.spitsyn at oracle.com wrote:
>> Mikael,
>>
>> Looks good.
>> Q: What is our current policy for outdated copyright comments?
>
> The official policy is that it is optional to update the copyright 
> year. In my opinion this is a really bad policy since it does not give 
> any clues to reviewers whether or not to comment on it.
>
> FWIW, the GC team has agreed on a policy to not update the copyright 
> year and instead let RE deal with it. Since this is a change to a GC 
> file it seems good to not update the copyright year.
>
> Bengt
>
>
>>
>> Thanks,
>> Serguei
>>
>>
>> On 1/31/13 9:11 AM, Mikael Vidstedt 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 <mailto:mikael.vidstedt at oracle.com>> wrote:
>>>>
>>>>
>>>>     Please review the following webrev:
>>>>
>>>>     http://cr.openjdk.java.net/~mikael/8007257/webrev.00/
>>>>     <http://cr.openjdk.java.net/%7Emikael/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/6d7882bc/attachment.html