Request for review (#4) 7174978: NPG: Fix bactrace builder for class redefinition

[David Holmes]

On 17/01/2013 6:48 AM, Coleen Phillimore wrote:
> On 1/16/2013 2:19 AM, David Holmes wrote:
>>>> src/share/vm/oops/constantPool.hpp
>>>>
>>>> ! void add_version(int version) {
>>>> ! if (version == -1) {
>>>> ! _saved._version = version; // keep overflow
>>>> ! } else {
>>>> ! int new_version = version++;
>>>> ! _saved._version = version == INT_MAX ? -1 : version;
>>>> ! }
>>>> ! }
>>>>
>>>> new_version is unused here but I think you meant to do
>>>>
>>>> _saved._version = new_version == INT_MAX ? -1 : version;
>>>>
>>>
>>> This was a pre-code-review cleanup... I didn't mean to assign version++
>>> into new_version. The second line stays the same. I retested this fix.
>>
>> I'm unclear what the final form of this code now looks like.
>
> void add_version(int version) {
> if (version == -1) {
> _saved._version = version; // keep overflow
> } else {
> version++;
> _saved._version = (version == INT_MAX) ? -1 : version;
> }
> }

But now you "overflow" too early - version==INT_MAX is not an overflow. 
If you check the preincrement value against INT_MAX then you have an 
overflow on the increment - hence I assume you intended

  int new_version = version++;
  _saved._version = new_version == INT_MAX ? -1 : version;

Thanks,
David