matthiasblaesing commented on code in PR #6157:
URL: https://github.com/apache/netbeans/pull/6157#discussion_r1254905049
##########
java/debugger.jpda/src/org/netbeans/modules/debugger/jpda/models/ShortenedStrings.java:
##########
@@ -472,4 +656,20 @@ private static class StringValueInfo {
this.shortValueRef = new WeakReference<String>(shortenedValue);
}
}
+ private enum Utf16State{
Review Comment:
```suggestion
private enum InternalStringEncoding {
```
##########
java/debugger.jpda/src/org/netbeans/modules/debugger/jpda/models/ShortenedStrings.java:
##########
@@ -472,4 +656,20 @@ private static class StringValueInfo {
this.shortValueRef = new WeakReference<String>(shortenedValue);
}
}
+ private enum Utf16State{
+ /**
+ * The string is backed by an array of chars
+ */
+ Chars,
Review Comment:
```suggestion
CHAR_ARRAY,
```
##########
java/debugger.jpda/src/org/netbeans/modules/debugger/jpda/models/ShortenedStrings.java:
##########
@@ -472,4 +656,20 @@ private static class StringValueInfo {
this.shortValueRef = new WeakReference<String>(shortenedValue);
}
}
+ private enum Utf16State{
+ /**
+ * The string is backed by an array of chars
+ */
+ Chars,
+ /**
+ * The string is backed by an array of bytes with one byte per char
+ */
+ Latin1,
Review Comment:
```suggestion
BYTE_ARRAY_LATIN1,
```
##########
java/debugger.jpda/src/org/netbeans/modules/debugger/jpda/models/ShortenedStrings.java:
##########
@@ -422,31 +606,31 @@ public String getFullString() {
public Reader getContent() {
return new Reader() {
-
int pos = 0;
-
@Override
public int read(char[] cbuf, int off, int len) throws
IOException {
if (pos + len > length) {
len = length - pos;
}
- List<Value> values;
+ if (len == 0){
+ return -1;
+ }
+// List<Value> values;
+// int grabCount = backingState == Utf16State.Utf16 ? len *
2 :
+// len;
Review Comment:
Please remove
##########
java/debugger.jpda/src/org/netbeans/modules/debugger/jpda/models/ShortenedStrings.java:
##########
@@ -472,4 +656,20 @@ private static class StringValueInfo {
this.shortValueRef = new WeakReference<String>(shortenedValue);
}
}
+ private enum Utf16State{
+ /**
+ * The string is backed by an array of chars
+ */
+ Chars,
+ /**
+ * The string is backed by an array of bytes with one byte per char
+ */
+ Latin1,
+ /**
+ * The string is backed by an array of bytes with two bytes per char.
+ * Use {@link #isLittleEndian(com.sun.jdi.VirtualMachine)} to determine
+ * the byte order
+ */
+ Utf16
Review Comment:
```suggestion
BYTE_ARRAY_UTF16
```
##########
java/debugger.jpda/src/org/netbeans/modules/debugger/jpda/models/ShortenedStrings.java:
##########
@@ -204,6 +209,9 @@ static String getStringWithLengthControl(StringReference
sr) throws
}
String string = null;
boolean isShort = true;
+ Utf16State backingState = Utf16State.Chars;
+ //I think we are going with 'true' by default
+ boolean isLittleEndian;// = true;
Review Comment:
Please move declaration of `isLittleEndian` to initialisation site in 307. I
would suggest to make it a `Boolean`, so that it is clear, that it might
contain bogus data/is not valid. I thing asking the VM once for endianess
should not be to big an issue (there is cache tied to the target machine), but
ok.
##########
java/debugger.jpda/src/org/netbeans/modules/debugger/jpda/models/ShortenedStrings.java:
##########
@@ -262,6 +270,8 @@ else if (valuesField.type() instanceof ArrayType &&
isUTF16 = true;
}
}
+ backingState = isUTF16 ? Utf16State.Utf16 :
Review Comment:
```suggestion
backingEncoding = isUTF16 ? Utf16State.Utf16 :
```
##########
java/debugger.jpda/src/org/netbeans/modules/debugger/jpda/models/ShortenedStrings.java:
##########
@@ -385,19 +348,240 @@ else if (valuesField.type() instanceof ArrayType &&
}
return string;
}
+ //would return char, but exceptions are expensive and so is boxing into
+ //a Character
+ /**
+ * (Currently untested) Grab the char at the given index in the array.
+ * Returns -1 on an error
+ * @param sourceArray Backing array reference. May be a byte or char array
+ * @param index
+ * @param backing
+ * @param isLittleEndian
+ * @return
+ */
+ private static int charAt(ArrayReference sourceArray, int index,
+ Utf16State backing, boolean isLittleEndian) throws
+ InternalExceptionWrapper, ObjectCollectedExceptionWrapper,
+ VMDisconnectedExceptionWrapper{
+ if (backing == Utf16State.Chars){
+ //that was easy
+ Value v = ArrayReferenceWrapper.getValue(sourceArray, index);
+ if (!(v instanceof CharValue)){
+ return -1;
+ }
+ return ((CharValue)v).charValue();
+ }
+ if (backing == Utf16State.Latin1){
+ //that was also easy
+ Value v = ArrayReferenceWrapper.getValue(sourceArray, index);
+ if (!(v instanceof ByteValue)){
+ return -1;
+ }
+ char c = (char)((ByteValue)v).byteValue();
+ //strip off the sign value
+ c &= 0xff;
+ return c;
+ }
+ //uft16 it is
+ int hiByteShift, lowByteShift;
+ if (isLittleEndian){
+ hiByteShift = 0;
+ lowByteShift = 8;
+ }
+ else{
+ hiByteShift = 8;
+ lowByteShift = 0;
+ }
+ List<Value> vals = ArrayReferenceWrapper.getValues(sourceArray,
+ index * 2, 2);
+ Value left = vals.get(0);
+ Value right = vals.get(1);
+ if (!(left instanceof ByteValue && right instanceof ByteValue)){
+ return -1;
+ }
+ return utf16Combine(((ByteValue)left).byteValue(),
+ ((ByteValue)right).value(), hiByteShift, lowByteShift);
+ }
+ /**
+ * (Currently untested) Grab the char at the given index in the array.
+ * Returns -1 on an error
+ * @param sourceArray Backing array reference. May be a byte or char array
+ * @param index
+ * @param backing
+ * @param isLittleEndian
+ * @return
+ */
+ private static int charAt(List<Value> sourceArray, int index,
+ Utf16State backing, boolean isLittleEndian){
+ if (backing == Utf16State.Chars){
+ //that was easy
+ Value v = sourceArray.get(index);
+ if (!(v instanceof CharValue)){
+ return -1;
+ }
+ return ((CharValue)v).charValue();
+ }
+ if (backing == Utf16State.Latin1){
+ //that was also easy
+ Value v = sourceArray.get(index);
+ if (!(v instanceof ByteValue)){
+ return -1;
+ }
+ char c = (char)((ByteValue)v).byteValue();
+ //strip off the sign value
+ c &= 0xff;
+ return c;
+ }
+ //uft16 it is
+ int hiByteShift, lowByteShift;
+ if (isLittleEndian){
+ hiByteShift = 0;
+ lowByteShift = 8;
+ }
+ else{
+ hiByteShift = 8;
+ lowByteShift = 0;
+ }
+ Value left = sourceArray.get(index * 2);
+ Value right = sourceArray.get((index * 2) + 1);
+ if (!(left instanceof ByteValue && right instanceof ByteValue)){
+ return -1;
+ }
+ return utf16Combine(((ByteValue)left).byteValue(),
+ ((ByteValue)right).value(), hiByteShift, lowByteShift);
+ }
+ /**
+ * Copy the input to the destination array as if by {@link
System#arrayCopy}
+ * @param sourceArray Backing array reference. May be a byte or char array
+ * @param backing
+ * @param isLittleEndian
+ * @param start
+ * @param length
+ * @param dest
+ * @return Null on success, otherwise an error message
+ */
+ private static String copyToCharArray(ArrayReference sourceArray,
+ int srcPos, char[] dest, int destPos, int length,
+ Utf16State backing, boolean isLittleEndian) throws
+ ObjectCollectedExceptionWrapper, VMDisconnectedExceptionWrapper,
+ InternalExceptionWrapper{
+ //grab applicable values
+ int realStart = srcPos;
+ int realLength = length;
+ if (backing == Utf16State.Utf16){
+ realStart *= 2;
+ realLength *= 2;
+ }
+ List<Value> values = ArrayReferenceWrapper.getValues(sourceArray,
+ realStart, realLength);
+ //copy them
+ return copyToCharArray(values, 0, dest, destPos, length, backing,
+ isLittleEndian);
+ }
+ /**
+ * Copy the input to the destination array as if by {@link
System#arrayCopy}
+ * @param sourceArray Backing array reference. May be a byte or char array
+ * @param backing
+ * @param isLittleEndian
+ * @param start
+ * @param length
+ * @param dest
+ * @return Null on success, otherwise an error message
Review Comment:
From my POV this makes no sense. The call sites already have exception
handling in place and unless we expect this to fail oftern, I would not add
optimization prematurely. To this looks like a place to throw an IOException,
which for the "read full string" does not even need to be caught (calling code
already has to deal with it).
##########
java/debugger.jpda/src/org/netbeans/modules/debugger/jpda/models/ShortenedStrings.java:
##########
@@ -385,19 +348,240 @@ else if (valuesField.type() instanceof ArrayType &&
}
return string;
}
+ //would return char, but exceptions are expensive and so is boxing into
+ //a Character
+ /**
+ * (Currently untested) Grab the char at the given index in the array.
+ * Returns -1 on an error
+ * @param sourceArray Backing array reference. May be a byte or char array
+ * @param index
+ * @param backing
+ * @param isLittleEndian
+ * @return
+ */
+ private static int charAt(ArrayReference sourceArray, int index,
+ Utf16State backing, boolean isLittleEndian) throws
+ InternalExceptionWrapper, ObjectCollectedExceptionWrapper,
+ VMDisconnectedExceptionWrapper{
+ if (backing == Utf16State.Chars){
+ //that was easy
+ Value v = ArrayReferenceWrapper.getValue(sourceArray, index);
+ if (!(v instanceof CharValue)){
+ return -1;
+ }
+ return ((CharValue)v).charValue();
+ }
+ if (backing == Utf16State.Latin1){
+ //that was also easy
+ Value v = ArrayReferenceWrapper.getValue(sourceArray, index);
+ if (!(v instanceof ByteValue)){
+ return -1;
+ }
+ char c = (char)((ByteValue)v).byteValue();
+ //strip off the sign value
+ c &= 0xff;
+ return c;
+ }
+ //uft16 it is
+ int hiByteShift, lowByteShift;
+ if (isLittleEndian){
+ hiByteShift = 0;
+ lowByteShift = 8;
+ }
+ else{
+ hiByteShift = 8;
+ lowByteShift = 0;
+ }
+ List<Value> vals = ArrayReferenceWrapper.getValues(sourceArray,
+ index * 2, 2);
+ Value left = vals.get(0);
+ Value right = vals.get(1);
+ if (!(left instanceof ByteValue && right instanceof ByteValue)){
+ return -1;
+ }
+ return utf16Combine(((ByteValue)left).byteValue(),
+ ((ByteValue)right).value(), hiByteShift, lowByteShift);
+ }
+ /**
+ * (Currently untested) Grab the char at the given index in the array.
+ * Returns -1 on an error
+ * @param sourceArray Backing array reference. May be a byte or char array
+ * @param index
+ * @param backing
+ * @param isLittleEndian
+ * @return
+ */
+ private static int charAt(List<Value> sourceArray, int index,
+ Utf16State backing, boolean isLittleEndian){
+ if (backing == Utf16State.Chars){
+ //that was easy
+ Value v = sourceArray.get(index);
+ if (!(v instanceof CharValue)){
+ return -1;
+ }
+ return ((CharValue)v).charValue();
+ }
+ if (backing == Utf16State.Latin1){
+ //that was also easy
+ Value v = sourceArray.get(index);
+ if (!(v instanceof ByteValue)){
+ return -1;
+ }
+ char c = (char)((ByteValue)v).byteValue();
+ //strip off the sign value
+ c &= 0xff;
+ return c;
+ }
+ //uft16 it is
+ int hiByteShift, lowByteShift;
+ if (isLittleEndian){
+ hiByteShift = 0;
+ lowByteShift = 8;
+ }
+ else{
+ hiByteShift = 8;
+ lowByteShift = 0;
+ }
+ Value left = sourceArray.get(index * 2);
+ Value right = sourceArray.get((index * 2) + 1);
+ if (!(left instanceof ByteValue && right instanceof ByteValue)){
+ return -1;
+ }
+ return utf16Combine(((ByteValue)left).byteValue(),
+ ((ByteValue)right).value(), hiByteShift, lowByteShift);
+ }
+ /**
+ * Copy the input to the destination array as if by {@link
System#arrayCopy}
+ * @param sourceArray Backing array reference. May be a byte or char array
+ * @param backing
+ * @param isLittleEndian
+ * @param start
+ * @param length
+ * @param dest
+ * @return Null on success, otherwise an error message
+ */
+ private static String copyToCharArray(ArrayReference sourceArray,
+ int srcPos, char[] dest, int destPos, int length,
+ Utf16State backing, boolean isLittleEndian) throws
+ ObjectCollectedExceptionWrapper, VMDisconnectedExceptionWrapper,
+ InternalExceptionWrapper{
+ //grab applicable values
+ int realStart = srcPos;
+ int realLength = length;
+ if (backing == Utf16State.Utf16){
+ realStart *= 2;
+ realLength *= 2;
+ }
+ List<Value> values = ArrayReferenceWrapper.getValues(sourceArray,
+ realStart, realLength);
+ //copy them
+ return copyToCharArray(values, 0, dest, destPos, length, backing,
+ isLittleEndian);
+ }
+ /**
+ * Copy the input to the destination array as if by {@link
System#arrayCopy}
+ * @param sourceArray Backing array reference. May be a byte or char array
+ * @param backing
+ * @param isLittleEndian
+ * @param start
+ * @param length
+ * @param dest
+ * @return Null on success, otherwise an error message
+ */
+ private static String copyToCharArray(List<Value> sourceArray, int srcPos,
+ char[] dest, int destPos, int length, Utf16State backing,
+ boolean isLittleEndian){
+ if (backing == Utf16State.Chars){
+ //that was easy
+ for (int i = 0; i < length; i++) {
+ Value v = sourceArray.get(i + srcPos);
+ if (!(v instanceof CharValue)){
+ return MessageFormat.format("Char at {0} is not a
character: {1}", srcPos + i, v);
+ }
+ dest[destPos + i] = ((CharValue)v).charValue();
+ }
+ return null;
+ }
+ if (backing == Utf16State.Latin1){
+ //that was also easy
+ for (int i = 0; i < length; i++) {
+ Value v = sourceArray.get(i + srcPos);
+ if (!(v instanceof ByteValue)){
+ return MessageFormat.format("Char at {0} is not a byte:
{1}", srcPos + i, v);
+ }
+ char c = (char)((ByteValue)v).byteValue();
+ //strip off the sign value
+ c &= 0xff;
+ dest[destPos + i] = c;
+ }
+ return null;
+ }
+ //uft16 it is
+ assert backing == Utf16State.Utf16;
+ int hiByteShift, lowByteShift;
+ if (isLittleEndian){
+ hiByteShift = 0;
+ lowByteShift = 8;
+ }
+ else{
+ hiByteShift = 8;
+ lowByteShift = 0;
+ }
+ for (int i = 0; i < length; i++) {
+ Value left = sourceArray.get(i * 2);
+ Value right = sourceArray.get((i * 2) + 1);
+ if (!(left instanceof ByteValue && right instanceof ByteValue)){
+ return MessageFormat.format("Char at {0} is not a byte pair: "
+ + "{1},{2}", srcPos + i, left, right);
+ }
+ dest[destPos + i] = utf16Combine(((ByteValue)left).byteValue(),
+ ((ByteValue)right).byteValue(), hiByteShift, lowByteShift);
+ }
+ return null;
+ }
+ private static char utf16Combine(byte left, byte right, int hiByteShift,
Review Comment:
`hiByteShift` and `lowByteShift` should be moved into `utf18Combine` and
that method gets the `isLittleEndian` information. Then the logic is in one
place.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
