GP-1316 - Enums - Updates for PR to add comments to enum values

Ghidra/Features/Base/src/main/help/help/topics/DataTypeEditors/EnumEditor.htm

@@ -25,17 +25,24 @@
       <ul>
         <li>The <i>Name</i> column in the table is the name of the enum entry.</li>
         <li>The <i>Value</i> column in the table is the value of the enum entry.</li>
+        <li>The <i>Comment</i> column in the table is the comment for the enum entry.</li>
+                
+        
         <li>The <i>Name</i> field below the table shows the name of the enum.
             Edit this field to change the name.
         </li>
         <li>The <i>Description</i> field shows a short description for the enum; 
             edit this field to update the description.
         </li>
-        <li>The <i>Category</i> field shows where the enum resides which corresponds to the folder you were selecting when creating the enum;
+        <li>The <i>Category</i> field shows where the enum resides which corresponds to the folder 
-            the field is not editable, however, you can move it using the Data Type Manager after you have created it if you want to move it. 
+        	you were selecting when creating the enum;
+            the field is not editable, however, you can move it using the Data Type Manager after 
+            you have created it if you want to move it. 
         </li>
         <li>The <i>Size</i> field shows the number of bytes required when you apply
-            the enum. To edit this field, use the dropdown menu to choose a new size. Note: If you are applying an enum to a data definition and do not see expected results in the decompiler it is probably because the size is incorrect.
+            the enum. To edit this field, use the drop-down menu to choose a new size. Note: If you 
+            are applying an enum to a data definition and do not see expected results in the 
+            decompiler it is probably because the size is incorrect.
         </li>
       </ul>
       <p>When you make any change to the enum, the 

Ghidra/Features/Base/src/main/java/ghidra/app/merge/datatypes/DataTypePanel.java

@@ -246,6 +246,30 @@ class DataTypePanel extends JPanel {
 		insertLength(comp);
 	}
 
+	private class EnumEntry implements Comparable<EnumEntry> {
+
+		private final String name;
+		private final long value;
+		private final String comment;
+
+		EnumEntry(String name, long value, String comment) {
+			this.name = name;
+			this.value = value;
+			this.comment = comment;
+
+		}
+
+		@Override
+		public int compareTo(EnumEntry o) {
+			int c = Long.compare(value, o.value);
+			if (c == 0) {
+				c = name.compareTo(o.name);
+			}
+			return c;
+		}
+
+	}
+
 	private void formatEnumText(Enum enuum) {
 		formatSourceArchive(enuum);
 		formatPath(enuum);
@@ -254,28 +278,41 @@ class DataTypePanel extends JPanel {
 
 		StringBuffer sb = new StringBuffer();
 
-		String[] names = enuum.getNames();
+		int maxNameLength = 0;
-		int maxLength = 0;
+		int maxValueLength = 0;
-		for (String name : names) {
-			if (name.length() > maxLength) {
-				maxLength = name.length();
-			}
-		}
-		long[] values = enuum.getValues();
-		Arrays.sort(values);
 
-		for (int i = 0; i < values.length; i++) {
+		String[] names = enuum.getNames();
-			String name = enuum.getName(values[i]);
+		EnumEntry[] entries = new EnumEntry[names.length];
-			name = pad(name, maxLength);
+		for (int i = 0; i < names.length; i++) {
-			sb.append("    " + name + " = 0x" + Long.toHexString(values[i]) + " ");
+			String name = names[i];
-			if (i < values.length - 1) {
+			EnumEntry entry = new EnumEntry(name, enuum.getValue(name), enuum.getComment(name));
-				sb.append("\n");
+			entries[i] = entry;
-			}
+			maxNameLength = Math.max(maxNameLength, name.length());
+			String valStr = Long.toHexString(entry.value);
+			maxValueLength = Math.max(maxValueLength, valStr.length());
 		}
-		sb.append("\n }\n");
+		Arrays.sort(entries);
+
+		for (EnumEntry entry : entries) {
+			renderEnumEntry(entry, maxNameLength, maxValueLength);
+		}
+		sb.append("}\n");
 		insertString(sb.toString(), contentAttrSet);
 	}
 
+	private void renderEnumEntry(EnumEntry entry, int maxNameLength, int maxValueLength) {
+		String name = entry.name;
+		name = pad(name, maxNameLength);
+		String valStr = Long.toHexString(entry.value);
+		valStr = pad(valStr, maxValueLength);
+		insertString("    " + name, fieldNameAttrSet);
+		insertString(" = 0x" + valStr, contentAttrSet);
+		if (entry.comment != null) {
+			insertString("   " + entry.comment, commentAttrSet);
+		}
+		insertString("\n", contentAttrSet);
+	}
+
 	private void formatTypeDefText(TypeDef td) {
 		formatSourceArchive(td);
 		formatPath(td);

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/editor/EnumEntry.java

@@ -1,6 +1,5 @@
 /* ###
  * IP: GHIDRA
- * REVIEWED: YES
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +15,8 @@
  */
 package ghidra.app.plugin.core.datamgr.editor;
 
+import generic.json.Json;
+
 public class EnumEntry {
 	private String name;
 	private long value;
@@ -50,4 +51,9 @@ public class EnumEntry {
 	public void setComment(String newComment) {
 		this.comment = newComment;
 	}
+
+	@Override
+	public String toString() {
+		return Json.toString(this);
+	}
 }

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/editor/EnumTableModel.java

@@ -266,8 +266,8 @@ class EnumTableModel extends AbstractSortedTableModel<EnumEntry> {
 	}
 
 	private long findNextValue(int afterRow) {
-		if (enumEntryList.size() == 0) {
+		if (enumEntryList.isEmpty()) {
-			return new Long(0);
+			return 0;
 		}
 		if (afterRow < 0 || afterRow >= enumEntryList.size()) {
 			afterRow = 0;

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/prototype/dataArchiveUtilities/ComplexName.java

@@ -1,141 +0,0 @@
-/* ###
- * IP: GHIDRA
- * REVIEWED: YES
- *
- * 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.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-/*
- * Created on Aug 9, 2005
- *
- * TODO To change the template for this generated file go to
- * Window - Preferences - Java - Code Style - Code Templates
- */
-package ghidra.app.plugin.prototype.dataArchiveUtilities;
-
-import ghidra.program.model.data.*;
-import ghidra.util.Msg;
-
-import java.util.Hashtable;
-
-public class ComplexName {
-	boolean isPointer;
-	boolean isArray;
-	boolean usesNamespace;
-	String complexName;
-	String namespace;
-	String name;
-	int count = 1;
-	
-	public ComplexName(String complexName) {
-		this.complexName = complexName;
-		if (complexName.startsWith("Pointer%")) {
-			isPointer = true;
-			complexName = complexName.substring(complexName.indexOf("%")+1);
-		}
-		if (complexName.startsWith("Array%")) {
-			isArray = true;
-			complexName = complexName.substring(complexName.indexOf("%")+1);
-			String countStr = complexName.substring(0, complexName.indexOf("%"));
-			if (countStr.compareToIgnoreCase("") != 0) {
-				count = new Integer(countStr).intValue();
-			} else {
-				count = 1;
-			}
-			complexName = complexName.substring(complexName.indexOf("%")+1);
-			complexName = complexName.substring(complexName.indexOf("%")+1);
-		}
-		usesNamespace = (complexName.indexOf(":") >= 0);
-		if (usesNamespace) {
-			namespace = complexName.substring(complexName.indexOf("/"), complexName.indexOf(":"));
-			name      = complexName.substring(0, complexName.indexOf("/"))+
-					    complexName.substring(complexName.indexOf(":")+1, complexName.length());
-		} else {
-			namespace = "";
-			name      = complexName;
-		}
-	}
-	public boolean isArray() {
-		return isArray;
-	}
-	public boolean isPointer() {
-		return isPointer;
-	}
-	public String getName() {
-		return name;
-	}
-	public String getNamespace() {
-		return namespace;
-	}
-	public boolean usesNamespace() {
-		return usesNamespace;
-	}
-	public CategoryPath getCategoryPath() {
-		return (usesNamespace ? new CategoryPath(namespace) : CategoryPath.ROOT);
-	}
-	
-	public DataType getDataType(FileDataTypeManager dtMgr, Hashtable<String,DataType> dataTypes) {
-		return getDataType(dtMgr, dataTypes, true);
-	}
-	public DataType getDataType(DataTypeManager dtMgr, Hashtable<String,DataType> dataTypes, boolean generateUI) {
-		DataType dt;
-		if (name.indexOf("%") >= 0) {
-			ComplexName cName = new ComplexName(name);
-			dt = cName.getDataType(dtMgr, dataTypes, generateUI);
-			name = cName.getName();
-		} else {
-			if (dataTypes == null) {
-				dt = dtMgr.getDataType(getCategoryPath(), getName());
-			} else {
-				dt = dataTypes.get(getCategoryPath()+getName());
-			}
-			//if (dt == null) {
-			//	dt = dtMgr.getDataType(getCategoryPath(), getName());
-			//}
-		}
-		if (dt == null) { 
-			if (generateUI) {
-				// Add placeholders for the archive entries that are missing
-				dt = genUIData(dtMgr, dataTypes, new ComplexName(complexName), 4);
-			} else {
-			    Msg.warn(this, "Data type ("+name+") not found.");
-				return dt;
-			}
-		}
-		if (isPointer) {
-			dt = new Pointer32DataType(dt);
-		}
-		if (isArray) {
-			if (dt.getLength() >= 0) {
-				dt = new ArrayDataType(dt, count, dt.getLength());
-			} else {
-			    Msg.error(this, "Error in array length ("+dt.getLength()+") for "+dt.getName());
-				return null;
-			}
-		}
-		return dt;
-	}
-	
-	public DataType genUIData(DataTypeManager dtMgr, Hashtable<String,DataType> dataTypes, ComplexName cName, int len) {
-		TypedefDataType dt = new TypedefDataType(cName.getCategoryPath(), cName.getName(), 
-				new ArrayDataType(new ByteDataType(), len, 1));
-    	addDataType(dtMgr, dataTypes, dt);
-		return dt;
-    }
-	
-	private void addDataType(DataTypeManager dtMgr, Hashtable<String,DataType> dataTypes, DataType dt) {	
-		DataType type = dtMgr.addDataType(dt, DataTypeConflictHandler.REPLACE_HANDLER);
-		dataTypes.put(type.getCategoryPath()+type.getName(), type);
-	}
-	
-	
-}

Ghidra/Features/Base/src/main/java/ghidra/app/util/html/EnumDataTypeHTMLRepresentation.java

@@ -40,8 +40,7 @@ public class EnumDataTypeHTMLRepresentation extends HTMLDataTypeRepresentation {
 
 	// private constructor for making diff copies
 	private EnumDataTypeHTMLRepresentation(Enum enumDataType, List<ValidatableLine> headerLines,
-			TextLine displayName,
+			TextLine displayName, List<ValidatableLine> bodyContent, TextLine footerLine) {
-			List<ValidatableLine> bodyContent, TextLine footerLine) {
 		this.enumDataType = enumDataType;
 		this.headerContent = headerLines;
 		this.displayName = displayName;
@@ -113,7 +112,13 @@ public class EnumDataTypeHTMLRepresentation extends HTMLDataTypeRepresentation {
 				int length = hexString.length();
 				hexString = hexString.substring(length - (n * 2));
 			}
-			list.add(new TextLine(name + " = 0x" + hexString));
+
+			String comment = enumDataType.getComment(name);
+			if (trim && comment != null) {
+				comment = StringUtilities.trim(comment, ToolTipUtils.LINE_LENGTH);
+			}
+
+			list.add(new TextLine(name + " = 0x" + hexString + "    " + comment));
 		}
 
 		return list;
@@ -147,7 +152,7 @@ public class EnumDataTypeHTMLRepresentation extends HTMLDataTypeRepresentation {
 		displayNameText = HTMLUtilities.friendlyEncodeHTML(displayNameText);
 		displayNameText = wrapStringInColor(displayNameText, displayName.getTextColor());
 		//@formatter:off
-		append(fullHtml, truncatedHtml, lineCount, TT_OPEN, 
+		append(fullHtml, truncatedHtml, lineCount, TT_OPEN,
                                                    displayNameText,
                                                    TT_CLOSE,
                                                    HTML_SPACE,
@@ -192,8 +197,8 @@ public class EnumDataTypeHTMLRepresentation extends HTMLDataTypeRepresentation {
 		return fullHtml.toString();
 	}
 
-	private static void append(StringBuilder fullHtml, StringBuilder truncatedHtml,
+	private static void append(StringBuilder fullHtml, StringBuilder truncatedHtml, int lineCount,
-			int lineCount, String... content) {
+			String... content) {
 
 		for (String string : content) {
 			fullHtml.append(string);
@@ -247,8 +252,7 @@ public class EnumDataTypeHTMLRepresentation extends HTMLDataTypeRepresentation {
 				diffDisplayName, bodyDiff.getLeftLines(), footerLine),
 			new EnumDataTypeHTMLRepresentation(enumRepresentation.enumDataType,
 				headerDiff.getRightLines(), otherDiffDisplayName, bodyDiff.getRightLines(),
-				enumRepresentation.footerLine)
+				enumRepresentation.footerLine) };
-		};
 	}
 
 }

Ghidra/Features/Base/src/main/java/ghidra/app/util/html/HTMLDataTypeRepresentation.java

@@ -70,7 +70,7 @@ public abstract class HTMLDataTypeRepresentation {
 	// Note 1: Indentation tags (note: we switched from <DIV> tags because the Java rendering engine
 	// does not keep the color of the div's parent tags.  The <P> tag seems to work).
 	// Note 2: Switch back to <DIV> from <P>, since the <P> tag gets broken by the <TABLE> tag
-	// used by composite types.   If not inheriting the color becomes an issue, then we will need 
+	// used by composite types.   If not inheriting the color becomes an issue, then we will need
 	// to find another solution for indentation.
 	protected static final String INDENT_OPEN = "<DIV STYLE='margin-left: 10px;'>";
 	protected static final String INDENT_CLOSE = "</DIV>";
@@ -87,7 +87,7 @@ public abstract class HTMLDataTypeRepresentation {
 	protected final static Color DIFF_COLOR = ValidatableLine.INVALID_COLOR;
 
 	private static String createSpace(int numberOfSpaces) {
-		StringBuffer buffer = new StringBuffer();
+		StringBuilder buffer = new StringBuilder();
 		for (int i = 0; i < numberOfSpaces; i++) {
 			buffer.append(HTML_SPACE);
 		}
@@ -134,7 +134,7 @@ public abstract class HTMLDataTypeRepresentation {
 	 * Returns the plain-text value of the data type's description.
 	 * <p>
 	 * If there were html tags in the string, they are escaped.
-	 * 
+	 *
 	 * @param dataType the type to get the description / comment for
 	 * @return plain-text string, w/html escaped
 	 */
@@ -167,7 +167,7 @@ public abstract class HTMLDataTypeRepresentation {
 	/**
 	 * Formats a multi-line plain-text comment string into a HTML string where the text has been
 	 * wrapped at MAX_LINE_LENGTH.
-	 *   
+	 *
 	 * @param string plain-text string
 	 * @return list of html strings
 	 */
@@ -241,9 +241,9 @@ public abstract class HTMLDataTypeRepresentation {
 
 	/**
 	 * Formats a multi-line plain-text comment as a list of HTML marked-up lines.
-	 *  
+	 *
 	 * @param comment multi-line plain-text string
-	 * @param maxLines max number of formatted lines to return 
+	 * @param maxLines max number of formatted lines to return
 	 * @return list of html marked-up {@link TextLine}s
 	 */
 	protected static List<TextLine> createCommentLines(String comment, int maxLines) {
@@ -325,9 +325,9 @@ public abstract class HTMLDataTypeRepresentation {
 	}
 
 	/**
-	 * Returns an HTML string for this data representation object.  The HTML returned will be 
+	 * Returns an HTML string for this data representation object.  The HTML returned will be
 	 * truncated if it is too long.   To get the full HTML, call {@link #getFullHTMLString()}.
-	 * 
+	 *
 	 * @return the html
 	 * @see #getFullHTMLString()
 	 */
@@ -337,7 +337,7 @@ public abstract class HTMLDataTypeRepresentation {
 
 	/**
 	 * Returns an HTML string for this data representation object
-	 * 
+	 *
 	 * @return the html
 	 * @see #getHTMLString()
 	 */
@@ -345,16 +345,16 @@ public abstract class HTMLDataTypeRepresentation {
 		return HTML_OPEN + originalHTMLData + HTML_CLOSE;
 	}
 
-	/** 
+	/**
-	 * This is like {@link #getHTMLString()}, but does not put HTML tags around the data 
+	 * This is like {@link #getHTMLString()}, but does not put HTML tags around the data
 	 * @return the content
 	 */
 	public String getHTMLContentString() {
 		return originalHTMLData; // default to full text; subclasses can override
 	}
 
-	/** 
+	/**
-	* This is like {@link #getHTMLString()}, but does not put HTML tags around the data 
+	* This is like {@link #getHTMLString()}, but does not put HTML tags around the data
 	* @return the content
 	*/
 	public String getFullHTMLContentString() {

Ghidra/Features/Base/src/test.slow/java/ghidra/app/merge/datatypes/DataTypeMerge2Test.java

@@ -1115,12 +1115,8 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest {
 
 	@Test
 	public void testEditEnum() throws Exception {
-		// edit DLL_Table in latest; edit DLL_Table in private
+
-		// only DLL_Table should be in conflict; not the ones where it is used.
 		mtf.initialize("notepad", new ProgramModifierListener() {
-			/* (non-Javadoc)
-			 * @see ghidra.framework.data.ProgramModifierListener#modifyLatest(ghidra.program.database.ProgramDB)
-			 */
 			@Override
 			public void modifyLatest(ProgramDB program) {
 				boolean commit = false;
@@ -1140,9 +1136,6 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest {
 				}
 			}
 
-			/* (non-Javadoc)
-			 * @see ghidra.framework.data.ProgramModifierListener#modifyPrivate(ghidra.program.database.ProgramDB)
-			 */
 			@Override
 			public void modifyPrivate(ProgramDB program) {
 				boolean commit = false;
@@ -1180,7 +1173,164 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest {
 		assertNotNull(dt);
 		Enum enumm = (Enum) dt;
 		assertEquals(0x10, enumm.getValue("Pink"));
+	}
 
+	@Test
+	public void testEditEnumComments_NoConflict_CommentAddedInLatest() throws Exception {
+
+		mtf.initialize("notepad", new ProgramModifierListener() {
+
+			@Override
+			public void modifyLatest(ProgramDB program) {
+				boolean commit = false;
+				DataTypeManager dtm = program.getDataTypeManager();
+				int transactionID = program.startTransaction("test");
+				Category c = dtm.getCategory(new CategoryPath("/MISC"));
+				DataType dt = c.getDataType("FavoriteColors");
+				try {
+					Enum enumm = (Enum) dt;
+					String valueName = "Pink";
+					long value = enumm.getValue(valueName);
+					enumm.remove(valueName);
+					enumm.add(valueName, value, "This is the latest comment on server");
+					commit = true;
+				}
+				finally {
+					program.endTransaction(transactionID, commit);
+				}
+			}
+
+			@Override
+			public void modifyPrivate(ProgramDB program) {
+				// no change
+			}
+
+		});
+		executeMerge(DataTypeMergeManager.OPTION_MY);
+		DataTypeManager dtm = resultProgram.getDataTypeManager();
+
+		Category c = dtm.getCategory(new CategoryPath("/MISC"));
+		DataType dt = c.getDataType("FavoriteColors");
+		assertNotNull(dt);
+		Enum enumm = (Enum) dt;
+		assertEquals(0x3, enumm.getValue("Pink"));
+		assertEquals("This is the latest comment on server", enumm.getComment("Pink"));
+	}
+
+	@Test
+	public void testEditEnumComments_Conflict_TakeMyChanges() throws Exception {
+
+		mtf.initialize("notepad", new ProgramModifierListener() {
+
+			@Override
+			public void modifyLatest(ProgramDB program) {
+				boolean commit = false;
+				DataTypeManager dtm = program.getDataTypeManager();
+				int transactionID = program.startTransaction("test");
+				Category c = dtm.getCategory(new CategoryPath("/MISC"));
+				DataType dt = c.getDataType("FavoriteColors");
+				try {
+					Enum enumm = (Enum) dt;
+					String valueName = "Pink";
+					long value = enumm.getValue(valueName);
+					enumm.remove(valueName);
+					enumm.add(valueName, value, "This is the latest comment on server");
+					commit = true;
+				}
+				finally {
+					program.endTransaction(transactionID, commit);
+				}
+			}
+
+			@Override
+			public void modifyPrivate(ProgramDB program) {
+				boolean commit = false;
+				DataTypeManager dtm = program.getDataTypeManager();
+				int transactionID = program.startTransaction("test");
+				Category c = dtm.getCategory(new CategoryPath("/MISC"));
+				DataType dt = c.getDataType("FavoriteColors");
+
+				try {
+					Enum enumm = (Enum) dt;
+					String valueName = "Pink";
+					long value = enumm.getValue(valueName);
+					enumm.remove(valueName);
+					enumm.add(valueName, value, "This my local updated comment");
+					commit = true;
+				}
+				finally {
+					program.endTransaction(transactionID, commit);
+				}
+			}
+
+		});
+		executeMerge(DataTypeMergeManager.OPTION_MY);
+		DataTypeManager dtm = resultProgram.getDataTypeManager();
+
+		Category c = dtm.getCategory(new CategoryPath("/MISC"));
+		DataType dt = c.getDataType("FavoriteColors");
+		assertNotNull(dt);
+		Enum enumm = (Enum) dt;
+		assertEquals(0x3, enumm.getValue("Pink"));
+		assertEquals("This my local updated comment", enumm.getComment("Pink"));
+	}
+
+	@Test
+	public void testEditEnumComments_Conflict_TakeLatestChanges() throws Exception {
+
+		mtf.initialize("notepad", new ProgramModifierListener() {
+
+			@Override
+			public void modifyLatest(ProgramDB program) {
+				boolean commit = false;
+				DataTypeManager dtm = program.getDataTypeManager();
+				int transactionID = program.startTransaction("test");
+				Category c = dtm.getCategory(new CategoryPath("/MISC"));
+				DataType dt = c.getDataType("FavoriteColors");
+				try {
+					Enum enumm = (Enum) dt;
+					String valueName = "Pink";
+					long value = enumm.getValue(valueName);
+					enumm.remove(valueName);
+					enumm.add(valueName, value, "This is the latest comment on server");
+					commit = true;
+				}
+				finally {
+					program.endTransaction(transactionID, commit);
+				}
+			}
+
+			@Override
+			public void modifyPrivate(ProgramDB program) {
+				boolean commit = false;
+				DataTypeManager dtm = program.getDataTypeManager();
+				int transactionID = program.startTransaction("test");
+				Category c = dtm.getCategory(new CategoryPath("/MISC"));
+				DataType dt = c.getDataType("FavoriteColors");
+
+				try {
+					Enum enumm = (Enum) dt;
+					String valueName = "Pink";
+					long value = enumm.getValue(valueName);
+					enumm.remove(valueName);
+					enumm.add(valueName, value, "This my local updated comment");
+					commit = true;
+				}
+				finally {
+					program.endTransaction(transactionID, commit);
+				}
+			}
+
+		});
+		executeMerge(DataTypeMergeManager.OPTION_LATEST);
+		DataTypeManager dtm = resultProgram.getDataTypeManager();
+
+		Category c = dtm.getCategory(new CategoryPath("/MISC"));
+		DataType dt = c.getDataType("FavoriteColors");
+		assertNotNull(dt);
+		Enum enumm = (Enum) dt;
+		assertEquals(0x3, enumm.getValue("Pink"));
+		assertEquals("This is the latest comment on server", enumm.getComment("Pink"));
 	}
 
 	@Test
@@ -1355,9 +1505,8 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest {
 				DataTypeManager dtm = program.getDataTypeManager();
 				int transactionID = program.startTransaction("test");
 				Structure bar = (Structure) dtm.getDataType(new CategoryPath("/MISC"), "Bar");
-				FunctionDefinition fd =
+				FunctionDefinition fd = (FunctionDefinition) dtm
-					(FunctionDefinition) dtm.getDataType(new CategoryPath("/MISC"),
+						.getDataType(new CategoryPath("/MISC"), "MyFunctionDef");
-						"MyFunctionDef");
 
 				try {
 					fd.setReturnType(bar);
@@ -1376,9 +1525,8 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest {
 				boolean commit = false;
 				DataTypeManager dtm = program.getDataTypeManager();
 				Structure foo = (Structure) dtm.getDataType(new CategoryPath("/MISC"), "Foo");
-				FunctionDefinition fd =
+				FunctionDefinition fd = (FunctionDefinition) dtm
-					(FunctionDefinition) dtm.getDataType(new CategoryPath("/MISC"),
+						.getDataType(new CategoryPath("/MISC"), "MyFunctionDef");
-						"MyFunctionDef");
 				ParameterDefinition[] vars = fd.getArguments();
 
 				int transactionID = program.startTransaction("test");
@@ -1423,9 +1571,8 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest {
 				DataTypeManager dtm = program.getDataTypeManager();
 				int transactionID = program.startTransaction("test");
 				Structure bar = (Structure) dtm.getDataType(new CategoryPath("/MISC"), "Bar");
-				FunctionDefinition fd =
+				FunctionDefinition fd = (FunctionDefinition) dtm
-					(FunctionDefinition) dtm.getDataType(new CategoryPath("/MISC"),
+						.getDataType(new CategoryPath("/MISC"), "MyFunctionDef");
-						"MyFunctionDef");
 
 				try {
 					fd.setReturnType(bar);
@@ -1446,9 +1593,8 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest {
 				boolean commit = false;
 				DataTypeManager dtm = program.getDataTypeManager();
 				Structure foo = (Structure) dtm.getDataType(new CategoryPath("/MISC"), "Foo");
-				FunctionDefinition fd =
+				FunctionDefinition fd = (FunctionDefinition) dtm
-					(FunctionDefinition) dtm.getDataType(new CategoryPath("/MISC"),
+						.getDataType(new CategoryPath("/MISC"), "MyFunctionDef");
-						"MyFunctionDef");
 				ParameterDefinition[] vars = fd.getArguments();
 
 				int transactionID = program.startTransaction("test");
@@ -1491,9 +1637,8 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest {
 				boolean commit = false;
 				DataTypeManager dtm = program.getDataTypeManager();
 				int transactionID = program.startTransaction("test");
-				FunctionDefinition fd =
+				FunctionDefinition fd = (FunctionDefinition) dtm
-					(FunctionDefinition) dtm.getDataType(new CategoryPath("/MISC"),
+						.getDataType(new CategoryPath("/MISC"), "MyFunctionDef");
-						"MyFunctionDef");
 
 				try {
 					fd.setVarArgs(true);
@@ -1514,9 +1659,8 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest {
 				boolean commit = false;
 				DataTypeManager dtm = program.getDataTypeManager();
 				Structure foo = (Structure) dtm.getDataType(new CategoryPath("/MISC"), "Foo");
-				FunctionDefinition fd =
+				FunctionDefinition fd = (FunctionDefinition) dtm
-					(FunctionDefinition) dtm.getDataType(new CategoryPath("/MISC"),
+						.getDataType(new CategoryPath("/MISC"), "MyFunctionDef");
-						"MyFunctionDef");
 				ParameterDefinition[] vars = fd.getArguments();
 
 				int transactionID = program.startTransaction("test");
@@ -1560,9 +1704,8 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest {
 				boolean commit = false;
 				DataTypeManager dtm = program.getDataTypeManager();
 				int transactionID = program.startTransaction("test");
-				FunctionDefinition fd =
+				FunctionDefinition fd = (FunctionDefinition) dtm
-					(FunctionDefinition) dtm.getDataType(new CategoryPath("/MISC"),
+						.getDataType(new CategoryPath("/MISC"), "MyFunctionDef");
-						"MyFunctionDef");
 
 				try {
 					fd.setVarArgs(true);
@@ -1583,9 +1726,8 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest {
 				boolean commit = false;
 				DataTypeManager dtm = program.getDataTypeManager();
 				Structure foo = (Structure) dtm.getDataType(new CategoryPath("/MISC"), "Foo");
-				FunctionDefinition fd =
+				FunctionDefinition fd = (FunctionDefinition) dtm
-					(FunctionDefinition) dtm.getDataType(new CategoryPath("/MISC"),
+						.getDataType(new CategoryPath("/MISC"), "MyFunctionDef");
-						"MyFunctionDef");
 				ParameterDefinition[] vars = fd.getArguments();
 
 				int transactionID = program.startTransaction("test");
@@ -1633,9 +1775,8 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest {
 				DataTypeManager dtm = program.getDataTypeManager();
 				int transactionID = program.startTransaction("test");
 
-				FunctionDefinition fd =
+				FunctionDefinition fd = (FunctionDefinition) dtm
-					(FunctionDefinition) dtm.getDataType(new CategoryPath("/MISC"),
+						.getDataType(new CategoryPath("/MISC"), "MyFunctionDef");
-						"MyFunctionDef");
 
 				try {
 					fd.setReturnType(VoidDataType.dataType);
@@ -1655,9 +1796,8 @@ public class DataTypeMerge2Test extends AbstractDataTypeMergeTest {
 				boolean commit = false;
 				DataTypeManager dtm = program.getDataTypeManager();
 
-				FunctionDefinition fd =
+				FunctionDefinition fd = (FunctionDefinition) dtm
-					(FunctionDefinition) dtm.getDataType(new CategoryPath("/MISC"),
+						.getDataType(new CategoryPath("/MISC"), "MyFunctionDef");
-						"MyFunctionDef");
 				ParameterDefinition[] vars = fd.getArguments();
 
 				int transactionID = program.startTransaction("test");

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/datamgr/editor/EnumEditor1Test.java

@@ -77,8 +77,7 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 	public void testEnumFields() throws Exception {
 		Category c = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		Enum enumm = createEnum(c, "TestEnum", 1);
 		edit(enumm);
 
@@ -132,8 +131,7 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 	public void testEnumSize1() throws Exception {
 		Category category = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		Enum enumm = createEnum(category, "TestEnum", 1);
 		edit(enumm);
 
@@ -184,8 +182,7 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 		// test entering too large a value
 		Category category = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		Enum enumm = createEnum(category, "TestEnum", 1);
 		edit(enumm);
 
@@ -228,8 +225,7 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 	public void testEnumSize4BadInput() throws Exception {
 		Category category = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		Enum enumm = createEnum(category, "MyTestEnum", 4);
 		edit(enumm);
 
@@ -280,8 +276,7 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 	public void testBadInputForValue() throws Exception {
 		Category cat = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		Enum enumm = createEnum(cat, "TestEnum", 1);
 		edit(enumm);
 
@@ -310,8 +305,7 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 
 		Category cat = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		final Enum enumm = new EnumDataType("Colors", 1);
 		enumm.add("Red", 0);
 		enumm.add("Green", 1);
@@ -375,8 +369,7 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 	public void testValueForNewEntry() throws Exception {
 		Category cat = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		final Enum enumm = new EnumDataType("Colors", 1);
 		enumm.add("Red", 0x10);
 		enumm.add("Green", 0x20);
@@ -656,7 +649,7 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 		EnumEditorPanel panel = findEditorPanel(tool.getToolFrame());
 		JTable table = panel.getTable();
 
-		// 
+		//
 		// First, let's try forward then backward
 		//
 		int startRow = 1;
@@ -665,12 +658,18 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 		Component c = getEditorComponent(editor);
 
 		triggerActionKey(c, 0, KeyEvent.VK_TAB);
+		editor = assertEditingCell(table, startRow, startCol + 1);
+		c = getEditorComponent(editor);
 
+		triggerActionKey(c, 0, KeyEvent.VK_TAB);
+		editor = assertEditingCell(table, startRow, startCol + 2);
+		c = getEditorComponent(editor);
+
+		triggerActionKey(c, InputEvent.SHIFT_DOWN_MASK, KeyEvent.VK_TAB);
 		editor = assertEditingCell(table, startRow, startCol + 1);
 		c = getEditorComponent(editor);
 
 		triggerActionKey(c, InputEvent.SHIFT_DOWN_MASK, KeyEvent.VK_TAB);
-
 		editor = assertEditingCell(table, startRow, startCol);
 		c = getEditorComponent(editor);
 
@@ -678,7 +677,7 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 		// Now, let's try going around the world and back
 		//
 		int lastRow = 2;
-		int lastCol = 1;
+		int lastCol = 2;
 		editor = startEditTableCell(table, lastRow, lastCol);
 		c = getEditorComponent(editor);
 
@@ -703,7 +702,7 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 
 		JTable table = getEditTable();
 
-		// 
+		//
 		// First, let's try up and down
 		//
 		int startRow = 0;
@@ -739,7 +738,7 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 	@Test
 	public void testNewEnumFromAction() throws Exception {
 		//
-		// This test works differently that the others in that it uses the same path as the 
+		// This test works differently that the others in that it uses the same path as the
 		// GUI action to start the editing process.
 		//
 		DataTypeManager dtm = program.getListing().getDataTypeManager();
@@ -764,8 +763,7 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 
 		Category category = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		Enum enumm = createEnum(category, "EnumX", 2);
 
 		int transactionID = program.startTransaction("Test");
@@ -835,8 +833,7 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 
 		Category category = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		Enum enumm = createEnum(category, "EnumX", 2);
 
 		int transactionID = program.startTransaction("Test");
@@ -1049,8 +1046,8 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 		try {
 			Category category = dtm.getCategory(enummDt.getCategoryPath());
 			Category parentCategory = category.getParent();
-			assertTrue("Did not remove category", parentCategory.removeCategory(category.getName(),
+			assertTrue("Did not remove category",
-				TaskMonitor.DUMMY));
+				parentCategory.removeCategory(category.getName(), TaskMonitor.DUMMY));
 		}
 		finally {
 			program.endTransaction(txID, true);
@@ -1126,8 +1123,8 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 
 	private TableCellEditor assertEditingCell(final JTable table, final int row, final int col) {
 		Pair<Integer, Integer> rowCol = getEditingCell(table);
-		assertEquals(row, (int) rowCol.first);
+		assertEquals("Not editing expected row", row, (int) rowCol.first);
-		assertEquals(col, (int) rowCol.second);
+		assertEquals("Not editing expected column", col, (int) rowCol.second);
 		return runSwing(() -> table.getCellEditor());
 	}
 
@@ -1144,8 +1141,7 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 	private Enum createRedGreenBlueEnum() {
 		Category cat = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		final Enum enumm = new EnumDataType("Colors", 1);
 		enumm.add("Red", 0);
 		enumm.add("Green", 1);
@@ -1235,7 +1231,7 @@ public class EnumEditor1Test extends AbstractGhidraHeadedIntegrationTest {
 		addEnumValue();
 		waitForSwing();
 		final int row = model.getRowCount() - 1;
-		// change entry 
+		// change entry
 		table.addRowSelectionInterval(row, row);
 		Rectangle rect = table.getCellRect(row, NAME_COL, true);
 		clickMouse(table, 1, rect.x, rect.y, 2, 0);

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/datamgr/editor/EnumEditor2Test.java

@@ -41,10 +41,6 @@ public class EnumEditor2Test extends AbstractGhidraHeadedIntegrationTest {
 	private TestEnv env;
 	private DataTypeManagerPlugin plugin;
 
-	public EnumEditor2Test() {
-		super();
-	}
-
 	@Before
 	public void setUp() throws Exception {
 
@@ -70,8 +66,7 @@ public class EnumEditor2Test extends AbstractGhidraHeadedIntegrationTest {
 
 		Category cat = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		final Enum enumm = new EnumDataType("Colors", 1);
 		enumm.add("Red", 0);
 		enumm.add("Green", 0x10);
@@ -107,8 +102,7 @@ public class EnumEditor2Test extends AbstractGhidraHeadedIntegrationTest {
 	public void testSortColumns() throws Exception {
 		Category cat = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		final Enum enumm = new EnumDataType("Colors", 1);
 		enumm.add("Red", 0);
 		enumm.add("Green", 0x10);
@@ -151,8 +145,7 @@ public class EnumEditor2Test extends AbstractGhidraHeadedIntegrationTest {
 	public void testSortOrder() throws Exception {
 		Category cat = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		final Enum enumm = new EnumDataType("Colors", 1);
 		enumm.add("Red", 0);
 		enumm.add("Green", 0x10);
@@ -197,8 +190,7 @@ public class EnumEditor2Test extends AbstractGhidraHeadedIntegrationTest {
 	public void testInsertRowByName() throws Exception {
 		Category cat = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		final Enum enumm = new EnumDataType("Colors", 1);
 		enumm.add("Red", 0);
 		enumm.add("Green", 0x10);
@@ -324,8 +316,7 @@ public class EnumEditor2Test extends AbstractGhidraHeadedIntegrationTest {
 			waitForSwing();
 		}
 
-		final ComponentProvider provider =
+		final ComponentProvider provider = waitForComponentProvider(EnumEditorProvider.class);
-			waitForComponentProvider(EnumEditorProvider.class);
 		assertNotNull(provider);
 		SwingUtilities.invokeLater(() -> provider.closeComponent());
 		waitForSwing();
@@ -360,8 +351,7 @@ public class EnumEditor2Test extends AbstractGhidraHeadedIntegrationTest {
 			waitForSwing();
 		}
 
-		final ComponentProvider provider =
+		final ComponentProvider provider = waitForComponentProvider(EnumEditorProvider.class);
-			waitForComponentProvider(EnumEditorProvider.class);
 		assertNotNull(provider);
 		SwingUtilities.invokeLater(() -> provider.closeComponent());
 		waitForSwing();
@@ -390,8 +380,7 @@ public class EnumEditor2Test extends AbstractGhidraHeadedIntegrationTest {
 			waitForSwing();
 		}
 
-		final ComponentProvider provider =
+		final ComponentProvider provider = waitForComponentProvider(EnumEditorProvider.class);
-			waitForComponentProvider(EnumEditorProvider.class);
 		assertNotNull(provider);
 		SwingUtilities.invokeLater(() -> provider.closeComponent());
 		waitForSwing();
@@ -455,8 +444,7 @@ public class EnumEditor2Test extends AbstractGhidraHeadedIntegrationTest {
 
 		Category cat = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		final Enum enumm = new EnumDataType("Colors", 1);
 		enumm.add("Red", 0);
 		enumm.add("Green", 0x10);
@@ -760,7 +748,7 @@ public class EnumEditor2Test extends AbstractGhidraHeadedIntegrationTest {
 		});
 		waitForSwing();
 		final int newRow = model.getRowCount() - 1;
-		// change entry 
+		// change entry
 		runSwing(() -> table.addRowSelectionInterval(newRow, newRow));
 		Rectangle rect = table.getCellRect(newRow, EnumTableModel.NAME_COL, true);
 		clickMouse(table, 1, rect.x, rect.y, 2, 0);
@@ -828,8 +816,7 @@ public class EnumEditor2Test extends AbstractGhidraHeadedIntegrationTest {
 	private Enum editSampleEnum() {
 		Category cat = program.getListing()
 				.getDataTypeManager()
-				.getCategory(
+				.getCategory(new CategoryPath(CategoryPath.ROOT, "Category1"));
-					new CategoryPath(CategoryPath.ROOT, "Category1"));
 		final Enum enumm = new EnumDataType("Colors", 1);
 		enumm.add("Red", 0);
 		enumm.add("Green", 0x10);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/EnumDB.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,8 @@ import java.io.IOException;
 import java.math.BigInteger;
 import java.util.*;
 
+import org.apache.commons.lang3.StringUtils;
+
 import db.DBRecord;
 import db.Field;
 import ghidra.docking.settings.Settings;
@@ -33,7 +35,6 @@ import ghidra.util.UniversalID;
 
 /**
  * Database implementation for the enumerated data type.
- *
  */
 class EnumDB extends DataTypeDB implements Enum {
 
@@ -42,6 +43,7 @@ class EnumDB extends DataTypeDB implements Enum {
 
 	private EnumDBAdapter adapter;
 	private EnumValueDBAdapter valueAdapter;
+
 	private Map<String, Long> nameMap; // name to value
 	private Map<Long, List<String>> valueMap; // value to names
 	private Map<String, String> commentMap; // name to comment
@@ -88,21 +90,22 @@ class EnumDB extends DataTypeDB implements Enum {
 		commentMap = new HashMap<>();
 
 		Field[] ids = valueAdapter.getValueIdsInEnum(key);
-
 		for (Field id : ids) {
 			DBRecord rec = valueAdapter.getRecord(id.getLongValue());
 			String valueName = rec.getString(EnumValueDBAdapter.ENUMVAL_NAME_COL);
 			long value = rec.getLongValue(EnumValueDBAdapter.ENUMVAL_VALUE_COL);
-			String valueNameComment = rec.getString(EnumValueDBAdapter.ENUMVAL_COMMENT_COL);
+			String comment = rec.getString(EnumValueDBAdapter.ENUMVAL_COMMENT_COL);
-			addToCache(valueName, value, valueNameComment);
+			addToCache(valueName, value, comment);
 		}
 	}
 
-	private void addToCache(String valueName, long value, String valueNameComment) {
+	private void addToCache(String valueName, long value, String comment) {
 		nameMap.put(valueName, value);
 		List<String> list = valueMap.computeIfAbsent(value, v -> new ArrayList<>());
 		list.add(valueName);
-		commentMap.put(valueName, valueNameComment);
+		if (!StringUtils.isBlank(comment)) {
+			commentMap.put(valueName, comment);
+		}
 	}
 
 	private boolean removeFromCache(String valueName) {
@@ -121,10 +124,7 @@ class EnumDB extends DataTypeDB implements Enum {
 		if (list.isEmpty()) {
 			valueMap.remove(value);
 		}
-		String comment = commentMap.remove(valueName);
+		commentMap.remove(valueName);
-		if (comment == null) {
-			return false;
-		}
 		return true;
 	}
 
@@ -168,7 +168,7 @@ class EnumDB extends DataTypeDB implements Enum {
 	}
 
 	@Override
-	public String getComment(String valueName) throws NoSuchElementException {
+	public String getComment(String valueName) {
 		lock.acquire();
 		try {
 			checkIsValid();
@@ -214,19 +214,6 @@ class EnumDB extends DataTypeDB implements Enum {
 		}
 	}
 
-	@Override
-	public String[] getComments() {
-		lock.acquire();
-		try {
-			checkIsValid();
-			initializeIfNeeded();
-			return commentMap.keySet().toArray(new String[commentMap.size()]);
-		}
-		finally {
-			lock.release();
-		}
-	}
-
 	@Override
 	public int getCount() {
 		lock.acquire();
@@ -242,12 +229,11 @@ class EnumDB extends DataTypeDB implements Enum {
 
 	@Override
 	public void add(String valueName, long value) {
-		String valueNameComment = "";
+		add(valueName, value, null);
-		add(valueName, value, valueNameComment);
 	}
 
 	@Override
-	public void add(String valueName, long value, String valueNameComment) {
+	public void add(String valueName, long value, String comment) {
 		lock.acquire();
 		try {
 			checkDeleted();
@@ -256,10 +242,15 @@ class EnumDB extends DataTypeDB implements Enum {
 			if (nameMap.containsKey(valueName)) {
 				throw new IllegalArgumentException(valueName + " already exists in this enum");
 			}
+
+			if (StringUtils.isBlank(comment)) {
+				comment = null; // use null values in the db to save space
+			}
+
 			bitGroups = null;
-			valueAdapter.createRecord(key, valueName, value, valueNameComment);
+			valueAdapter.createRecord(key, valueName, value, comment);
 			adapter.updateRecord(record, true);
-			addToCache(valueName, value, valueNameComment);
+			addToCache(valueName, value, comment);
 			dataMgr.dataTypeChanged(this, false);
 		}
 		catch (IOException e) {
@@ -294,10 +285,9 @@ class EnumDB extends DataTypeDB implements Enum {
 			if (!removeFromCache(valueName)) {
 				return;
 			}
+
 			bitGroups = null;
-
 			Field[] ids = valueAdapter.getValueIdsInEnum(key);
-
 			for (Field id : ids) {
 				DBRecord rec = valueAdapter.getRecord(id.getLongValue());
 				if (valueName.equals(rec.getString(EnumValueDBAdapter.ENUMVAL_NAME_COL))) {
@@ -321,6 +311,7 @@ class EnumDB extends DataTypeDB implements Enum {
 		if (!(dataType instanceof Enum)) {
 			throw new IllegalArgumentException();
 		}
+
 		Enum enumm = (Enum) dataType;
 		lock.acquire();
 		try {
@@ -338,19 +329,21 @@ class EnumDB extends DataTypeDB implements Enum {
 
 			int oldLength = getLength();
 			int newLength = enumm.getLength();
-
 			if (oldLength != newLength) {
 				record.setByteValue(EnumDBAdapter.ENUM_SIZE_COL, (byte) newLength);
 				adapter.updateRecord(record, true);
 			}
 
 			String[] names = enumm.getNames();
-			for (String name2 : names) {
+			for (String valueName : names) {
-				long value = enumm.getValue(name2);
+				long value = enumm.getValue(valueName);
-				String comment = enumm.getComment(name2);
+				String comment = enumm.getComment(valueName);
-				valueAdapter.createRecord(key, name2, value, comment);
+				if (StringUtils.isBlank(comment)) {
+					comment = null; // use null values in the db to save space
+				}
+				valueAdapter.createRecord(key, valueName, value, comment);
 				adapter.updateRecord(record, true);
-				addToCache(name2, value, comment);
+				addToCache(valueName, value, comment);
 			}
 
 			if (oldLength != newLength) {
@@ -525,7 +518,7 @@ class EnumDB extends DataTypeDB implements Enum {
 			return "0";
 		}
 		List<BitGroup> list = getBitGroups();
-		StringBuffer buf = new StringBuffer();
+		StringBuilder buf = new StringBuilder();
 		for (BitGroup bitGroup : list) {
 			long subValue = bitGroup.getMask() & value;
 			if (subValue != 0) {
@@ -576,21 +569,39 @@ class EnumDB extends DataTypeDB implements Enum {
 			getLength() != enumm.getLength() || getCount() != enumm.getCount()) {
 			return false;
 		}
+
+		if (!isEachValueEquivalent(enumm)) {
+			return false;
+		}
+		return true;
+	}
+
+	private boolean isEachValueEquivalent(Enum enumm) {
 		String[] names = getNames();
 		String[] otherNames = enumm.getNames();
 		try {
 			for (int i = 0; i < names.length; i++) {
+				if (!names[i].equals(otherNames[i])) {
+					return false;
+				}
+
 				long value = getValue(names[i]);
 				long otherValue = enumm.getValue(names[i]);
-				if (!names[i].equals(otherNames[i]) || value != otherValue) {
+				if (value != otherValue) {
+					return false;
+				}
+
+				String comment = getComment(names[i]);
+				String otherComment = enumm.getComment(names[i]);
+				if (!comment.equals(otherComment)) {
 					return false;
 				}
 			}
+			return true;
 		}
 		catch (NoSuchElementException e) {
 			return false; // named element not found
 		}
-		return true;
 	}
 
 	@Override
@@ -598,6 +609,7 @@ class EnumDB extends DataTypeDB implements Enum {
 		try {
 			nameMap = null;
 			valueMap = null;
+			commentMap = null;
 			bitGroups = null;
 			DBRecord rec = adapter.getRecord(key);
 			if (rec != null) {
@@ -731,5 +743,4 @@ class EnumDB extends DataTypeDB implements Enum {
 			lock.release();
 		}
 	}
-
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/EnumValueDBAdapter.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.
@@ -30,15 +30,16 @@ import ghidra.util.task.TaskMonitor;
 /**
  * Adapter to access the Enumeration data type values tables.
  */
-abstract class EnumValueDBAdapter {
+abstract class EnumValueDBAdapter implements RecordTranslator {
 
 	static final String ENUM_VALUE_TABLE_NAME = "Enumeration Values";
-	static final Schema ENUM_VALUE_SCHEMA = EnumValueDBAdapterV0.V0_ENUM_VALUE_SCHEMA;
+	static final Schema ENUM_VALUE_SCHEMA = EnumValueDBAdapterV1.SCHEMA;
+
 	// Enum Value Columns
-	static final int ENUMVAL_NAME_COL = EnumValueDBAdapterV0.V0_ENUMVAL_NAME_COL;
+	static final int ENUMVAL_NAME_COL = 0;
-	static final int ENUMVAL_VALUE_COL = EnumValueDBAdapterV0.V0_ENUMVAL_VALUE_COL;
+	static final int ENUMVAL_VALUE_COL = 1;
-	static final int ENUMVAL_ID_COL = EnumValueDBAdapterV0.V0_ENUMVAL_ID_COL;
+	static final int ENUMVAL_ID_COL = 2;
-	static final int ENUMVAL_COMMENT_COL = EnumValueDBAdapterV0.V0_ENUMVAL_COMMENT_COL;
+	static final int ENUMVAL_COMMENT_COL = 3;
 
 	/**
 	 * Gets an adapter for working with the enumeration data type values database table. The adapter is based
@@ -53,16 +54,16 @@ abstract class EnumValueDBAdapter {
 	static EnumValueDBAdapter getAdapter(DBHandle handle, int openMode, TaskMonitor monitor)
 			throws VersionException, IOException {
 		if (openMode == DBConstants.CREATE) {
-			return new EnumValueDBAdapterV0(handle, true);
+			return new EnumValueDBAdapterV1(handle, true);
 		}
 		try {
-			return new EnumValueDBAdapterV0(handle, false);
+			return new EnumValueDBAdapterV1(handle, false);
 		}
 		catch (VersionException e) {
 			if (!e.isUpgradable() || openMode == DBConstants.UPDATE) {
 				throw e;
 			}
-			EnumValueDBAdapter adapter = new EnumValueDBAdapterNoTable(handle);
+			EnumValueDBAdapter adapter = findReadOnlyAdapter(handle);
 			if (openMode == DBConstants.UPGRADE) {
 				adapter = upgrade(handle, adapter);
 			}
@@ -70,8 +71,18 @@ abstract class EnumValueDBAdapter {
 		}
 	}
 
+	static EnumValueDBAdapter findReadOnlyAdapter(DBHandle handle) {
+		try {
+			return new EnumValueDBAdapterV0(handle);
+		}
+		catch (VersionException e) {
+			return new EnumValueDBAdapterNoTable(handle);
+		}
+	}
+
 	/**
-	 * Upgrades the Enumeration Data Type Values table from the oldAdapter's version to the current version.
+	 * Upgrades the Enumeration Data Type Values table from the oldAdapter's version to the current
+	 * version.
 	 * @param handle handle to the database whose table is to be upgraded to a newer version.
 	 * @param oldAdapter the adapter for the existing table to be upgraded.
 	 * @return the adapter for the new upgraded version of the table.
@@ -81,7 +92,30 @@ abstract class EnumValueDBAdapter {
 	 */
 	static EnumValueDBAdapter upgrade(DBHandle handle, EnumValueDBAdapter oldAdapter)
 			throws VersionException, IOException {
-		return new EnumValueDBAdapterV0(handle, true);
+
+		DBHandle tmpHandle = new DBHandle();
+		long id = tmpHandle.startTransaction();
+		EnumValueDBAdapter tmpAdapter = null;
+		try {
+			tmpAdapter = new EnumValueDBAdapterV1(tmpHandle, true);
+			RecordIterator it = oldAdapter.getRecords();
+			while (it.hasNext()) {
+				DBRecord rec = it.next();
+				tmpAdapter.updateRecord(rec);
+			}
+			oldAdapter.deleteTable(handle);
+			EnumValueDBAdapter newAdapter = new EnumValueDBAdapterV1(handle, true);
+			it = tmpAdapter.getRecords();
+			while (it.hasNext()) {
+				DBRecord rec = it.next();
+				newAdapter.updateRecord(rec);
+			}
+			return newAdapter;
+		}
+		finally {
+			tmpHandle.endTransaction(id, true);
+			tmpHandle.close();
+		}
 	}
 
 	/**
@@ -90,7 +124,7 @@ abstract class EnumValueDBAdapter {
 	 * @param name value name
 	 * @param value numeric value
 	 * @param comment the field comment
-	 * @throws IOException if IO error occurs
+	 * @throws IOException if there was a problem accessing the database
 	 */
 	abstract void createRecord(long enumID, String name, long value, String comment)
 			throws IOException;
@@ -99,10 +133,26 @@ abstract class EnumValueDBAdapter {
 	 * Get enum value record which corresponds to specified value record ID
 	 * @param valueID value record ID
 	 * @return value record or null
-	 * @throws IOException if IO error occurs
+	 * @throws IOException if there was a problem accessing the database
 	 */
 	abstract DBRecord getRecord(long valueID) throws IOException;
 
+	/**
+	 * Returns an iterator over the value records inside of this Enum
+	 * @return the iterator
+	 * @throws IOException if there was a problem accessing the database
+	 */
+	abstract RecordIterator getRecords() throws IOException;
+
+	/**
+	 * Deletes the table; used when upgrading
+	 * @param handle the handle used to delete the table
+	 * @throws IOException if there was a problem accessing the database
+	 */
+	void deleteTable(DBHandle handle) throws IOException {
+		handle.deleteTable(ENUM_VALUE_TABLE_NAME);
+	}
+
 	/**
 	 * Remove the record for the given enum Value ID.
 	 * @param valueID ID of the value record to delete
@@ -113,7 +163,7 @@ abstract class EnumValueDBAdapter {
 	/**
 	 * Updates the enum data type values table with the provided record.
 	 * @param record the new record
-	 * @throws IOException if the database can't be accessed.
+	 * @throws IOException if there was a problem accessing the database
 	 */
 	abstract void updateRecord(DBRecord record) throws IOException;
 
@@ -121,8 +171,7 @@ abstract class EnumValueDBAdapter {
 	 * Get enum value record IDs which correspond to specified enum datatype ID
 	 * @param enumID enum datatype ID
 	 * @return enum value record IDs as LongField values within Field array
-	 * @throws IOException if IO error occurs
+	 * @throws IOException if there was a problem accessing the database
 	 */
 	abstract Field[] getValueIdsInEnum(long enumID) throws IOException;
-
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/EnumValueDBAdapterNoTable.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.
@@ -17,7 +17,10 @@ package ghidra.program.database.data;
 
 import java.io.IOException;
 
+import javax.help.UnsupportedOperationException;
+
 import db.*;
+import ghidra.program.database.util.EmptyRecordIterator;
 
 /**
  * Adapter needed for a read-only version of data type manager that is not going
@@ -59,4 +62,18 @@ class EnumValueDBAdapterNoTable extends EnumValueDBAdapter {
 		return Field.EMPTY_ARRAY;
 	}
 
+	@Override
+	RecordIterator getRecords() throws IOException {
+		return new EmptyRecordIterator();
+	}
+
+	@Override
+	protected void deleteTable(DBHandle handle) throws IOException {
+		// do nothing
+	}
+
+	@Override
+	public DBRecord translateRecord(DBRecord rec) {
+		throw new UnsupportedOperationException();
+	}
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/EnumValueDBAdapterV0.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.
@@ -18,92 +18,84 @@ package ghidra.program.database.data;
 import java.io.IOException;
 
 import db.*;
-import ghidra.program.model.lang.ConstantPool.Record;
 import ghidra.util.exception.VersionException;
 
 /**
  * Version 0 implementation for the enumeration tables adapter.
- *
  */
 class EnumValueDBAdapterV0 extends EnumValueDBAdapter {
 	static final int VERSION = 0;
 
-	// Enum Value Columns
+//  Keep for reference
-	static final int V0_ENUMVAL_NAME_COL = 0;
+//	static final Schema SCHEMA = new Schema(0, "Enum Value ID",
-	static final int V0_ENUMVAL_VALUE_COL = 1;
+//		new Field[] { StringField.INSTANCE, LongField.INSTANCE, LongField.INSTANCE },
-	static final int V0_ENUMVAL_ID_COL = 2;
+//		new String[] { "Name", "Value", "Enum ID" });
-	static final int V0_ENUMVAL_COMMENT_COL = 3;
 
-	static final Schema V0_ENUM_VALUE_SCHEMA = new Schema(0, "Enum Value ID",
+	private Table table;
-		new Field[] { StringField.INSTANCE, LongField.INSTANCE, LongField.INSTANCE },
-		new String[] { "Name", "Value", "Enum ID" });
-
-//	static final Schema V1_ENUM_VALUE_SCHEMA = new Schema(0, "Enum Value ID",
-//		new Class[] { StringField.class, LongField.class, LongField.class, StringField.class },
-//		new String[] { "Name", "Value", "Enum ID", "Comment" });
-
-	private Table valueTable;
 
 	/**
 	 * Gets a version 0 adapter for the Enumeration Data Type Values database table.
 	 * @param handle handle to the database containing the table.
-	 * @param create true if this constructor should create the table.
 	 * @throws VersionException if the the table's version does not match the expected version
 	 * for this adapter.
-	 * @throws IOException if IO error occurs
 	 */
-	public EnumValueDBAdapterV0(DBHandle handle, boolean create)
+	public EnumValueDBAdapterV0(DBHandle handle) throws VersionException {
-			throws VersionException, IOException {
 
-		if (create) {
+		table = handle.getTable(ENUM_VALUE_TABLE_NAME);
-			valueTable = handle.createTable(ENUM_VALUE_TABLE_NAME, V0_ENUM_VALUE_SCHEMA,
+		if (table == null) {
-				new int[] { V0_ENUMVAL_ID_COL });
+			throw new VersionException("Missing Table: " + ENUM_VALUE_TABLE_NAME);
 		}
-		else {
+
-			valueTable = handle.getTable(ENUM_VALUE_TABLE_NAME);
+		int version = table.getSchema().getVersion();
-			if (valueTable == null) {
+		if (version != VERSION) {
-				throw new VersionException("Missing Table: " + ENUM_VALUE_TABLE_NAME);
+			String msg = "Expected version " + VERSION + " for table " + ENUM_VALUE_TABLE_NAME +
-			}
+				" but got " + table.getSchema().getVersion();
-			int version = valueTable.getSchema().getVersion();
+			throw new VersionException(msg, VersionException.NEWER_VERSION, false);
-			if (version != VERSION) {
-				String msg = "Expected version " + VERSION + " for table " + ENUM_VALUE_TABLE_NAME +
-					" but got " + valueTable.getSchema().getVersion();
-				if (version < VERSION) {
-					throw new VersionException(msg, VersionException.OLDER_VERSION, true);
-				}
-				throw new VersionException(msg, VersionException.NEWER_VERSION, false);
-			}
 		}
 	}
 
 	@Override
 	public void createRecord(long enumID, String name, long value, String comment)
 			throws IOException {
-		Record record = V0_ENUM_VALUE_SCHEMA.createRecord(valueTable.getKey());
+		throw new UnsupportedOperationException("Cannot update Version 0");
-		record.setLongValue(V0_ENUMVAL_ID_COL, enumID);
-		record.setString(V0_ENUMVAL_NAME_COL, name);
-		record.setLongValue(V0_ENUMVAL_VALUE_COL, value);
-		record.setString(V0_ENUMVAL_COMMENT_COL, comment);
-		valueTable.putRecord(record);
 	}
 
 	@Override
 	public DBRecord getRecord(long valueID) throws IOException {
-		return valueTable.getRecord(valueID);
+		return translateRecord(table.getRecord(valueID));
 	}
 
 	@Override
 	public void removeRecord(long valueID) throws IOException {
-		valueTable.deleteRecord(valueID);
+		throw new UnsupportedOperationException("Cannot remove Version 0");
 	}
 
 	@Override
 	public void updateRecord(DBRecord record) throws IOException {
-		valueTable.putRecord(record);
+		throw new UnsupportedOperationException("Cannot update Version 0");
 	}
 
 	@Override
 	public Field[] getValueIdsInEnum(long enumID) throws IOException {
-		return valueTable.findRecords(new LongField(enumID), V0_ENUMVAL_ID_COL);
+		return table.findRecords(new LongField(enumID), ENUMVAL_ID_COL);
+	}
+
+	@Override
+	public DBRecord translateRecord(DBRecord oldRec) {
+		if (oldRec == null) {
+			return null;
+		}
+
+		DBRecord record = EnumValueDBAdapter.ENUM_VALUE_SCHEMA.createRecord(oldRec.getKey());
+		record.setLongValue(ENUMVAL_ID_COL, oldRec.getLongValue(ENUMVAL_ID_COL));
+		record.setString(ENUMVAL_NAME_COL, oldRec.getString(ENUMVAL_NAME_COL));
+		record.setLongValue(ENUMVAL_VALUE_COL, oldRec.getLongValue(ENUMVAL_VALUE_COL));
+		record.setString(ENUMVAL_COMMENT_COL, null);
+		return record;
+	}
+
+	@Override
+	RecordIterator getRecords() throws IOException {
+		return new TranslatedRecordIterator(table.iterator(), this);
 	}
 }

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

@@ -0,0 +1,107 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * 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.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ghidra.program.database.data;
+
+import java.io.IOException;
+
+import db.*;
+import ghidra.util.exception.VersionException;
+
+/**
+ * Version 1 implementation for the enumeration tables adapter.
+ */
+class EnumValueDBAdapterV1 extends EnumValueDBAdapter {
+
+	static final int VERSION = 1;
+
+	static final Schema SCHEMA = new Schema(VERSION, "Enum Value ID",
+		new Class[] { StringField.class, LongField.class, LongField.class, StringField.class },
+		new String[] { "Name", "Value", "Enum ID", "Comment" });
+
+	private Table table;
+
+	/**
+	 * Gets a version 1 adapter for the Enumeration Data Type Values database table.
+	 * @param handle handle to the database containing the table.
+	 * @param create true if this constructor should create the table.
+	 * @throws VersionException if the the table's version does not match the expected version
+	 * for this adapter.
+	 * @throws IOException if IO error occurs
+	 */
+	public EnumValueDBAdapterV1(DBHandle handle, boolean create)
+			throws VersionException, IOException {
+
+		if (create) {
+			table = handle.createTable(ENUM_VALUE_TABLE_NAME, SCHEMA, new int[] { ENUMVAL_ID_COL });
+		}
+		else {
+			table = handle.getTable(ENUM_VALUE_TABLE_NAME);
+			if (table == null) {
+				throw new VersionException(true);
+			}
+			int version = table.getSchema().getVersion();
+			if (version != VERSION) {
+				String msg = "Expected version " + VERSION + " for table " + ENUM_VALUE_TABLE_NAME +
+					" but got " + table.getSchema().getVersion();
+				if (version < VERSION) {
+					throw new VersionException(msg, VersionException.OLDER_VERSION, true);
+				}
+				throw new VersionException(msg, VersionException.NEWER_VERSION, false);
+			}
+		}
+	}
+
+	@Override
+	public void createRecord(long enumID, String name, long value, String comment)
+			throws IOException {
+		DBRecord record = SCHEMA.createRecord(table.getKey());
+		record.setLongValue(ENUMVAL_ID_COL, enumID);
+		record.setString(ENUMVAL_NAME_COL, name);
+		record.setLongValue(ENUMVAL_VALUE_COL, value);
+		record.setString(ENUMVAL_COMMENT_COL, comment);
+		table.putRecord(record);
+	}
+
+	@Override
+	public DBRecord getRecord(long valueID) throws IOException {
+		return table.getRecord(valueID);
+	}
+
+	@Override
+	public void removeRecord(long valueID) throws IOException {
+		table.deleteRecord(valueID);
+	}
+
+	@Override
+	public void updateRecord(DBRecord record) throws IOException {
+		table.putRecord(record);
+	}
+
+	@Override
+	public Field[] getValueIdsInEnum(long enumID) throws IOException {
+		return table.findRecords(new LongField(enumID), ENUMVAL_ID_COL);
+	}
+
+	@Override
+	RecordIterator getRecords() throws IOException {
+		return table.iterator();
+	}
+
+	@Override
+	public DBRecord translateRecord(DBRecord r) {
+		return r;
+	}
+}

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

@@ -24,9 +24,9 @@ import ghidra.util.task.TaskMonitor;
 
 /**
  * Adapter to access the Pointer database table for Pointer data types.
- * 
+ *
  */
-abstract class PointerDBAdapter {
+abstract class PointerDBAdapter implements RecordTranslator {
 	static final String POINTER_TABLE_NAME = "Pointers";
 
 	static final Schema SCHEMA = new Schema(PointerDBAdapterV2.VERSION, "Pointer ID",
@@ -96,7 +96,7 @@ abstract class PointerDBAdapter {
 	}
 
 	/**
-	 * 
+	 *
 	 */
 	abstract void deleteTable(DBHandle handle) throws IOException;
 
@@ -105,6 +105,7 @@ abstract class PointerDBAdapter {
 	 * @param dataTypeID data type ID of the date type being pointed to
 	 * @param categoryID the category ID of the datatype
 	 * @param length pointer size in bytes
+	 * @return the record
 	 * @throws IOException if there was a problem accessing the database
 	 */
 	abstract DBRecord createRecord(long dataTypeID, long categoryID, int length) throws IOException;
@@ -117,6 +118,11 @@ abstract class PointerDBAdapter {
 	 */
 	abstract DBRecord getRecord(long pointerID) throws IOException;
 
+	/**
+	 * An iterator over the records of this adapter
+	 * @return the iterator
+	 * @throws IOException if there was a problem accessing the database
+	 */
 	abstract RecordIterator getRecords() throws IOException;
 
 	/**
@@ -135,51 +141,11 @@ abstract class PointerDBAdapter {
 	abstract void updateRecord(DBRecord record) throws IOException;
 
 	/**
-	 * Gets all the pointer data types that are contained in the category that 
+	 * Gets all the pointer data types that are contained in the category that
 	 * have the indicated ID.
 	 * @param categoryID the category whose pointer data types are wanted.
 	 * @return an array of IDs for the pointer data types in the category.
 	 * @throws IOException if the database can't be accessed.
 	 */
 	abstract Field[] getRecordIdsInCategory(long categoryID) throws IOException;
-
-	DBRecord translateRecord(DBRecord rec) {
-		return rec;
-	}
-
-	class TranslatedRecordIterator implements RecordIterator {
-		private RecordIterator it;
-
-		TranslatedRecordIterator(RecordIterator it) {
-			this.it = it;
-		}
-
-		@Override
-		public boolean delete() throws IOException {
-			throw new UnsupportedOperationException();
-		}
-
-		@Override
-		public boolean hasNext() throws IOException {
-			return it.hasNext();
-		}
-
-		@Override
-		public boolean hasPrevious() throws IOException {
-			return it.hasPrevious();
-		}
-
-		@Override
-		public DBRecord next() throws IOException {
-			DBRecord rec = it.next();
-			return translateRecord(rec);
-		}
-
-		@Override
-		public DBRecord previous() throws IOException {
-			DBRecord rec = it.previous();
-			return translateRecord(rec);
-		}
-	}
-
 }

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

@@ -22,14 +22,15 @@ import ghidra.util.exception.VersionException;
 
 /**
  * Version 0 implementation for the accessing the pointer database table.
- * 
+ *
  */
 class PointerDBAdapterV0 extends PointerDBAdapter {
 	final static int VERSION = 0;
 
-	static final int OLD_PTR_DTD_COL = 0;
+	static final int V0_PTR_DTD_COL_ = 0;
-	static final int OLD_PTR_SIZE_COL = 1;
+	static final int V0_PTR_SIZE_COL = 1;
 
+//  Keep for reference
 //	static final Schema SCHEMA = new Schema(VERSION, "Pointer ID",
 //								new Class[] {LongField.class, IntField.class},
 //								new String[] {"Data Type ID", "Size"});
@@ -60,7 +61,7 @@ class PointerDBAdapterV0 extends PointerDBAdapter {
 
 	@Override
 	RecordIterator getRecords() throws IOException {
-		return new TranslatedRecordIterator(table.iterator());
+		return new TranslatedRecordIterator(table.iterator(), this);
 	}
 
 	@Override
@@ -79,13 +80,14 @@ class PointerDBAdapterV0 extends PointerDBAdapter {
 	}
 
 	@Override
-	DBRecord translateRecord(DBRecord oldRec) {
+	public DBRecord translateRecord(DBRecord oldRec) {
 		if (oldRec == null) {
 			return null;
 		}
 		DBRecord rec = PointerDBAdapter.SCHEMA.createRecord(oldRec.getKey());
-		rec.setLongValue(PTR_DT_ID_COL, oldRec.getLongValue(OLD_PTR_DTD_COL));
+		rec.setLongValue(PTR_DT_ID_COL, oldRec.getLongValue(V0_PTR_DTD_COL_));
 		rec.setLongValue(PTR_CATEGORY_COL, 0);
+		rec.setByteValue(PTR_LENGTH_COL, (byte) -1);
 		return rec;
 	}
 

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

@@ -26,11 +26,13 @@ import ghidra.util.exception.VersionException;
 class PointerDBAdapterV1 extends PointerDBAdapter {
 	final static int VERSION = 1;
 
-	static final int OLD_PTR_DT_ID_COL = 0;
+	static final int V1_PTR_DT_ID_COL = 0;
-	static final int OLD_PTR_CATEGORY_COL = 1;
+	static final int V1_PTR_CATEGORY_COL = 1;
-	static final Schema V1_SCHEMA =
+
-		new Schema(VERSION, "Pointer ID", new Field[] { LongField.INSTANCE, LongField.INSTANCE },
+//  Keep for reference
-			new String[] { "Data Type ID", "Category ID" });
+//	static final Schema V1_SCHEMA =
+//		new Schema(VERSION, "Pointer ID", new Field[] { LongField.INSTANCE, LongField.INSTANCE },
+//			new String[] { "Data Type ID", "Category ID" });
 
 	private Table table;
 
@@ -49,13 +51,13 @@ class PointerDBAdapterV1 extends PointerDBAdapter {
 	}
 
 	@Override
-	DBRecord translateRecord(DBRecord oldRec) {
+	public DBRecord translateRecord(DBRecord oldRec) {
 		if (oldRec == null) {
 			return null;
 		}
 		DBRecord rec = PointerDBAdapter.SCHEMA.createRecord(oldRec.getKey());
-		rec.setLongValue(PTR_DT_ID_COL, oldRec.getLongValue(OLD_PTR_DT_ID_COL));
+		rec.setLongValue(PTR_DT_ID_COL, oldRec.getLongValue(V1_PTR_DT_ID_COL));
-		rec.setLongValue(PTR_CATEGORY_COL, oldRec.getLongValue(OLD_PTR_CATEGORY_COL));
+		rec.setLongValue(PTR_CATEGORY_COL, oldRec.getLongValue(V1_PTR_CATEGORY_COL));
 		rec.setByteValue(PTR_LENGTH_COL, (byte) -1);
 		return rec;
 	}
@@ -72,7 +74,7 @@ class PointerDBAdapterV1 extends PointerDBAdapter {
 
 	@Override
 	RecordIterator getRecords() throws IOException {
-		return new TranslatedRecordIterator(table.iterator());
+		return new TranslatedRecordIterator(table.iterator(), this);
 	}
 
 	@Override
@@ -87,7 +89,7 @@ class PointerDBAdapterV1 extends PointerDBAdapter {
 
 	@Override
 	Field[] getRecordIdsInCategory(long categoryID) throws IOException {
-		return table.findRecords(new LongField(categoryID), OLD_PTR_CATEGORY_COL);
+		return table.findRecords(new LongField(categoryID), V1_PTR_CATEGORY_COL);
 	}
 
 	@Override

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

@@ -86,7 +86,10 @@ class PointerDBAdapterV2 extends PointerDBAdapter {
 	@Override
 	void deleteTable(DBHandle handle) throws IOException {
 		handle.deleteTable(POINTER_TABLE_NAME);
-
 	}
 
+	@Override
+	public DBRecord translateRecord(DBRecord rec) {
+		return rec;
+	}
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/map/AddressMapDBAdapter.java

@@ -1,6 +1,5 @@
 /* ###
  * IP: GHIDRA
- * REVIEWED: YES
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,16 +15,15 @@
  */
 package ghidra.program.database.map;
 
-import ghidra.program.model.address.Address;
-import ghidra.program.model.address.AddressFactory;
-import ghidra.util.exception.VersionException;
-import ghidra.util.task.TaskMonitor;
-
 import java.io.IOException;
 import java.util.List;
 
 import db.DBConstants;
 import db.DBHandle;
+import ghidra.program.model.address.Address;
+import ghidra.program.model.address.AddressFactory;
+import ghidra.util.exception.VersionException;
+import ghidra.util.task.TaskMonitor;
 
 /**
  * Database adapter for address map

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

@@ -24,9 +24,9 @@ public interface Enum extends DataType {
 
 	/**
 	 * Get the value for the given name.
-	 * @param name name of the entry
+	 * @param name name of the entry.
-	 * @return the value
+	 * @return the value.
-	 * @throws NoSuchElementException if the name does not exist in this Enum
+	 * @throws NoSuchElementException if the name does not exist in this Enum.
 	 */
 	public long getValue(String name) throws NoSuchElementException;
 
@@ -39,11 +39,11 @@ public interface Enum extends DataType {
 
 	/**
 	 * Get the comment for the given name.
-	 * @param name name of the entry
+	 * @param name name of the entry.
-	 * @return the comment
+	 * @return the comment or the empty string if the name does not exist in this enum or if no
-	 * @throws NoSuchElementException if the name does not exist in this Enum
+	 * comment is set.
 	 */
-	public String getComment(String name) throws NoSuchElementException;
+	public String getComment(String name);
 
 	/**
 	 * Get the values of the enum entries.
@@ -52,17 +52,15 @@ public interface Enum extends DataType {
 	public long[] getValues();
 
 	/**
-	 * Get the names of the enum entries.
+	 * Get the names of the enum entries.  The returned names are sorted using String's natural
+	 * sort order.
+	 * @return the names of the enum entries.
 	 */
 	public String[] getNames();
 
 	/**
-	 * Get the comments of the enum entries.
+	 * Get the number of entries in this Enum.
-	 */
+	 * @return the number of entries in this Enum.
-	public String[] getComments();
-
-	/**
-	 * Get the number of entries in this Enum. 
 	 */
 	public int getCount();
 
@@ -82,14 +80,14 @@ public interface Enum extends DataType {
 	public void add(String name, long value, String comment);
 
 	/**
-	 * Remove the enum entry with the given name. 
+	 * Remove the enum entry with the given name.
 	 * @param name name of entry to remove.
 	 */
 	public void remove(String name);
 
 	/**
 	 * Set the description for this Enum.
-	 * @param description
+	 * @param description the description
 	 */
 	@Override
 	public void setDescription(String description);
@@ -98,6 +96,7 @@ public interface Enum extends DataType {
 	 * Get enum representation of the big-endian value.
 	 * @param bigInt BigInteger value with the appropriate sign
 	 * @param settings integer format settings (PADDING, FORMAT, etc.)
+	 * @param bitLength the bit length
 	 * @return formatted integer string
 	 */
 	public String getRepresentation(BigInteger bigInt, Settings settings, int bitLength);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/EnumDataType.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.
@@ -18,6 +18,8 @@ package ghidra.program.model.data;
 import java.math.BigInteger;
 import java.util.*;
 
+import org.apache.commons.lang3.StringUtils;
+
 import ghidra.docking.settings.Settings;
 import ghidra.docking.settings.SettingsDefinition;
 import ghidra.program.database.data.DataTypeUtilities;
@@ -95,12 +97,12 @@ public class EnumDataType extends GenericDataType implements Enum {
 	}
 
 	@Override
-	public String getComment(String valueName) throws NoSuchElementException {
+	public String getComment(String valueName) {
-		String valueNameComment = commentMap.get(valueName);
+		String comment = commentMap.get(valueName);
-		if (valueNameComment == null) {
+		if (comment == null) {
-			valueNameComment = "";
+			comment = "";
 		}
-		return valueNameComment;
+		return comment;
 	}
 
 	@Override
@@ -117,11 +119,6 @@ public class EnumDataType extends GenericDataType implements Enum {
 		return names;
 	}
 
-	@Override
-	public String[] getComments() {
-		return commentMap.values().toArray(new String[commentMap.size()]);
-	}
-
 	@Override
 	public int getCount() {
 		return nameMap.size();
@@ -129,28 +126,25 @@ public class EnumDataType extends GenericDataType implements Enum {
 
 	@Override
 	public void add(String valueName, long value) {
-		String valueNameComment = "";
+		add(valueName, value, null);
-		add(valueName, value, valueNameComment);
 	}
 
 	@Override
-	public void add(String valueName, long value, String valueNameComment) {
+	public void add(String valueName, long value, String comment) {
 		bitGroups = null;
 		checkValue(value);
 		if (nameMap.containsKey(valueName)) {
 			throw new IllegalArgumentException(valueName + " already exists in this enum");
 		}
+
 		nameMap.put(valueName, value);
-		List<String> list = valueMap.get(value);
+		List<String> list = valueMap.computeIfAbsent(value, v -> new ArrayList<>());
-		if (list == null) {
-			list = new ArrayList<>();
-			valueMap.put(value, list);
-		}
 		list.add(valueName);
-		if (valueNameComment == null) {
+
-			valueNameComment = "";
+		if (!StringUtils.isBlank(comment)) {
+			commentMap.put(valueName, comment);
 		}
-		commentMap.put(valueName, valueNameComment);
+
 	}
 
 	private void checkValue(long value) {
@@ -180,21 +174,24 @@ public class EnumDataType extends GenericDataType implements Enum {
 	public void remove(String valueName) {
 		bitGroups = null;
 		Long value = nameMap.get(valueName);
-		if (value != null) {
+		if (value == null) {
-			nameMap.remove(valueName);
+			return;
-			List<String> list = valueMap.get(value);
-			Iterator<String> iter = list.iterator();
-			while (iter.hasNext()) {
-				if (valueName.equals(iter.next())) {
-					iter.remove();
-					break;
-				}
-			}
-			if (list.isEmpty()) {
-				valueMap.remove(value);
-			}
-			commentMap.remove(valueName);
 		}
+
+		nameMap.remove(valueName);
+		List<String> list = valueMap.get(value);
+		Iterator<String> iter = list.iterator();
+		while (iter.hasNext()) {
+			if (valueName.equals(iter.next())) {
+				iter.remove();
+				break;
+			}
+		}
+		if (list.isEmpty()) {
+			valueMap.remove(value);
+		}
+
+		commentMap.remove(valueName);
 	}
 
 	@Override
@@ -231,11 +228,11 @@ public class EnumDataType extends GenericDataType implements Enum {
 
 	public void setLength(int length) {
 		String[] names = getNames();
-		for (String enumName : names) {
+		for (String valueName : names) {
-			long value = getValue(enumName);
+			long value = getValue(valueName);
 			if (isTooBig(length, value)) {
 				throw new IllegalArgumentException("Setting the length of this Enum to a size " +
-					"that cannot contain the current value for \"" + enumName + "\" of " +
+					"that cannot contain the current value for \"" + valueName + "\" of " +
 					Long.toHexString(value));
 			}
 		}
@@ -327,7 +324,7 @@ public class EnumDataType extends GenericDataType implements Enum {
 			return "0";
 		}
 		List<BitGroup> list = getBitGroups();
-		StringBuffer buf = new StringBuffer();
+		StringBuilder buf = new StringBuilder();
 		for (BitGroup bitGroup : list) {
 			long subValue = bitGroup.getMask() & value;
 			if (subValue != 0) {
@@ -372,30 +369,45 @@ public class EnumDataType extends GenericDataType implements Enum {
 		if (dt == null || !(dt instanceof Enum)) {
 			return false;
 		}
-		Enum enumm = (Enum) dt;
 
+		Enum enumm = (Enum) dt;
 		if (!DataTypeUtilities.equalsIgnoreConflict(name, enumm.getName()) ||
 			length != enumm.getLength() || getCount() != enumm.getCount()) {
 			return false;
 		}
+
+		if (!isEachValueEquivalent(enumm)) {
+			return false;
+		}
+		return true;
+	}
+
+	private boolean isEachValueEquivalent(Enum enumm) {
 		String[] names = getNames();
 		String[] otherNames = enumm.getNames();
 		try {
 			for (int i = 0; i < names.length; i++) {
+				if (!names[i].equals(otherNames[i])) {
+					return false;
+				}
+
 				long value = getValue(names[i]);
 				long otherValue = enumm.getValue(names[i]);
+				if (value != otherValue) {
+					return false;
+				}
+
 				String comment = getComment(names[i]);
 				String otherComment = enumm.getComment(names[i]);
-				if (!names[i].equals(otherNames[i]) || value != otherValue ||
+				if (!comment.equals(otherComment)) {
-					!comment.equals(otherComment)) {
 					return false;
 				}
 			}
+			return true;
 		}
 		catch (NoSuchElementException e) {
 			return false; // named element not found
 		}
-		return true;
 	}
 
 	@Override
@@ -410,14 +422,14 @@ public class EnumDataType extends GenericDataType implements Enum {
 		commentMap = new HashMap<>();
 		setLength(enumm.getLength());
 		String[] names = enumm.getNames();
-		for (String name2 : names) {
+		for (String valueName : names) {
-			add(name2, enumm.getValue(name2), enumm.getComment(name2));
+			add(valueName, enumm.getValue(valueName), enumm.getComment(valueName));
 		}
 		stateChanged(null);
 	}
 
 	@Override
 	public String getDefaultLabelPrefix() {
-		return name == null ? null : name.toUpperCase();
+		return name;
 	}
 }

Ghidra/Test/IntegrationTest/src/screen/java/help/screenshot/DataTypeEditorsScreenShots.java

@@ -66,7 +66,7 @@ public class DataTypeEditorsScreenShots extends GhidraScreenShotGenerator {
 		dataTypeWindows.add(dataTypeDialog);
 
 		captureComponents(dataTypeWindows);
-		closeAllWindowsAndFrames();
+		closeAllWindows();
 	}
 
 	private DropDownSelectionTextField<?> showTypeChooserDialog() throws Exception {
@@ -189,7 +189,7 @@ public class DataTypeEditorsScreenShots extends GhidraScreenShotGenerator {
 		waitForSwing();
 
 		captureDialog();
-		closeAllWindowsAndFrames();
+		closeAllWindows();
 	}
 
 	@Test