Hi Daniil,

It looks pretty good in general.
A couple of nits below.

http://cr.openjdk.java.net/~dtitov/8241080/webrev.01/src/jdk.jdwp.agent/share/native/libjdwp/invoker.c.udiff.html
+    void *cursor;
+    jbyte argumentTag;
+    jint argIndex = 0;
+    jvalue *argument = request->arguments;;
. . .
     void *cursor;
     jint argIndex = 0;
+    jbyte argumentTag;
     jvalue *argument = request->arguments;

It is better if the local variables 'cursor' and 'argumentTag' get initialized.
There is double semicolon: "arguments;;"


http://cr.openjdk.java.net/~dtitov/8241080/webrev.01/src/jdk.jdwp.agent/share/native/libjdwp/signature.h.html
  43 static inline jbyte basicType(const char *signature) {

It'd be nice to rename it to basicTypeTag() to get it unified with other functions below.

It is more safe to run tier5 as well.

Thanks,
Serguei


On 5/9/20 09:29, Daniil Titov wrote:
Please review a change[1] that centralizes the signature processing in serviceability tools to make it capable of being easily extensible in the future.

Testing: Mach5 tier1-tier3 tests successfully passed.

[1] http://cr.openjdk.java.net/~dtitov/8241080/webrev.01 
[2] https://bugs.openjdk.java.net/browse/JDK-8241080 

Thank you,
Daniil



Reply via email to