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, [email protected] 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



Reply via email to