[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR(S): JDK-8139762 Format warnings in generateJvmOffsets.cpp and libjvm_db.c
From:       "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date:       2015-10-19 17:52:12
Message-ID: 56252DCC.6010505 () oracle ! com
[Download RAW message or body]

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@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
>>>
>

[prev in list] [next in list] [prev in thread] [next in thread]