diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/data/EditDataFieldDialog.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/data/EditDataFieldDialog.java index 0a937a984c..bc2051480e 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/data/EditDataFieldDialog.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/data/EditDataFieldDialog.java @@ -183,10 +183,11 @@ public class EditDataFieldDialog extends DialogComponentProvider { boolean hasNameChange() { String newName = getNewFieldName(); - if (newName.equals(component.getFieldName())) { - return false; + String currentName = component.getFieldName(); + if (currentName == null) { + currentName = component.getDefaultFieldName(); } - if (newName.equals(component.getDefaultFieldName())) { + if (newName.equals(currentName)) { return false; } return true; diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge1Test.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge1Test.java index d30d792b64..ee5eaa4599 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge1Test.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge1Test.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -98,8 +98,9 @@ public class DataTypeMerge1Test extends AbstractDataTypeMergeTest { @Override public void modifyLatest(ProgramDB program) { // change the name - Category c = program.getDataTypeManager().getCategory( - new CategoryPath("/Category1/Category2/Category5")); + Category c = program.getDataTypeManager() + .getCategory( + new CategoryPath("/Category1/Category2/Category5")); try { c.createCategory("AnotherCategory"); } @@ -259,8 +260,9 @@ public class DataTypeMerge1Test extends AbstractDataTypeMergeTest { @Override public void modifyLatest(ProgramDB program) { // change the name - Category c = program.getDataTypeManager().getCategory( - new CategoryPath("/Category1/Category2/Category5")); + Category c = program.getDataTypeManager() + .getCategory( + new CategoryPath("/Category1/Category2/Category5")); try { c.createCategory("AnotherCategory"); Structure dt = new StructureDataType("Test", 0); @@ -312,8 +314,9 @@ public class DataTypeMerge1Test extends AbstractDataTypeMergeTest { @Override public void modifyLatest(ProgramDB program) { // change the name - Category c = program.getDataTypeManager().getCategory( - new CategoryPath("/Category1/Category2/Category5")); + Category c = program.getDataTypeManager() + .getCategory( + new CategoryPath("/Category1/Category2/Category5")); try { c.createCategory("AnotherCategory"); StructureDataType dt = new StructureDataType("Test", 0); @@ -442,7 +445,7 @@ public class DataTypeMerge1Test extends AbstractDataTypeMergeTest { // /Category1/Category2/Category3 DataType dt = dtm.getDataType(new CategoryPath("/Category1/Category2/Category3"), "IntStruct"); - dtm.remove(dt, TaskMonitor.DUMMY); + dtm.remove(dt, TaskMonitor.DUMMY); } }); executeMerge(DataTypeMergeManager.OPTION_MY); @@ -525,7 +528,7 @@ public class DataTypeMerge1Test extends AbstractDataTypeMergeTest { public void modifyLatest(ProgramDB program) { DataTypeManager dtm = program.getDataTypeManager(); DataType dt = - dtm.getDataType(new CategoryPath("/Category1/Category2"), "CoolUnion"); + dtm.getDataType(new CategoryPath("/Category1/Category2"), "CoolUnion"); dtm.remove(dt, TaskMonitor.DUMMY); } @@ -603,7 +606,7 @@ public class DataTypeMerge1Test extends AbstractDataTypeMergeTest { // /Category1/Category2/Category3 DataType dt = dtm.getDataType(new CategoryPath("/Category1/Category2/Category3"), "IntStruct"); - + try { dt.setName("MyIntStruct"); } @@ -952,7 +955,7 @@ public class DataTypeMerge1Test extends AbstractDataTypeMergeTest { (Structure) dtm.getDataType(new CategoryPath("/Category1/Category2/Category3"), "IntStruct"); DataTypeComponent dtc = s.getComponent(2); - assertEquals("My Field Three", dtc.getFieldName()); + assertEquals("My_Field_Three", dtc.getFieldName()); assertEquals("my comments for Field 3", dtc.getComment()); dtc = s.getComponent(0); diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge3Test.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge3Test.java index e1bf3354fc..1bd9792443 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge3Test.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge3Test.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -143,7 +143,7 @@ public class DataTypeMerge3Test extends AbstractDataTypeMergeTest { DataType dt = dtcs[3].getDataType(); assertTrue(dt.isEquivalent(new FloatDataType())); assertEquals("my comments", dtcs[3].getComment()); - assertEquals("Float Field", dtcs[3].getFieldName()); + assertEquals("Float_Field", dtcs[3].getFieldName()); } @Test @@ -201,7 +201,7 @@ public class DataTypeMerge3Test extends AbstractDataTypeMergeTest { DataType dt = dtcs[5].getDataType(); assertTrue(dt.isEquivalent(new FloatDataType())); assertEquals("my comments", dtcs[5].getComment()); - assertEquals("Float Field", dtcs[5].getFieldName()); + assertEquals("Float_Field", dtcs[5].getFieldName()); } @Test @@ -429,7 +429,7 @@ public class DataTypeMerge3Test extends AbstractDataTypeMergeTest { assertEquals(3, barComps.length); assertEquals(bar, coolUnionComps[5].getDataType()); - assertEquals("My field name", coolUnionComps[5].getFieldName()); + assertEquals("My_field_name", coolUnionComps[5].getFieldName()); assertEquals("My comments", coolUnionComps[5].getComment()); assertTrue(barComps[2].getDataType() instanceof BadDataType); @@ -1168,7 +1168,7 @@ public class DataTypeMerge3Test extends AbstractDataTypeMergeTest { dtcs = union.getComponents(); assertEquals(6, dtcs.length); assertEquals("my comments", dtcs[5].getComment()); - assertEquals("Float Field", dtcs[5].getFieldName()); + assertEquals("Float_Field", dtcs[5].getFieldName()); } @@ -1237,7 +1237,7 @@ public class DataTypeMerge3Test extends AbstractDataTypeMergeTest { dtcs = union.getComponents(); assertEquals(4, dtcs.length); assertEquals("my comments", dtcs[3].getComment()); - assertEquals("Float Field", dtcs[3].getFieldName()); + assertEquals("Float_Field", dtcs[3].getFieldName()); } @Test diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge5Test.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge5Test.java index 13f4f1ffe9..d4faca2fc1 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge5Test.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge5Test.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -40,7 +40,7 @@ import ghidra.util.task.TaskMonitor; public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { @Test - public void testTypeDefUndefined() throws Exception { + public void testTypeDefUndefined() throws Exception { mtf.initialize("notepad", new ProgramModifierListener() { @@ -118,7 +118,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { } @Test - public void testTypeDefs() throws Exception { + public void testTypeDefs() throws Exception { mtf.initialize("notepad", new ProgramModifierListener() { @@ -219,7 +219,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { } @Test - public void testTypeDefs2() throws Exception { + public void testTypeDefs2() throws Exception { mtf.initialize("notepad", new ProgramModifierListener() { @@ -300,7 +300,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { DataTypeComponent[] dtcs = union.getComponents(); assertEquals(7, dtcs.length); - assertEquals("typedef field name TD_MyEnumPointer", dtcs[6].getFieldName()); + assertEquals("typedef_field_name_TD_MyEnumPointer", dtcs[6].getFieldName()); assertEquals("a typedef", dtcs[6].getComment()); Enum enumm = (Enum) dtm.getDataType(new CategoryPath("/Category1"), "MyEnum"); @@ -328,7 +328,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { } @Test - public void testTypeDefs3() throws Exception { + public void testTypeDefs3() throws Exception { mtf.initialize("notepad2", new ProgramModifierListener() { @@ -420,7 +420,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { DataTypeComponent[] dtcs = union.getComponents(); assertEquals(7, dtcs.length); - assertEquals("typedef field name TD_MyEnumPointer", dtcs[6].getFieldName()); + assertEquals("typedef_field_name_TD_MyEnumPointer", dtcs[6].getFieldName()); assertEquals("a typedef", dtcs[6].getComment()); Enum enumm = (Enum) dtm.getDataType(new CategoryPath("/MISC"), "FavoriteColors"); @@ -451,7 +451,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { } @Test - public void testTypeDefs4() throws Exception { + public void testTypeDefs4() throws Exception { mtf.initialize("notepad2", new ProgramModifierListener() { @@ -541,7 +541,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { DataTypeComponent[] dtcs = union.getComponents(); assertEquals(7, dtcs.length); - assertEquals("typedef field name TD_MyEnumPointer", dtcs[6].getFieldName()); + assertEquals("typedef_field_name_TD_MyEnumPointer", dtcs[6].getFieldName()); assertEquals("a typedef", dtcs[6].getComment()); Enum enumm = (Enum) dtm.getDataType(new CategoryPath("/MISC"), "FavoriteColors"); @@ -572,14 +572,14 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { } @Test - public void testTypeDefs5() throws Exception { + public void testTypeDefs5() throws Exception { mtf.initialize("notepad2", new ProgramModifierListener() { @Override public void modifyLatest(ProgramDB program) { DataTypeManager dtm = program.getDataTypeManager(); - + Structure s = (Structure) dtm.getDataType(CategoryPath.ROOT, "DLL_Table"); dtm.remove(s, TaskMonitor.DUMMY); DataType dt = @@ -667,7 +667,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { DataTypeComponent[] dtcs = union.getComponents(); assertEquals(6, dtcs.length); - assertEquals("Float Field", dtcs[5].getFieldName()); + assertEquals("Float_Field", dtcs[5].getFieldName()); assertEquals("my comments", dtcs[5].getComment()); TypeDef td = (TypeDef) dtm.getDataType(new CategoryPath("/Category1"), "TD_MyEnumPointer"); @@ -681,14 +681,14 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { } @Test - public void testTypeDefs6() throws Exception { + public void testTypeDefs6() throws Exception { mtf.initialize("notepad2", new ProgramModifierListener() { @Override public void modifyLatest(ProgramDB program) { DataTypeManager dtm = program.getDataTypeManager(); - + Structure s = (Structure) dtm.getDataType(CategoryPath.ROOT, "DLL_Table"); dtm.remove(s, TaskMonitor.DUMMY); DataType dt = @@ -773,7 +773,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { DataTypeComponent[] dtcs = union.getComponents(); assertEquals(7, dtcs.length); - assertEquals("typedef field name TD_MyEnumPointer *", dtcs[6].getFieldName()); + assertEquals("typedef_field_name_TD_MyEnumPointer_*", dtcs[6].getFieldName()); assertEquals("a pointer to a typedef", dtcs[6].getComment()); Enum enumm = (Enum) dtm.getDataType(new CategoryPath("/MISC"), "FavoriteColors"); @@ -806,7 +806,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { } @Test - public void testTypeDefs7() throws Exception { + public void testTypeDefs7() throws Exception { mtf.initialize("notepad2", new ProgramModifierListener() { @@ -901,7 +901,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { DataTypeComponent[] dtcs = union.getComponents(); assertEquals(7, dtcs.length); - assertEquals("typedef field name", dtcs[6].getFieldName()); + assertEquals("typedef_field_name", dtcs[6].getFieldName()); assertEquals("a typedef on a pointer to a typedef", dtcs[6].getComment()); Enum enumm = (Enum) dtm.getDataType(new CategoryPath("/MISC"), "FavoriteColors"); @@ -941,10 +941,10 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { } @Test - public void testTypeDefs8() throws Exception { + public void testTypeDefs8() throws Exception { mtf.initialize("notepad2", new ProgramModifierListener() { - + @Override public void modifyLatest(ProgramDB program) { DataTypeManager dtm = program.getDataTypeManager(); @@ -1039,7 +1039,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { DataTypeComponent[] dtcs = union.getComponents(); assertEquals(7, dtcs.length); - assertEquals("array of typedef field name", dtcs[6].getFieldName()); + assertEquals("array_of_typedef_field_name", dtcs[6].getFieldName()); assertEquals("an array of typedefs on a pointer to a typedef", dtcs[6].getComment()); Enum enumm = (Enum) dtm.getDataType(new CategoryPath("/MISC"), "FavoriteColors"); @@ -1083,7 +1083,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { } @Test - public void testTypeDefs9() throws Exception { + public void testTypeDefs9() throws Exception { mtf.initialize("notepad2", new ProgramModifierListener() { @@ -1221,7 +1221,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { @Override public void modifyOriginal(ProgramDB program) throws Exception { DataTypeManager dtm = program.getDataTypeManager(); - + // must specify datatype manager when constructing to allow for settings to be made Pointer p = dtm.getPointer(CharDataType.dataType); TypeDef td = @@ -1242,7 +1242,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { @Override public void modifyLatest(ProgramDB program) { DataTypeManager dtm = program.getDataTypeManager(); - + TypeDef td = (TypeDef) dtm.getDataType(new CategoryPath("/MISC"), "PtrTypeDef"); Settings settings = td.getDefaultSettings(); @@ -1320,7 +1320,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { @Override public void modifyLatest(ProgramDB program) { DataTypeManager dtm = program.getDataTypeManager(); - + try { TypeDef td = (TypeDef) dtm.getDataType(new CategoryPath("/MISC"), "Foo * " + formatAttributes("image-base-relative")); @@ -1342,7 +1342,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { @Override public void modifyPrivate(ProgramDB program) { DataTypeManager dtm = program.getDataTypeManager(); - + try { TypeDef td = (TypeDef) dtm.getDataType(new CategoryPath("/MISC"), "Foo * " + formatAttributes("image-base-relative")); @@ -1396,7 +1396,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { @Override public void modifyOriginal(ProgramDB program) throws Exception { DataTypeManager dtm = program.getDataTypeManager(); - + // must specify datatype manager when constructing to allow for settings to be made Structure foo = (Structure) dtm.getDataType(new CategoryPath("/MISC"), "Foo"); PointerTypedef td = @@ -1408,7 +1408,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { @Override public void modifyLatest(ProgramDB program) { DataTypeManager dtm = program.getDataTypeManager(); - + try { TypeDef td = (TypeDef) dtm.getDataType(new CategoryPath("/MISC"), "Foo * " + formatAttributes("image-base-relative")); @@ -1459,7 +1459,7 @@ public class DataTypeMerge5Test extends AbstractDataTypeMergeTest { } @Test - public void testArrays() throws Exception { + public void testArrays() throws Exception { mtf.initialize("notepad2", new ProgramModifierListener() { diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/util/StringUtilities.java b/Ghidra/Framework/Generic/src/main/java/ghidra/util/StringUtilities.java index 53d4601290..01ea5638ac 100644 --- a/Ghidra/Framework/Generic/src/main/java/ghidra/util/StringUtilities.java +++ b/Ghidra/Framework/Generic/src/main/java/ghidra/util/StringUtilities.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -1207,4 +1207,17 @@ public class StringUtilities { public static String wrapToWidth(String str, int width) { return new LineWrapper(width).append(str).finish(); } + + /** + * Removes any whitespace from start or end of string, then replaces any non-printable + * character (< 32) or spaces (32) with an underscore. + * @param s the string to adjust + * @return a new trimmed string with underscores replacing any non-printable characters. + */ + public static String whitespaceToUnderscores(String s) { + if (s == null) { + return null; + } + return s.trim().replaceAll("[\\x00-\\x20]", "_"); + } } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ComponentDBAdapterV0.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ComponentDBAdapterV0.java index 3bb97c3635..5dc9ebee2d 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ComponentDBAdapterV0.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ComponentDBAdapterV0.java @@ -18,6 +18,7 @@ package ghidra.program.database.data; import java.io.IOException; import db.*; +import ghidra.util.StringUtilities; import ghidra.util.exception.VersionException; /** @@ -73,13 +74,16 @@ class ComponentDBAdapterV0 extends ComponentDBAdapter { @Override DBRecord createRecord(long dataTypeID, long parentID, int length, int ordinal, int offset, String name, String comment) throws IOException { + // Don't allow whitespace in field names. Until we change the API to throw an exception + // when a field name has whitespace, just silently replace whitespace with underscores. + String fieldName = StringUtilities.whitespaceToUnderscores(name); long key = DataTypeManagerDB.createKey(DataTypeManagerDB.COMPONENT, componentTable.getKey()); DBRecord record = ComponentDBAdapter.COMPONENT_SCHEMA.createRecord(key); record.setLongValue(ComponentDBAdapter.COMPONENT_PARENT_ID_COL, parentID); record.setLongValue(ComponentDBAdapter.COMPONENT_OFFSET_COL, offset); record.setLongValue(ComponentDBAdapter.COMPONENT_DT_ID_COL, dataTypeID); - record.setString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL, name); + record.setString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL, fieldName); record.setString(ComponentDBAdapter.COMPONENT_COMMENT_COL, comment); record.setIntValue(ComponentDBAdapter.COMPONENT_SIZE_COL, length); record.setIntValue(ComponentDBAdapter.COMPONENT_ORDINAL_COL, ordinal); @@ -107,5 +111,4 @@ class ComponentDBAdapterV0 extends ComponentDBAdapter { return componentTable.findRecords(new LongField(compositeID), ComponentDBAdapter.COMPONENT_PARENT_ID_COL); } - } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeComponentDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeComponentDB.java index d25c7425c1..7ed48df25c 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeComponentDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeComponentDB.java @@ -227,37 +227,12 @@ class DataTypeComponentDB implements InternalDataTypeComponent { @Override public void setFieldName(String name) throws DuplicateNameException { if (record != null) { - name = checkFieldName(name); - record.setString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL, name); + String fieldName = cleanupFieldName(name); + record.setString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL, fieldName); updateRecord(true); } } - private void checkDuplicateName(String name) throws DuplicateNameException { - DataTypeComponentImpl.checkDefaultFieldName(name); - for (DataTypeComponent comp : parent.getDefinedComponents()) { - if (comp == this) { - continue; - } - if (name.equals(comp.getFieldName())) { - throw new DuplicateNameException("Duplicate field name: " + name); - } - } - } - - private String checkFieldName(String name) throws DuplicateNameException { - if (name != null) { - name = name.trim(); - if (name.length() == 0 || name.equals(getDefaultFieldName())) { - name = null; - } - else { - checkDuplicateName(name); - } - } - return name; - } - @Override public int hashCode() { // It is not expected that these objects ever be put in a hash map @@ -431,9 +406,8 @@ class DataTypeComponentDB implements InternalDataTypeComponent { if (StringUtils.isBlank(comment)) { comment = null; } - // TODO: Need to check field name and throw DuplicateNameException - // name = checkFieldName(name); - record.setString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL, name); + String fieldName = cleanupFieldName(name); + record.setString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL, fieldName); record.setLongValue(ComponentDBAdapter.COMPONENT_DT_ID_COL, dataMgr.getResolvedID(dt)); record.setString(ComponentDBAdapter.COMPONENT_COMMENT_COL, comment); updateRecord(false); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeComponentImpl.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeComponentImpl.java index 48cb1cd47c..449e2052b2 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeComponentImpl.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataTypeComponentImpl.java @@ -58,7 +58,7 @@ public class DataTypeComponentImpl implements InternalDataTypeComponent, Seriali this.ordinal = ordinal; this.offset = offset; this.length = length; - this.fieldName = fieldName; + this.fieldName = cleanupFieldName(fieldName); setDataType(dataType); setComment(comment); } @@ -130,32 +130,7 @@ public class DataTypeComponentImpl implements InternalDataTypeComponent, Seriali @Override public void setFieldName(String name) throws DuplicateNameException { - this.fieldName = checkFieldName(name); - } - - private void checkDuplicateName(String name) throws DuplicateNameException { - checkDefaultFieldName(name); - if (parent == null) { - return; // Bad situation - } - for (DataTypeComponent comp : parent.getDefinedComponents()) { - if (comp != this && name.equals(comp.getFieldName())) { - throw new DuplicateNameException("Duplicate field name: " + name); - } - } - } - - private String checkFieldName(String name) throws DuplicateNameException { - if (name != null) { - name = name.trim(); - if (name.length() == 0 || name.equals(getDefaultFieldName())) { - name = null; - } - else { - checkDuplicateName(name); - } - } - return name; + this.fieldName = cleanupFieldName(name); } public static void checkDefaultFieldName(String fieldName) throws DuplicateNameException { @@ -192,22 +167,20 @@ public class DataTypeComponentImpl implements InternalDataTypeComponent, Seriali /** * Perform special-case component update that does not result in size or alignment changes. * @param name new component name - * @param dt new resolved datatype - * @param cmt new comment + * @param newDataType new resolved datatype + * @param newComment new comment */ - void update(String name, DataType dt, String cmt) { - // TODO: Need to check field name and throw DuplicateNameException - // this.fieldName = = checkFieldName(name); - this.fieldName = name; - this.dataType = dt; - this.comment = StringUtils.isBlank(cmt) ? null : cmt; + void update(String name, DataType newDataType, String newComment) { + this.fieldName = cleanupFieldName(name); + this.dataType = newDataType; + this.comment = StringUtils.isBlank(newComment) ? null : newComment; } @Override - public void update(int ordinal, int offset, int length) { - this.ordinal = ordinal; - this.offset = offset; - this.length = length; + public void update(int newOrdinal, int newOffset, int newLength) { + this.ordinal = newOrdinal; + this.offset = newOffset; + this.length = newLength; } /** diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/InternalDataTypeComponent.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/InternalDataTypeComponent.java index 4c03640b6e..2dd6fbc2d7 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/InternalDataTypeComponent.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/InternalDataTypeComponent.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -15,6 +15,10 @@ */ package ghidra.program.model.data; +import org.apache.commons.lang3.StringUtils; + +import ghidra.util.StringUtilities; + public interface InternalDataTypeComponent extends DataTypeComponent { /** @@ -51,4 +55,19 @@ public interface InternalDataTypeComponent extends DataTypeComponent { return buffer.toString(); } + /** + * Internal method for cleaning up field names. + * @param name the new field name + * @return the name with bad chars removed and also set back to null in the event + * the new name is the default name. + */ + public default String cleanupFieldName(String name) { + // For now, silently convert whitespace to underscores + String fieldName = StringUtilities.whitespaceToUnderscores(name); + if (StringUtils.isBlank(fieldName) || fieldName.equals(getDefaultFieldName())) { + fieldName = null; + } + return fieldName; + } + } diff --git a/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/data/StructureDBTest.java b/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/data/StructureDBTest.java index 32102d1de5..0c3d00ca7a 100644 --- a/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/data/StructureDBTest.java +++ b/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/data/StructureDBTest.java @@ -26,6 +26,7 @@ import com.google.common.collect.Sets; import generic.test.AbstractGenericTest; import ghidra.program.model.data.*; import ghidra.util.InvalidNameException; +import ghidra.util.exception.DuplicateNameException; import ghidra.util.task.TaskMonitor; import ghidra.util.task.TaskMonitorAdapter; @@ -1449,35 +1450,35 @@ public class StructureDBTest extends AbstractGenericTest { assertEquals(dtc1, barStruct.getComponent(6)); } - + @Test public void testSetLength() { assertEquals(8, struct.getLength()); assertEquals(4, struct.getNumComponents()); assertEquals(4, struct.getNumDefinedComponents()); - + struct.setLength(20); assertEquals(20, struct.getLength()); assertEquals(16, struct.getNumComponents()); assertEquals(4, struct.getNumDefinedComponents()); - + // new length is offcut within 3rd component at offset 0x3 which should get cleared struct.setLength(4); assertEquals(4, struct.getLength()); assertEquals(3, struct.getNumComponents()); assertEquals(2, struct.getNumDefinedComponents()); - + // Maximum length supported by GUI editor is ~Integer.MAX_VALUE/10 int len = Integer.MAX_VALUE / 10; struct.setLength(len); assertEquals(len, struct.getLength()); assertEquals(len - 1, struct.getNumComponents()); assertEquals(2, struct.getNumDefinedComponents()); - + len /= 2; - struct.replaceAtOffset(len-2, WordDataType.dataType, -1, "x", null); // will be preserved below - struct.replaceAtOffset(len+2, WordDataType.dataType, -1, "y", null); // will be cleared below + struct.replaceAtOffset(len - 2, WordDataType.dataType, -1, "x", null); // will be preserved below + struct.replaceAtOffset(len + 2, WordDataType.dataType, -1, "y", null); // will be cleared below struct.setLength(len); assertEquals(len, struct.getLength()); assertEquals(len - 2, struct.getNumComponents()); @@ -2616,4 +2617,23 @@ public class StructureDBTest extends AbstractGenericTest { } } + @Test + public void testFieldNameWhitespaceConvertedToUnderscores() throws DuplicateNameException { + StructureDataType newStruct = new StructureDataType("Test", 0); + DataTypeComponent component = newStruct.add(new ByteDataType(), " name with spaces", null); + assertEquals("name_with_spaces", component.getFieldName()); + + struct = (StructureDB) dataMgr.resolve(newStruct, null); + component = struct.getComponent(0); + component.setFieldName("name in db with spaces"); + assertEquals("name_in_db_with_spaces", component.getFieldName()); + + component = struct.add(new ByteDataType(), "another test", null); + assertEquals("another_test", component.getFieldName()); + + struct.insert(0, new ByteDataType(), 1, "insert test", ""); + component = struct.getComponent(0); + assertEquals("insert_test", component.getFieldName()); + } + }