RFR(S): JDK-8139762 Format warnings in generateJvmOffsets.cpp and libjvm_db.c
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Mon Oct 19 17:52:12 UTC 2015
Dmitry,
It loos good.
You may consider to fix the copyright year as well.
Thanks,
Serguei
On 10/19/15 01:07, Dmitry Samersoff wrote:
> Serguei,
>
> I withdraw the changes to generateJvmOffsets.cpp,
> webrev with libjvm_db.c changes is:
>
> http://cr.openjdk.java.net/~dsamersoff/JDK-8139762/webrev.02/
>
> -Dmitry
>
>
>
> On 2015-10-16 22:10, serguei.spitsyn at oracle.com wrote:
>> The fixes in the libjvm_db.c look good.
>> But the fix in the generateJvmOffsets.cpp looks strange.
>> Why do you need to replace the format %d with %ld ?
>>
>> @@ -121,11 +121,11 @@
>> }
>>
>> #define GEN_VALUE(String,Value) \
>> switch(gen_variant) { \
>> case GEN_OFFSET: \
>> - printf("#define %-40s %d\n", #String, Value); \
>> + printf("#define %-40s %ld\n", #String, Value); \
>> break; \
>> case GEN_INDEX: \
>> printf("#define IDX_%-40s %d\n", #String, index++); \
>> break; \
>> case GEN_TABLE: \
>>
>>
>> First, the line below still has the %d format:
>>
>> case GEN_INDEX: \
>> printf("#define IDX_%-40s %d\n", #String, index++); \
>>
>>
>>
>> second, the Value in the GEN_VALUE macro is expected to be int:
>>
>> 219 GEN_VALUE(OFFSET_HeapBlockHeader_used, (int) offset_of(HeapBlock::Header, _used));
>>
>> 283 GEN_VALUE(SIZE_HeapBlockHeader, (int) sizeof(HeapBlock::Header));
>>
>>
>> If there is a type mismatch anywhere then it is better to use the type
>> cast to int as above.
>>
>>
>> Thanks,
>> Serguei
>>
>> On 10/16/15 07:56, Dmitry Samersoff wrote:
>>> Everybody.
>>>
>>> Please review the fix.
>>>
>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8139762/webrev.01/
>>>
>>> printf format characters %d and %ld misused in couple of places in
>>> generateJvmOffsets.cpp and libjvm_db.c
>>>
>>> -Dmitry
>>>
>
More information about the serviceability-dev
mailing list