ctf: only flatten structures, not arrays The goal of the patch is to handle fixed-length strings and have them parse in a human and machine readable way. Previous implementations would flatten arrays to something like: element[0]='H', element[1]='e'... etc.. This was done to accelerate the parsing of a trace. However, it leads to developers needing to reconstiture events. This patch changes the way it is done to have an array as follows: element = "Hello World!" for an array of characters element = [1,2,3,4,5,6....] for an array for bytes element = [0x00, 0x01, 0x02....] for an array of hex bytes The changes were needed as the array formatting was never implemented as it was blocked by flattening of arrays. [Changed] Handle fixed length strings in CTF [Changed] Make event arrays output more "JSON"-like [Changed] Improve byte array.toString() (Hex and decimal) [Changed] CTF String fields will be formated surrounded by "" Change-Id: I49c5e730cde6501732facd809d9282eec47a66f4 Signed-off-by: Matthew Khouzam <matthew.khouzam@ericsson.com> Reviewed-on: https://git.eclipse.org/r/c/tracecompass/org.eclipse.tracecompass/+/202665 Tested-by: Trace Compass Bot <tracecompass-bot@eclipse.org> Tested-by: Patrick Tasse <patrick.tasse@gmail.com> Reviewed-by: Patrick Tasse <patrick.tasse@gmail.com>
diff --git a/ctf/org.eclipse.tracecompass.ctf.core.tests/src/org/eclipse/tracecompass/ctf/core/tests/event/CTFEventFieldTest.java b/ctf/org.eclipse.tracecompass.ctf.core.tests/src/org/eclipse/tracecompass/ctf/core/tests/event/CTFEventFieldTest.java index 8f03735..f1d102a 100644 --- a/ctf/org.eclipse.tracecompass.ctf.core.tests/src/org/eclipse/tracecompass/ctf/core/tests/event/CTFEventFieldTest.java +++ b/ctf/org.eclipse.tracecompass.ctf.core.tests/src/org/eclipse/tracecompass/ctf/core/tests/event/CTFEventFieldTest.java
@@ -14,7 +14,9 @@ package org.eclipse.tracecompass.ctf.core.tests.event; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import java.nio.ByteBuffer; import java.nio.ByteOrder; @@ -31,26 +33,27 @@ import org.eclipse.tracecompass.ctf.core.event.types.StringDefinition; import org.eclipse.tracecompass.ctf.core.event.types.StructDeclaration; import org.eclipse.tracecompass.ctf.core.event.types.StructDefinition; +import org.eclipse.tracecompass.internal.ctf.core.event.types.ArrayDeclaration; import org.eclipse.tracecompass.internal.ctf.core.event.types.SequenceDeclaration; import org.junit.Test; /** - * The class <code>CTFEventFieldTest</code> contains tests for the class - * <code>{@link CTFEventField}</code>. + * The class <code>CTFEventFieldTest</code> contains tests for fields of CTF + * Events. Tests {@link IDefinition} mostly. * - * @author ematkho - * @version $Revision: 1.0 $ + * + * @author Matthew Khouzam */ -@SuppressWarnings("javadoc") public class CTFEventFieldTest { @NonNull - private static final String fieldName = "id"; + private static final String FIELD_NAME = "id"; /** * Run the CTFEventField parseField(Definition,String) method test. * * @throws CTFException + * An exception was thrown */ @Test public void testParseField_complex() throws CTFException { @@ -102,15 +105,116 @@ * Run the CTFEventField parseField(Definition,String) method test. * * @throws CTFException + * An exception was thrown */ @Test public void testParseField_simple() throws CTFException { final StringDeclaration elemType = StringDeclaration.getStringDeclaration(Encoding.UTF8); byte[] bytes = { 'T', 'e', 's', 't', '\0' }; ByteBuffer bb = testMemory(ByteBuffer.wrap(bytes)); - IDefinition fieldDef = elemType.createDefinition(null, fieldName, new BitBuffer(bb)); + IDefinition fieldDef = elemType.createDefinition(null, FIELD_NAME, new BitBuffer(bb)); assertNotNull(fieldDef); + assertEquals("\"Test\"", fieldDef.toString()); + } + + /** + * Run the CTFEventField parseField(Definition,Array) method test. + * + * @throws CTFException + * An exception was thrown + */ + @Test + public void testParseField_fixedLengthString() throws CTFException { + IntegerDeclaration charDeclaration = IntegerDeclaration.createDeclaration(8, false, 16, ByteOrder.nativeOrder(), Encoding.ASCII, "inner", 8, null); + int length = 128; + final ArrayDeclaration elemType = new ArrayDeclaration(length, charDeclaration); + byte[] bytes = new byte[length]; + bytes[0] = 'T'; + bytes[1] = 'e'; + bytes[2] = 's'; + bytes[3] = 't'; + ByteBuffer bb = testMemory(ByteBuffer.wrap(bytes)); + IDefinition fieldDef = elemType.createDefinition(null, FIELD_NAME, new BitBuffer(bb)); + + assertNotNull(fieldDef); + assertEquals("Test", fieldDef.toString()); + } + + /** + * Run the CTFEventField parseField(Definition,Array) method test. + * + * @throws CTFException + * An exception was thrown + */ + @Test + public void testParseField_fixedLengthArray() throws CTFException { + IntegerDeclaration charDeclaration = IntegerDeclaration.createDeclaration(8, false, 10, ByteOrder.nativeOrder(), Encoding.NONE, "inner", 8,null); + int length = 128; + final ArrayDeclaration elemType = new ArrayDeclaration(length, charDeclaration); + byte[] bytes = new byte[length]; + bytes[0] = 'T'; + bytes[1] = 'e'; + bytes[2] = 's'; + bytes[3] = 't'; + ByteBuffer bb = testMemory(ByteBuffer.wrap(bytes)); + IDefinition fieldDef = elemType.createDefinition(null, FIELD_NAME, new BitBuffer(bb)); + + assertNotNull(fieldDef); + String eventString = fieldDef.toString(); + assertNotEquals("Non string array read as a string", "Test", eventString); + assertTrue("String should start with \"[84, 101, 115, 116,\" " + eventString, eventString.startsWith("[84, 101, 115, 116,")); + } + + /** + * Tests a byte array filled with chars, no room for the null terminator + * + * @throws CTFException + * An exception was thrown + */ + @Test + public void testParseField_fixedLengthNotNullTerminatedArray() throws CTFException { + IntegerDeclaration charDeclaration = IntegerDeclaration.createDeclaration(8, false, 10, ByteOrder.nativeOrder(), Encoding.ASCII, "inner", 1, null); + int length = 6; + final ArrayDeclaration elemType = new ArrayDeclaration(length, charDeclaration); + byte[] bytes = new byte[length]; + bytes[0] = 'T'; + bytes[1] = 'e'; + bytes[2] = 's'; + bytes[3] = 't'; + bytes[4] = 68; + bytes[5] = 101; + ByteBuffer bb = testMemory(ByteBuffer.wrap(bytes)); + IDefinition fieldDef = elemType.createDefinition(null, FIELD_NAME, new BitBuffer(bb)); + assertNotNull(fieldDef); + String eventString = fieldDef.toString(); + assertNotEquals("Non string array read as a string", "Test", eventString); + assertTrue("String should start with \"Test\" " + eventString, eventString.startsWith("Test")); + } + + /** + * Run the CTFEventField parseField(Definition,Array) method test. + * + * @throws CTFException + * An exception was thrown + */ + @Test + public void testParseField_fixedHexLengthArray() throws CTFException { + IntegerDeclaration charDeclaration = IntegerDeclaration.createDeclaration(8, false, 16, ByteOrder.nativeOrder(), Encoding.NONE, "inner", 8, null); + int length = 128; + final ArrayDeclaration elemType = new ArrayDeclaration(length, charDeclaration); + byte[] bytes = new byte[length]; + bytes[0] = 'T'; + bytes[1] = 'e'; + bytes[2] = 's'; + bytes[3] = 't'; + ByteBuffer bb = testMemory(ByteBuffer.wrap(bytes)); + IDefinition fieldDef = elemType.createDefinition(null, FIELD_NAME, new BitBuffer(bb)); + + assertNotNull(fieldDef); + String eventString = fieldDef.toString(); + assertNotEquals("Non string array read as a string", "Test", eventString); + assertTrue("String should start with \"[0x54, 0x65, 0x73, 0x74,\" " + eventString, eventString.startsWith("[0x54, 0x65, 0x73, 0x74,")); } /** @@ -120,8 +224,7 @@ public void testParseField_simple2() { IntegerDefinition fieldDef = new IntegerDefinition( IntegerDeclaration.createDeclaration(1, false, 1, ByteOrder.BIG_ENDIAN, - Encoding.ASCII, "", 8, null), null, fieldName, 1L); - + Encoding.ASCII, "", 8, null), null, FIELD_NAME, 1L); assertNotNull(fieldDef); } @@ -131,11 +234,11 @@ @Test public void testParseField_simple3() { StringDefinition fieldDef = new StringDefinition( - StringDeclaration.getStringDeclaration(Encoding.UTF8), null, fieldName, "Hello World"); + StringDeclaration.getStringDeclaration(Encoding.UTF8), null, FIELD_NAME, "Hello World"); String other = "\"Hello World\""; assertNotNull(fieldDef); assertEquals(fieldDef.toString(), other); } -} \ No newline at end of file +}
diff --git a/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/ctf/core/event/types/CompoundDeclaration.java b/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/ctf/core/event/types/CompoundDeclaration.java index 4600575..590c301 100644 --- a/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/ctf/core/event/types/CompoundDeclaration.java +++ b/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/ctf/core/event/types/CompoundDeclaration.java
@@ -21,7 +21,8 @@ */ public abstract class CompoundDeclaration extends Declaration { - private static final int BIT_MASK = 0x03; + private static final long BIT_MASK = 0x03; + private static final boolean[] ALIGNED_BYTES_ALIGNMENT = { true, true, true, false, true, false, false, false }; private static final int BITS_PER_BYTE = 8; /** @@ -63,7 +64,13 @@ IDeclaration elementType = getElementType(); if (elementType instanceof IntegerDeclaration) { IntegerDeclaration elemInt = (IntegerDeclaration) elementType; - return (elemInt.getLength() == BITS_PER_BYTE) && ((getAlignment() & BIT_MASK) == 0); + long alignment = getAlignment(); + int lowBits = (int) (alignment & BIT_MASK); + /* + * Make sure that the alignment is 0, 1, 2 ,4 or 8. This would mean + * the bytes are aligned. + */ + return (elemInt.getLength() == BITS_PER_BYTE) && alignment <= BITS_PER_BYTE && ALIGNED_BYTES_ALIGNMENT[lowBits]; } return false; }
diff --git a/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/ArrayDeclaration.java b/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/ArrayDeclaration.java index 3f43bf9..86bd2a8 100644 --- a/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/ArrayDeclaration.java +++ b/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/ArrayDeclaration.java
@@ -110,7 +110,6 @@ throw new CTFException("Buffer underflow"); //$NON-NLS-1$ } input.get(data); - return new ByteArrayDefinition(this, definitionScope, fieldName, data); } @NonNull List<@NonNull Definition> definitions = read(input, definitionScope, fieldName);
diff --git a/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/ArrayDefinition.java b/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/ArrayDefinition.java index 28ffa26..3df431d 100644 --- a/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/ArrayDefinition.java +++ b/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/ArrayDefinition.java
@@ -22,6 +22,7 @@ import org.eclipse.tracecompass.ctf.core.event.types.AbstractArrayDefinition; import org.eclipse.tracecompass.ctf.core.event.types.CompoundDeclaration; import org.eclipse.tracecompass.ctf.core.event.types.Definition; +import org.eclipse.tracecompass.ctf.core.event.types.IntegerDefinition; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; @@ -89,6 +90,27 @@ @Override public String toString() { + if (fDefinitions.isEmpty() ) { + return "[]"; //$NON-NLS-1$ + } + Definition firstDefinition = fDefinitions.get(0); + if (firstDefinition instanceof IntegerDefinition) { + IntegerDefinition integerDefinition = (IntegerDefinition) firstDefinition; + if (integerDefinition.getDeclaration().isCharacter()) { + // String will return "String", no brackets. + // Brackets can be added by others like fields + StringBuilder b = new StringBuilder(); + for (Definition definition : fDefinitions) { + // Blind cast since all definitions are of the same type. + // 0 means the string is terminated. + if (((IntegerDefinition) definition).getIntegerValue() == 0) { + return b.toString(); + } + b.append(definition); + } + return b.toString(); + } + } StringBuilder b = new StringBuilder(); b.append('['); Joiner joiner = Joiner.on(", ").skipNulls(); //$NON-NLS-1$
diff --git a/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/ByteArrayDefinition.java b/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/ByteArrayDefinition.java index 39f667a..b00b4e2 100644 --- a/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/ByteArrayDefinition.java +++ b/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/ByteArrayDefinition.java
@@ -14,6 +14,7 @@ package org.eclipse.tracecompass.internal.ctf.core.event.types; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -23,6 +24,7 @@ import org.eclipse.tracecompass.ctf.core.event.types.AbstractArrayDefinition; import org.eclipse.tracecompass.ctf.core.event.types.CompoundDeclaration; import org.eclipse.tracecompass.ctf.core.event.types.Definition; +import org.eclipse.tracecompass.ctf.core.event.types.IDeclaration; import org.eclipse.tracecompass.ctf.core.event.types.IntegerDeclaration; import org.eclipse.tracecompass.ctf.core.event.types.IntegerDefinition; @@ -87,7 +89,8 @@ @Override public String toString() { - if (((CompoundDeclaration) getDeclaration()).isString()) { + CompoundDeclaration compoundDeclaration = (CompoundDeclaration) getDeclaration(); + if (compoundDeclaration.isString()) { /* * the string is a byte array and may contain more than the string * plus a null char, this will truncate it back to a null char @@ -104,7 +107,25 @@ } StringBuilder b = new StringBuilder(); b.append('['); - Joiner.on(", ").appendTo(b, Arrays.asList(fContent)); //$NON-NLS-1$ + IDeclaration elementType = compoundDeclaration.getElementType(); + // Handle 0xff hex + boolean isHex = false; + // this should ALWAYS be true, but in case... + if (elementType instanceof IntegerDeclaration) { + IntegerDeclaration integerDeclaration = (IntegerDeclaration) elementType; + isHex = integerDeclaration.getBase() == 16; + } + List<String> elements = new ArrayList<>(); + if (isHex) { + for (byte element : fContent) { + elements.add(String.format("0x%02X", element)); //$NON-NLS-1$ + } + } else { + for (byte element : fContent) { + elements.add(String.format("%d", element)); //$NON-NLS-1$ + } + } + Joiner.on(", ").appendTo(b, elements); //$NON-NLS-1$ b.append(']'); return b.toString(); }
diff --git a/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/StructDeclarationFlattener.java b/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/StructDeclarationFlattener.java index 74047e0..266efaa 100644 --- a/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/StructDeclarationFlattener.java +++ b/ctf/org.eclipse.tracecompass.ctf.core/src/org/eclipse/tracecompass/internal/ctf/core/event/types/StructDeclarationFlattener.java
@@ -91,16 +91,7 @@ if (dec instanceof ISimpleDatatypeDeclaration) { flatStruct.addField(path, dec); } else if (dec instanceof ArrayDeclaration) { - ArrayDeclaration ad = (ArrayDeclaration) dec; - int lastIndexOf = path.lastIndexOf('.'); - String name = (lastIndexOf > 0) ? path.substring(lastIndexOf) : path; - if (ad.isAlignedBytes()) { - flatStruct.addField(path, dec); - } else { - for (int i = 0; i < ad.getLength(); i++) { - depthFirstAdd(path + '.' + name + '[' + i + ']', flatStruct, ad.getElementType()); - } - } + flatStruct.addField(path, dec); } else if (dec instanceof StructDeclaration) { StructDeclaration sDec = ((StructDeclaration) dec); for (String name : sDec.getFieldsList()) {
diff --git a/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/event/CtfTmfEventFieldTest.java b/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/event/CtfTmfEventFieldTest.java index ed1ec17..83b501d 100644 --- a/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/event/CtfTmfEventFieldTest.java +++ b/ctf/org.eclipse.tracecompass.tmf.ctf.core.tests/src/org/eclipse/tracecompass/tmf/ctf/core/tests/event/CtfTmfEventFieldTest.java
@@ -252,7 +252,7 @@ public void testParseField_string() { IDefinition fieldDef = fixture.lookupDefinition(STR); CtfTmfEventField result = CtfTmfEventField.parseField(fieldDef, NAME); - assertEquals("test=two", result.toString()); + assertEquals("test=\"two\"", result.toString()); } /** @@ -263,7 +263,7 @@ public void testParseField_array_string() { IDefinition fieldDef = fixture.lookupArrayDefinition(ARRAY_STR); CtfTmfEventField result = CtfTmfEventField.parseField(fieldDef, NAME); - assertEquals("test=[two, two]", result.toString()); + assertEquals("test=[\"two\", \"two\"]", result.toString()); } /** @@ -273,7 +273,7 @@ public void testParseField_struct() { IDefinition fieldDef = fixture.lookupDefinition(STRUCT); CtfTmfEventField result = CtfTmfEventField.parseField(fieldDef, NAME); - assertEquals("test=[str=two, int=2]", result.toString()); + assertEquals("test=[str=\"two\", int=2]", result.toString()); } /** @@ -284,7 +284,7 @@ public void testParseField_array_struct() { IDefinition fieldDef = fixture.lookupArrayDefinition(ARRAY_STRUCT); CtfTmfEventField result = CtfTmfEventField.parseField(fieldDef, NAME); - assertEquals("test=[[str=two, int=2], [str=two, int=2]]", result.toString()); + assertEquals("test=[[str=\"two\", int=2], [str=\"two\", int=2]]", result.toString()); } /**
diff --git a/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/tmf/ctf/core/event/CtfTmfEventField.java b/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/tmf/ctf/core/event/CtfTmfEventField.java index 6d54bc7..d156b50 100644 --- a/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/tmf/ctf/core/event/CtfTmfEventField.java +++ b/ctf/org.eclipse.tracecompass.tmf.ctf.core/src/org/eclipse/tracecompass/tmf/ctf/core/event/CtfTmfEventField.java
@@ -282,6 +282,11 @@ public String getValue() { return (String) super.getValue(); } + + @Override + public String getFormattedValue() { + return "\""+getValue()+"\""; //$NON-NLS-1$//$NON-NLS-2$ + } } /**