Hi Alex,

The fix looks good.

> And I'm not sure why len+len/2+2 is used there.
> In my fix I allocated len*4+1 (for the worst case - each symbol requires 4 bytes to encode plus 1 byte for terminal 0).

I agree, the size len+len/2+2 looks very strange.
Most likely, we get error messages truncated.
Should we replace it with len * sizeof(int) + 1 ?

Thanks,
Serguei

On 5/22/20 11:33, Alex Menkov wrote:
Hi Alan,

thank you for the review.

On 05/22/2020 11:16, Alan Bateman wrote:
On 22/05/2020 18:50, Alex Menkov wrote:
Hi all,

please review the fix for
https://bugs.openjdk.java.net/browse/JDK-8244703
webrev:
http://cr.openjdk.java.net/~amenkov/jdk15/jdwp_javalib_dep/webrev/

The issue is a regression from JDK-8222529 which introduced dependency jdwp lib of java lib. The fix removes the dependency and implements platform to utf8 conversion using existing jdwp code.
This looks good to me. While we are in the area, can you look at printLastError in transport.c? I'm just wondering if parentheses could be added to "len+len/2+2" to make it clear how maxlen is set.

Not sure I understand what parentheses do you mean.

And I'm not sure why len+len/2+2 is used there.
In my fix I allocated len*4+1 (for the worst case - each symbol requires 4 bytes to encode plus 1 byte for terminal 0).

--alex


-Alan.



Reply via email to