From 9177eb74bfbeca9ced687cc9a057f64a82f92a70 Mon Sep 17 00:00:00 2001 From: Alexandre Montplaisir Date: Fri, 26 Apr 2013 15:50:14 -0400 Subject: [PATCH] tmf: Remove the serialization logic from TmfStateValue Serializing the state values is the history backend's job (as in, each backend could do it differently). Move the existing logic to HTInterval. This also fixes a very bad bug where the stringsEntrySize of intervals containing a Long state value would be incorrectly reported as 0. This would mean that nodes could try inserting a value that wouldn't fit, which would end up corrupting the interval data. Change-Id: I6c54ef905b45b06b69ed21ce59c70d2eaa16b7f8 Signed-off-by: Alexandre Montplaisir Reviewed-on: https://git.eclipse.org/r/12254 Tested-by: Hudson CI Reviewed-by: Patrick Tasse IP-Clean: Patrick Tasse --- .../backends/historytree/HTInterval.java | 62 +++++++++++-------- .../core/statevalue/IntegerStateValue.java | 5 -- .../tmf/core/statevalue/LongStateValue.java | 5 -- .../tmf/core/statevalue/NullStateValue.java | 5 -- .../tmf/core/statevalue/StringStateValue.java | 5 -- .../tmf/core/statevalue/TmfStateValue.java | 20 ------ 6 files changed, 36 insertions(+), 66 deletions(-) diff --git a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HTInterval.java b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HTInterval.java index ce733e8279..49100da9b3 100644 --- a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HTInterval.java +++ b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/statesystem/backends/historytree/HTInterval.java @@ -177,9 +177,6 @@ final class HTInterval implements ITmfStateInterval, Comparable { * @return The size of the Strings Entry that was written, if any. */ int writeInterval(ByteBuffer buffer, int endPosOfStringEntry) { - int sizeOfStringEntry; - byte[] byteArrayToWrite; - buffer.putLong(start); buffer.putLong(end); buffer.putInt(attribute); @@ -200,41 +197,37 @@ final class HTInterval implements ITmfStateInterval, Comparable { */ e.printStackTrace(); } - return 0; /* we didn't use a Strings section entry */ + break; case TYPE_STRING: - byteArrayToWrite = sv.toByteArray(); - /* - * Size to write (+2 = +1 for size at the start, +1 for the 0 at the - * end) - */ - sizeOfStringEntry = byteArrayToWrite.length + 2; + byte[] byteArrayToWrite; + try { + byteArrayToWrite = sv.unboxStr().getBytes(); + } catch (StateValueTypeException e1) { + /* Should not happen, we're in a switch/case for string type */ + throw new RuntimeException(); + } /* we use the valueOffset as an offset. */ - buffer.putInt(endPosOfStringEntry - sizeOfStringEntry); + buffer.putInt(endPosOfStringEntry - stringsEntrySize); buffer.mark(); - buffer.position(endPosOfStringEntry - sizeOfStringEntry); + buffer.position(endPosOfStringEntry - stringsEntrySize); /* * write the Strings entry (1st byte = size, then the bytes, then the 0) */ - buffer.put((byte) sizeOfStringEntry); + buffer.put((byte) stringsEntrySize); buffer.put(byteArrayToWrite); buffer.put((byte) 0); assert (buffer.position() == endPosOfStringEntry); buffer.reset(); - return sizeOfStringEntry; + break; case TYPE_LONG: - /* - * Size to write is the number of bytes in a Long - */ - sizeOfStringEntry = 8; - /* we use the valueOffset as an offset. */ - buffer.putInt(endPosOfStringEntry - sizeOfStringEntry); + buffer.putInt(endPosOfStringEntry - stringsEntrySize); buffer.mark(); - buffer.position(endPosOfStringEntry - sizeOfStringEntry); + buffer.position(endPosOfStringEntry - stringsEntrySize); /* * write the Long in the Strings section @@ -250,11 +243,12 @@ final class HTInterval implements ITmfStateInterval, Comparable { } assert (buffer.position() == endPosOfStringEntry); buffer.reset(); - return sizeOfStringEntry; + break; default: - return 0; + break; } + return stringsEntrySize; } @Override @@ -306,11 +300,27 @@ final class HTInterval implements ITmfStateInterval, Comparable { } private int computeStringsEntrySize() { - if (sv.toByteArray() == null) { + switch(sv.getType()) { + case NULL: + case INTEGER: + /* Those don't use the strings section at all */ return 0; + case LONG: + /* The value's bytes are written directly into the strings section */ + return 8; + case STRING: + try { + /* String's length + 2 (1 byte for size, 1 byte for \0 at the end */ + return sv.unboxStr().getBytes().length + 2; + } catch (StateValueTypeException e) { + /* We're inside a switch/case for the string type, can't happen */ + throw new RuntimeException(); + } + default: + /* It's very important that we know how to write the state value in + * the file!! */ + throw new RuntimeException(); } - return sv.toByteArray().length + 2; - /* (+1 for the first byte indicating the size, +1 for the 0'ed byte) */ } /** diff --git a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/IntegerStateValue.java b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/IntegerStateValue.java index d3db069ff1..6cb107bcce 100644 --- a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/IntegerStateValue.java +++ b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/IntegerStateValue.java @@ -41,11 +41,6 @@ final class IntegerStateValue extends TmfStateValue { return valueInt; } - @Override - public byte[] toByteArray() { - return null; - } - @Override public String toString() { return String.format("%3d", valueInt); //$NON-NLS-1$ diff --git a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/LongStateValue.java b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/LongStateValue.java index 3a353b3f89..0a879656ae 100644 --- a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/LongStateValue.java +++ b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/LongStateValue.java @@ -41,11 +41,6 @@ final class LongStateValue extends TmfStateValue { return valueLong; } - @Override - public byte[] toByteArray() { - return null; - } - @Override public String toString() { return String.format("%3d", valueLong); //$NON-NLS-1$ diff --git a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/NullStateValue.java b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/NullStateValue.java index 52de5e5fcc..c6bffdaca4 100644 --- a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/NullStateValue.java +++ b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/NullStateValue.java @@ -38,11 +38,6 @@ final class NullStateValue extends TmfStateValue { return null; } - @Override - public byte[] toByteArray() { - return null; - } - @Override public String toString() { return "nullValue"; //$NON-NLS-1$ diff --git a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/StringStateValue.java b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/StringStateValue.java index bf478cb9ba..64822a9122 100644 --- a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/StringStateValue.java +++ b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/StringStateValue.java @@ -42,11 +42,6 @@ final class StringStateValue extends TmfStateValue { return valueStr; } - @Override - public byte[] toByteArray() { - return valueStr.getBytes(); - } - @Override public String toString() { return valueStr; diff --git a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/TmfStateValue.java b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/TmfStateValue.java index b2405ed54d..c4398575e6 100644 --- a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/TmfStateValue.java +++ b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/tmf/core/statevalue/TmfStateValue.java @@ -43,16 +43,6 @@ public abstract class TmfStateValue implements ITmfStateValue { */ protected abstract Object getValue(); - /** - * Specify how to "serialize" this value when writing it to a file. - * Alternatively you can return "null" here if you do not need a byte-array - * indirection (the getValue will get written as-in instead of the offset in - * the file block) - * - * @return The state value in byte array form - */ - public abstract byte[] toByteArray(); - @Override public boolean equals(Object other) { if (this == other) { @@ -87,16 +77,6 @@ public abstract class TmfStateValue implements ITmfStateValue { return this.getValue().hashCode(); } - /** - * Return the max size that a variable-length state value can have when - * serialized. - * - * @return The maximum size in bytes - */ - public static int getStateValueMaxSize() { - return Byte.MAX_VALUE; - } - /* * Since all "null state values" are the same, we only need one copy in * memory. -- 2.34.1