Merge remote-tracking branch 'origin/Ghidra_12.0'

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ComponentDBAdapter.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.
@@ -19,6 +19,7 @@ import java.io.IOException;
 
 import db.*;
 import ghidra.framework.data.OpenMode;
+import ghidra.util.StringUtilities;
 import ghidra.util.exception.VersionException;
 
 /**
@@ -60,7 +61,8 @@ abstract class ComponentDBAdapter {
 	 * @param length the total length of this component.
 	 * @param ordinal the component's ordinal.
 	 * @param offset the component's offset.
-	 * @param name the component's name.
+	 * @param name the component's name.  This name will be subject to revision based upon 
+	 * {@link #cleanUpFieldName(String)} method use.
 	 * @param comment a comment about this component
 	 * @return the component data type record.
 	 * @throws IOException if there is a problem accessing the database.
@@ -86,6 +88,10 @@ abstract class ComponentDBAdapter {
 
 	/**
 	 * Updates the component data type table with the provided record.
+	 * <p>
+	 * IMPORTANT: Any modification of field name should be subject to {@link #cleanUpFieldName(String)}
+	 * use first.
+	 * 
 	 * @param record the new record
 	 * @throws IOException if there is a problem accessing the database.
 	 */

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ComponentDBAdapterV0.java

@@ -18,7 +18,7 @@ package ghidra.program.database.data;
 import java.io.IOException;
 
 import db.*;
-import ghidra.util.StringUtilities;
+import ghidra.program.model.data.InternalDataTypeComponent;
 import ghidra.util.exception.VersionException;
 
 /**
@@ -74,16 +74,14 @@ 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, fieldName);
+		record.setString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL,
+			InternalDataTypeComponent.cleanupFieldName(name));
 		record.setString(ComponentDBAdapter.COMPONENT_COMMENT_COL, comment);
 		record.setIntValue(ComponentDBAdapter.COMPONENT_SIZE_COL, length);
 		record.setIntValue(ComponentDBAdapter.COMPONENT_ORDINAL_COL, ordinal);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeComponentDB.java

@@ -228,7 +228,9 @@ class DataTypeComponentDB implements InternalDataTypeComponent {
 	@Override
 	public String getFieldName() {
 		if (record != null && !isZeroBitFieldComponent()) {
-			return record.getString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL);
+			String fieldName = record.getString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL);
+			// Blank check is required since we improperly allowed storage of blank names in the past
+			return StringUtils.isBlank(fieldName) ? null : fieldName;
 		}
 		return null;
 	}
@@ -236,7 +238,7 @@ class DataTypeComponentDB implements InternalDataTypeComponent {
 	@Override
 	public void setFieldName(String name) throws DuplicateNameException {
 		if (record != null) {
-			String fieldName = cleanupFieldName(name);
+			String fieldName = InternalDataTypeComponent.cleanupFieldName(name);
 			record.setString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL, fieldName);
 			updateRecord(true);
 		}
@@ -415,7 +417,7 @@ class DataTypeComponentDB implements InternalDataTypeComponent {
 			if (StringUtils.isBlank(comment)) {
 				comment = null;
 			}
-			String fieldName = cleanupFieldName(name);
+			String fieldName = InternalDataTypeComponent.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);

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 = cleanupFieldName(fieldName);
+		this.fieldName = InternalDataTypeComponent.cleanupFieldName(fieldName);
 		setDataType(dataType);
 		setComment(comment);
 	}
@@ -130,7 +130,7 @@ public class DataTypeComponentImpl implements InternalDataTypeComponent, Seriali
 
 	@Override
 	public void setFieldName(String name) throws DuplicateNameException {
-		this.fieldName = cleanupFieldName(name);
+		this.fieldName = InternalDataTypeComponent.cleanupFieldName(name);
 	}
 
 	public static void checkDefaultFieldName(String fieldName) throws DuplicateNameException {
@@ -171,7 +171,7 @@ public class DataTypeComponentImpl implements InternalDataTypeComponent, Seriali
 	 * @param newComment new comment
 	 */
 	void update(String name, DataType newDataType, String newComment) {
-		this.fieldName = cleanupFieldName(name);
+		this.fieldName = InternalDataTypeComponent.cleanupFieldName(name);
 		this.dataType = newDataType;
 		this.comment = StringUtils.isBlank(newComment) ? null : newComment;
 	}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/InternalDataTypeComponent.java

@@ -15,8 +15,6 @@
  */
 package ghidra.program.model.data;
 
-import org.apache.commons.lang3.StringUtils;
-
 import ghidra.util.StringUtilities;
 
 public interface InternalDataTypeComponent extends DataTypeComponent {
@@ -56,16 +54,29 @@ public interface InternalDataTypeComponent extends DataTypeComponent {
 	}
 
 	/**
-	 * Internal method for cleaning up field names. 
+	 * Modify field name to transform whitespace chars to underscores after triming and checking
-	 * @param name the new field name
+	 * for empty string.  Empty string is returned as null for storage to indicate default name use. 
-	 * @return the name with bad chars removed and also set back to null in the event
+	 * @param name original field name (may be null) 
-	 * the new name is the default name.
+	 * @return revised field name (may be null)
 	 */
-	public default String cleanupFieldName(String name) {
+	public static String cleanupFieldName(String name) {
-		// For now, silently convert whitespace to underscores
+		String fieldName = name;
-		String fieldName = StringUtilities.whitespaceToUnderscores(name);
+		if (name != null) {
-		if (StringUtils.isBlank(fieldName)) {
+
-			fieldName = null;
+			// Trim field name and ensure empty string is stored as null to indicate default field name
+			fieldName = name.trim();
+
+			if (fieldName.length() == 0) {
+				fieldName = null;
+			}
+			else {
+				// NOTE: Should we be checking for default field name pattern and disallow.
+				// If so, additional parameters would be required (e.g., struct vs union, is packed struct)
+
+				// 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.
+				fieldName = StringUtilities.whitespaceToUnderscores(fieldName);
+			}
 		}
 		return fieldName;
 	}

Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/data/StructureDBTest.java

@@ -2663,15 +2663,58 @@ public class StructureDBTest extends AbstractGenericTest {
 
 		struct = (StructureDB) dataMgr.resolve(newStruct, null);
 		component = struct.getComponent(0);
-		component.setFieldName("name in db with spaces");
+		component.setFieldName(" name in db with spaces ");
 		assertEquals("name_in_db_with_spaces", component.getFieldName());
 
-		component = struct.add(new ByteDataType(), "another test", null);
+		component = struct.add(new ByteDataType(), " another test ", null);
 		assertEquals("another_test", component.getFieldName());
 
-		struct.insert(0, new ByteDataType(), 1, "insert test", "");
+		struct.insert(0, new ByteDataType(), 1, " insert test ", "");
+		component = struct.getComponent(0);
+		assertEquals("insert_test", component.getFieldName());
+
+		struct.replace(0, new ByteDataType(), 1, " insert test ", "");
 		component = struct.getComponent(0);
 		assertEquals("insert_test", component.getFieldName());
 	}
 
+	@Test
+	public void testDefaultFieldNames() throws DuplicateNameException {
+		StructureDataType newStruct = new StructureDataType("Test", 0);
+		DataTypeComponent component = newStruct.add(new ByteDataType(), " ", null);
+		assertNull(component.getFieldName());
+
+		component = newStruct.add(new ByteDataType(), null, null);
+		assertNull(component.getFieldName());
+
+		struct = (StructureDB) dataMgr.resolve(newStruct, null);
+		component = struct.getComponent(0);
+		assertNull(component.getFieldName());
+
+		component.setFieldName(" ");
+		assertNull(component.getFieldName());
+
+		component = struct.add(new ByteDataType(), null, null);
+		assertNull(component.getFieldName());
+
+		component = struct.add(new ByteDataType(), " ", null);
+		assertNull(component.getFieldName());
+
+		struct.insert(0, new ByteDataType(), 1, null, "");
+		component = struct.getComponent(0);
+		assertNull(component.getFieldName());
+
+		struct.insert(0, new ByteDataType(), 1, " ", "");
+		component = struct.getComponent(0);
+		assertNull(component.getFieldName());
+
+		struct.replace(0, new ByteDataType(), 1, null, "");
+		component = struct.getComponent(0);
+		assertNull(component.getFieldName());
+
+		struct.replace(0, new ByteDataType(), 1, " ", "");
+		component = struct.getComponent(0);
+		assertNull(component.getFieldName());
+	}
+
 }