GT-2824 - Comments - fixed infinite loop when editing comments

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/comments/CommentHistoryPanel.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.
@@ -27,6 +26,7 @@ import javax.swing.text.*;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.listing.CommentHistory;
 import ghidra.program.model.listing.Program;
+import ghidra.util.Msg;
 
 /**
  * Panel that shows comment history for a particular comment type; uses
@@ -34,13 +34,13 @@ import ghidra.program.model.listing.Program;
  * readability.
  */
 class CommentHistoryPanel extends JPanel {
-	
+
 	private final static String NO_HISTORY = "No History Found";
 	private SimpleAttributeSet userAttrSet;
 	private SimpleAttributeSet dateAttrSet;
 	private SimpleAttributeSet textAttrSet;
 	private SimpleAttributeSet tabAttrSet;
-	
+
 	private StyledDocument doc;
 	private JTextPane textPane;
 
@@ -52,87 +52,87 @@ class CommentHistoryPanel extends JPanel {
 	 * @param commentType comment type
 	 */
 	CommentHistoryPanel(int commentType) {
-			
+
 		super(new BorderLayout());
 		setUpAttributes();
 		this.commentType = commentType;
 		formatter = new SimpleDateFormat("yyyy MMM dd hh:mm aaa");
 		create();
 	}
-	
+
 	/**
 	 * Show the comment history
 	 * @param program program 
 	 * @param addr address of comment history
 	 */
 	void showCommentHistory(Program program, Address addr) {
-		
+
 		textPane.setText("");
-		
+
-		CommentHistory[] historyItems = 
+		CommentHistory[] historyItems = program.getListing().getCommentHistory(addr, commentType);
-			program.getListing().getCommentHistory(addr, commentType);	
 		try {
 			if (historyItems.length == 0) {
 				doc.insertString(0, NO_HISTORY, null);
 				doc.setCharacterAttributes(0, NO_HISTORY.length(), textAttrSet, true);
 				return;
 			}
-			for (int i=0; i<historyItems.length; i++) {
+			for (CommentHistory historyItem : historyItems) {
-				formatHistory(historyItems[i]);
+				formatHistory(historyItem);
 			}
-		} catch (BadLocationException e) {
+		}
+		catch (BadLocationException e) {
+			// shouldn't happen
+			Msg.debug(this, "Error setting comment text field text", e);
 		}
 		textPane.setCaretPosition(0);
 	}
-	
+
 	private void create() {
 		textPane = new JTextPane();
 		textPane.setEditable(false);
 		add(textPane, BorderLayout.CENTER);
-		doc = textPane.getStyledDocument(); 
+		doc = textPane.getStyledDocument();
 	}
 
-	private void formatHistory(CommentHistory history) 
+	private void formatHistory(CommentHistory history) throws BadLocationException {
-		throws BadLocationException {
+
-			
 		int offset = doc.getLength();
 		String userName = history.getUserName();
-		
+
 		if (offset > 0) {
 			userName = "\n" + userName;
 		}
 		doc.insertString(offset, userName, userAttrSet);
-		
+
 		offset = doc.getLength();
-		doc.insertString(offset, "\t" + formatter.format(history.getModificationDate()), 
+		doc.insertString(offset, "\t" + formatter.format(history.getModificationDate()),
-						 dateAttrSet);
+			dateAttrSet);
 		doc.setParagraphAttributes(offset, 1, tabAttrSet, false);
-		
+
 		offset = doc.getLength();
-		doc.insertString(offset, "\n"+ history.getComments()+"\n", textAttrSet);
+		doc.insertString(offset, "\n" + history.getComments() + "\n", textAttrSet);
 	}
-	
+
 	private void setUpAttributes() {
 		textAttrSet = new SimpleAttributeSet();
 		textAttrSet.addAttribute(StyleConstants.FontFamily, "Monospaced");
-		textAttrSet.addAttribute(StyleConstants.FontSize, new Integer(12));
+		textAttrSet.addAttribute(StyleConstants.FontSize, Integer.valueOf(12));
 		textAttrSet.addAttribute(StyleConstants.Foreground, Color.BLUE);
-		
+
 		userAttrSet = new SimpleAttributeSet();
 		userAttrSet.addAttribute(StyleConstants.FontFamily, "Tahoma");
-		userAttrSet.addAttribute(StyleConstants.FontSize, new Integer(12));
+		userAttrSet.addAttribute(StyleConstants.FontSize, Integer.valueOf(12));
 		userAttrSet.addAttribute(StyleConstants.Bold, Boolean.TRUE);
 
 		dateAttrSet = new SimpleAttributeSet();
 		dateAttrSet.addAttribute(StyleConstants.FontFamily, "Tahoma");
-		dateAttrSet.addAttribute(StyleConstants.FontSize, new Integer(11));
+		dateAttrSet.addAttribute(StyleConstants.FontSize, Integer.valueOf(11));
 		dateAttrSet.addAttribute(StyleConstants.Bold, Boolean.TRUE);
-		dateAttrSet.addAttribute(StyleConstants.Foreground,
+		dateAttrSet.addAttribute(StyleConstants.Foreground, new Color(124, 37, 18));
-			new Color(124,37,18)); 
+
-		
 		tabAttrSet = new SimpleAttributeSet();
 		TabStop tabs = new TabStop(100, StyleConstants.ALIGN_LEFT, TabStop.LEAD_NONE);
-		StyleConstants.setTabSet(tabAttrSet, new TabSet(new TabStop[]{tabs}));
+		StyleConstants.setTabSet(tabAttrSet, new TabSet(new TabStop[] { tabs }));
 	}
 
 }

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/script/GhidraScriptComponentProvider.java

@@ -27,6 +27,8 @@ import javax.swing.text.html.HTMLEditorKit;
 import javax.swing.tree.TreePath;
 import javax.swing.tree.TreeSelectionModel;
 
+import org.apache.commons.lang3.StringUtils;
+
 import docking.ActionContext;
 import docking.action.KeyBindingData;
 import docking.event.mouse.GMouseListenerAdapter;
@@ -44,7 +46,8 @@ import ghidra.app.services.ConsoleService;
 import ghidra.framework.options.SaveState;
 import ghidra.framework.plugintool.ComponentProviderAdapter;
 import ghidra.program.model.listing.Program;
-import ghidra.util.*;
+import ghidra.util.HelpLocation;
+import ghidra.util.Msg;
 import ghidra.util.datastruct.WeakDataStructureFactory;
 import ghidra.util.datastruct.WeakSet;
 import ghidra.util.table.GhidraTableFilterPanel;
@@ -226,7 +229,7 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter {
 			reader.close();
 			writer.close();
 
-			FileUtilities.copyFile(temp, renameFile, TaskMonitorAdapter.DUMMY_MONITOR);
+			FileUtilities.copyFile(temp, renameFile, TaskMonitor.DUMMY);
 
 			if (!renameFile.exists()) {
 				Msg.showWarn(getClass(), getComponent(), "Unable to rename script",
@@ -378,8 +381,7 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter {
 
 			checkNewScriptDirectoryEnablement(newFile);
 
-			String category = StringUtilities.convertStringArray(getSelectedCategoryPath(),
+			String category = StringUtils.join(getSelectedCategoryPath(), ScriptInfo.DELIMITTER);
-				ScriptInfo.DELIMITTER);
 			provider.createNewScript(newFile, category);
 
 			GhidraScriptEditorComponentProvider editor =
@@ -477,7 +479,7 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter {
 		tableModel.fireTableDataChanged();
 	}
 
-	/**
+	/*
 	 * is more than just root node selected?
 	 */
 	boolean isSelectedCategory() {

Ghidra/Features/Base/src/main/java/ghidra/app/script/ScriptInfo.java

@@ -15,7 +15,8 @@
  */
 package ghidra.app.script;
 
-import static ghidra.util.HTMLUtilities.*;
+import static ghidra.util.HTMLUtilities.HTML_NEW_LINE;
+import static ghidra.util.HTMLUtilities.HTML_SPACE;
 
 import java.io.*;
 import java.util.List;
@@ -25,9 +26,12 @@ import java.util.regex.Pattern;
 import javax.swing.ImageIcon;
 import javax.swing.KeyStroke;
 
+import org.apache.commons.lang3.StringUtils;
+
 import docking.DockingKeyBindingAction;
 import generic.jar.ResourceFile;
-import ghidra.util.*;
+import ghidra.util.HTMLUtilities;
+import ghidra.util.Msg;
 import resources.ResourceManager;
 
 /**
@@ -77,8 +81,8 @@ public class ScriptInfo {
 		this.sourceFile = sourceFile;
 
 		if (!sourceFile.exists()) {
-			throw new IllegalArgumentException("Source file for script does not exist!: " +
+			throw new IllegalArgumentException(
-				sourceFile);
+				"Source file for script does not exist!: " + sourceFile);
 		}
 	}
 
@@ -92,7 +96,7 @@ public class ScriptInfo {
 		toolbarImage = null;
 		keybindingErrorMessage = null;
 	}
-	
+
 	/**
 	 * Setting the toolbar image to null forces it to be reloaded on the next request.
 	 */
@@ -402,7 +406,7 @@ public class ScriptInfo {
 	 * @return the script tool bar icon
 	 */
 	public ImageIcon getToolBarImage(boolean scaled) {
-		
+
 		parseHeader();
 		if (toolbar == null) {
 			return null;
@@ -432,18 +436,15 @@ public class ScriptInfo {
 	 * @return a string designed to be used as a tool tip
 	 */
 	public String getToolTipText() {
-		String htmlDescription =
+		String htmlDescription = description == null ? "No Description"
-			description == null ? "No Description" : description.replaceAll("\n", HTML_NEW_LINE +
+				: description.replaceAll("\n", HTML_NEW_LINE + HTML_SPACE);
-				HTML_SPACE);
 		String htmlAuthor = HTMLUtilities.bold("Author:") + HTML_SPACE + (toToolTip(author));
-		String htmlCategory =
+		String htmlCategory = HTMLUtilities.bold("Category:") + HTML_SPACE +
-			HTMLUtilities.bold("Category:") + HTML_SPACE +
+			toToolTip(StringUtils.join(category, DELIMITTER));
-				toToolTip(StringUtilities.convertStringArray(category, "."));
 		String htmlKeyBinding =
 			HTMLUtilities.bold("Key Binding:") + HTML_SPACE + getKeybindingToolTip();
-		String htmlMenuPath =
+		String htmlMenuPath = HTMLUtilities.bold("Menu Path:") + HTML_SPACE +
-			HTMLUtilities.bold("Menu Path:") + HTML_SPACE +
+			toToolTip(StringUtils.join(menupath, DELIMITTER));
-				toToolTip(StringUtilities.convertStringArray(menupath, "."));
 
 		StringBuffer buffer = new StringBuffer();
 		buffer.append("<h3>").append(HTML_SPACE).append(getName()).append("</h3>");

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/symboltree/SymbolTreeTestUtils.java

@@ -32,6 +32,8 @@ import javax.swing.JTree;
 import javax.swing.tree.DefaultTreeCellEditor;
 import javax.swing.tree.TreePath;
 
+import org.apache.commons.lang3.StringUtils;
+
 import docking.ActionContext;
 import docking.action.DockingActionIf;
 import docking.action.ToggleDockingAction;
@@ -46,7 +48,6 @@ import ghidra.program.model.data.Undefined1DataType;
 import ghidra.program.model.listing.*;
 import ghidra.program.model.symbol.*;
 import ghidra.test.ToyProgramBuilder;
-import ghidra.util.StringUtilities;
 
 /**
  * Utility class that has common methods needed by the Junit tests.
@@ -410,8 +411,7 @@ class SymbolTreeTestUtils {
 		if (!rootNode.getName().equals(rootName)) {
 			throw new RuntimeException(
 				"When selecting paths by name the first path element must be the " +
-					"name of the root node - path: " +
+					"name of the root node - path: " + StringUtils.join(path, '.'));
-					StringUtilities.convertStringArray(path, "."));
 		}
 		GTreeNode node = rootNode;
 		for (int i = 1; i < path.length; i++) {

Ghidra/Framework/Docking/src/main/java/docking/test/AbstractDockingTest.java

@@ -1989,15 +1989,14 @@ public abstract class AbstractDockingTest extends AbstractGenericTest {
 		if (!rootNode.getName().equals(rootName)) {
 			throw new RuntimeException(
 				"When selecting paths by name the first path element must be the " +
-					"name of the root node - path: " +
+					"name of the root node - path: " + StringUtils.join(path, '.'));
-					StringUtilities.convertStringArray(path, "."));
 		}
 		GTreeNode node = rootNode;
 		for (int i = 1; i < path.length; i++) {
 			GTreeNode child = node.getChild(path[i]);
 			if (child == null) {
-				throw new RuntimeException("Can't find path " +
+				throw new RuntimeException(
-					StringUtilities.convertStringArray(path, ".") + "   failed at " + path[i]);
+					"Can't find path " + StringUtils.join(path, '.') + "   failed at " + path[i]);
 			}
 			node = child;
 		}

Ghidra/Framework/Docking/src/main/java/docking/widgets/tree/tasks/GTreeSelectNodeByNameTask.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,17 +15,17 @@
  */
 package docking.widgets.tree.tasks;
 
-import ghidra.util.Msg;
-import ghidra.util.StringUtilities;
-import ghidra.util.exception.CancelledException;
-import ghidra.util.task.TaskMonitor;
-
 import javax.swing.JTree;
 import javax.swing.tree.TreePath;
 
+import org.apache.commons.lang3.StringUtils;
+
 import docking.widgets.tree.*;
 import docking.widgets.tree.internal.GTreeSelectionModel;
 import docking.widgets.tree.support.GTreeSelectionEvent.EventOrigin;
+import ghidra.util.Msg;
+import ghidra.util.exception.CancelledException;
+import ghidra.util.task.TaskMonitor;
 
 public class GTreeSelectNodeByNameTask extends GTreeTask {
 
@@ -49,7 +48,7 @@ public class GTreeSelectNodeByNameTask extends GTreeTask {
 		String rootName = names[0];
 		if (!node.getName().equals(rootName)) {
 			Msg.debug(this, "When selecting paths by name the first path element must be the " +
-				"name of the root node - path: " + StringUtilities.convertStringArray(names, "."));
+				"name of the root node - path: " + StringUtils.join(names, '.'));
 			return;
 		}
 
@@ -57,10 +56,8 @@ public class GTreeSelectNodeByNameTask extends GTreeTask {
 			monitor.checkCanceled();
 			node = findNodeByName(node, names[i], monitor);
 			if (node == null) {
-				Msg.debug(
+				Msg.debug(this,
-					this,
+					"Could not find node to select - path: " + StringUtils.join(names, '.'));
-					"Could not find node to select - path: " +
-						StringUtilities.convertStringArray(names, "."));
 				return;
 			}
 		}
@@ -80,17 +77,14 @@ public class GTreeSelectNodeByNameTask extends GTreeTask {
 	}
 
 	private void selectPath(final TreePath treePath, final TaskMonitor monitor) {
-		runOnSwingThread(new Runnable() {
+		runOnSwingThread(() -> {
-			@Override
+			if (monitor.isCancelled()) {
-			public void run() {
+				return; // we can be cancelled while waiting for Swing to run us
-				if (monitor.isCancelled()) {
-					return; // we can be cancelled while waiting for Swing to run us
-				}
-
-				GTreeSelectionModel selectionModel = tree.getGTSelectionModel();
-				selectionModel.setSelectionPaths(new TreePath[] { treePath }, origin);
-				jTree.scrollPathToVisible(treePath);
 			}
+
+			GTreeSelectionModel selectionModel = tree.getGTSelectionModel();
+			selectionModel.setSelectionPaths(new TreePath[] { treePath }, origin);
+			jTree.scrollPathToVisible(treePath);
 		});
 	}
 

Ghidra/Framework/Generic/src/main/java/ghidra/util/StringDiff.java

@@ -1,105 +0,0 @@
-/* ###
- * 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.util;
-
-import org.apache.commons.lang3.StringUtils;
-
-/**
- * Container object that holds a start and end position within a string.
- * A list of StringDiffs is used to keep track of changes made to a string.
- *
- */
-public class StringDiff {
-	/**
-	 * Start position of the string.
-	 */
-	public int pos1;
-	/**
-	 * End position of the string used when part of the string is replaced. 
-	 */
-	public int pos2;
-	/**
-	 * String being inserted.
-	 */
-	public String insertData;
-
-	/**
-	 * Construct a new StringDiff with pos1 and pos2 are initialized to -1.
-	 * @param replaceData string 
-	 */
-	public StringDiff(String replaceData) {
-		pos1 = -1;
-		pos2 = -1;
-		insertData = replaceData;
-	}
-
-	/**
-	 * Construct a new StringDiff that indicates text was deleted from
-	 * pos1 to pos2.
-	 * @param pos1 position 1 for the diff
-	 * @param pos2 position 2 for the diff
-	 */
-	public StringDiff(int pos1, int pos2) {
-		this.pos1 = pos1;
-		this.pos2 = pos2;
-	}
-
-	/**
-	 * Construct a new StringDiff that indicates that insertData was
-	 * inserted at pos.
-	 * @param pos position where the insertData was inserted
-	 * @param insertData inserted string
-	 */
-	public StringDiff(int pos, String insertData) {
-		this.pos1 = pos;
-		this.insertData = insertData;
-	}
-
-	/**
-	 * Construct a new StringDiff that indicates given data is inserted
-	 * from pos1 to pos2.
-	 * @param pos1 position 1
-	 * @param pos2 position 2
-	 * @param data data the replaces string data
-	 */
-	public StringDiff(int pos1, int pos2, String data) {
-		this.pos1 = pos1;
-		this.pos2 = pos2;
-		insertData = data;
-	}
-
-	@Override
-	public boolean equals(Object obj) {
-		if (obj instanceof StringDiff) {
-			StringDiff other = (StringDiff) obj;
-			return pos1 == other.pos1 && pos2 == other.pos2 &&
-				StringUtils.equals(insertData, other.insertData);
-		}
-		return false;
-	}
-
-	@Override
-	public String toString() {
-		if (insertData != null) {
-			if (pos1 >= 0) {
-				return "StringDiff: inserted <" + insertData + "> at " + pos1;
-			}
-
-			return "StringDiff: replace with <" + insertData + ">";
-		}
-		return "StringDiff: deleted text from " + pos1 + " to " + pos2;
-	}
-}

Ghidra/Framework/Generic/src/main/java/ghidra/util/StringUtilities.java

@@ -138,7 +138,9 @@ public class StringUtilities {
 	}
 
 	/**
-	 * Returns true if the character is displayable.
+	 * Returns true if the character is in displayable character range
+	 * @param c the character
+	 * @return true if the character is in displayable character range
 	 */
 	public static boolean isDisplayable(int c) {
 		return c >= 0x20 && c < 0x7F;
@@ -547,33 +549,6 @@ public class StringUtilities {
 		return buffer.toString();
 	}
 
-	/**
-	 * Convert a string array to single string with new line chars.
-	 */
-	public static String convertStringArray(String[] strings) {
-		return convertStringArray(strings, "\n");
-	}
-
-	/**
-	 * Convert a string array to single string with the given delimiter.
-	 */
-	public static String convertStringArray(String[] strings, String delimiter) {
-		if (strings == null || strings.length == 0) {
-			return null;
-		}
-		StringBuffer sb = new StringBuffer();
-		for (int i = 0; i < strings.length; i++) {
-			if (strings[i] == null) {
-				continue;
-			}
-			sb.append(strings[i]);
-			if (i < strings.length - 1) {
-				sb.append(delimiter);
-			}
-		}
-		return sb.toString();
-	}
-
 	/**
 	 * Parses a string containing multiple lines into an array where each
 	 * element in the array contains only a single line. The "\n" character is
@@ -638,6 +613,7 @@ public class StringUtilities {
 	 * @param source the original string to pad.
 	 * @param filler the type of characters with which to pad
 	 * @param length the length of padding to add (0 results in no changes)
+	 * @return the padded string
 	 * @deprecated use {@link #pad(String, char, int)}; functionally the same, but smaller
 	 *             and more consistent name
 	 */
@@ -654,6 +630,7 @@ public class StringUtilities {
 	 * @param source the original string to pad.
 	 * @param filler the type of characters with which to pad
 	 * @param length the length of padding to add (0 results in no changes)
+	 * @return the padded string
 	 */
 	public static String pad(String source, char filler, int length) {
 
@@ -690,6 +667,7 @@ public class StringUtilities {
 	 * This is useful for constructing complicated <code>toString()</code> representations.
 	 *
 	 * @param s the input string
+	 * @param indent the indent string; this will be appended as needed
 	 * @return the output string
 	 */
 	public static String indentLines(String s, String indent) {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/CodeManager.java

@@ -3365,12 +3365,15 @@ public class CodeManager implements ErrorHandler, ManagerDB {
 		if (newComment == null) {
 			newComment = "";
 		}
-		StringDiff[] diffs = getLineDiffs(newComment, oldComment);
+
+		StringDiff[] diffs = StringDiffer.getLineDiffs(newComment, oldComment);
+
+		long date = System.currentTimeMillis();
 		long addr = addrMap.getKey(address, true);
 		try {
 			for (StringDiff diff : diffs) {
-				historyAdapter.createRecord(addr, (byte) commentType, diff.pos1, diff.pos2,
+				historyAdapter.createRecord(addr, (byte) commentType, diff.start, diff.end,
-					diff.insertData);
+					diff.text, date);
 			}
 		}
 		catch (IOException e) {
@@ -3379,7 +3382,8 @@ public class CodeManager implements ErrorHandler, ManagerDB {
 	}
 
 	/**
-	 * Get the comment history for the comment type at the given address.
+	 * Get the comment history for the comment type at the given address 
+	 * 
 	 * @param addr address for the comment history
 	 * @param commentType comment type
 	 * @return zero length array if no history exists
@@ -3387,47 +3391,37 @@ public class CodeManager implements ErrorHandler, ManagerDB {
 	public CommentHistory[] getCommentHistory(Address addr, int commentType) {
 		lock.acquire();
 		try {
-			RecordIterator iter = historyAdapter.getRecordsByAddress(addr);
 
-			List<Record> list = new ArrayList<>();
+			// records are sorted by date ascending						
-			while (iter.hasNext()) {
+			List<Record> allRecords = getHistoryRecords(addr, commentType);
-				Record rec = iter.next();
 
-				if (rec.getByteValue(CommentHistoryAdapter.HISTORY_TYPE_COL) == commentType) {
+			List<CommentHistory> results = new ArrayList<>();
-					list.add(rec);
+			String comment = getComment(addr, commentType);
-				}
+			while (!allRecords.isEmpty()) {
-			}
+
-			List<CommentHistory> historyList = new ArrayList<>(); // CommentHistory objects
+				Record rec = allRecords.get(allRecords.size() - 1);
-			String comments = getComments(addr, commentType);
-			while (list.size() > 0) {
-				Record rec = list.get(list.size() - 1);
 				long date = rec.getLongValue(CommentHistoryAdapter.HISTORY_DATE_COL);
-				List<Record> subList = findHistoryRecords(date, list);
+				List<Record> records = subListByDate(allRecords, date);
-				StringDiff[] diffs = new StringDiff[subList.size()];
+				List<StringDiff> diffs = new ArrayList<>(records.size());
 
-				String userName = null;
+				String user = null;
-				Date modDate = null;
+				for (int i = 0; i < records.size(); i++) {
-				for (int j = 0; j < subList.size(); j++) {
+					Record r = records.get(i);
-					Record r = subList.get(j);
+					user = r.getString(CommentHistoryAdapter.HISTORY_USER_COL);
-					userName = r.getString(CommentHistoryAdapter.HISTORY_USER_COL);
+					int pos1 = r.getIntValue(CommentHistoryAdapter.HISTORY_POS1_COL);
-					modDate = new Date(r.getLongValue(CommentHistoryAdapter.HISTORY_DATE_COL));
+					int pos2 = r.getIntValue(CommentHistoryAdapter.HISTORY_POS2_COL);
-
+					String data = r.getString(CommentHistoryAdapter.HISTORY_STRING_COL);
-					diffs[j] = new StringDiff(r.getIntValue(CommentHistoryAdapter.HISTORY_POS1_COL),
+					diffs.add(StringDiff.restore(data, pos1, pos2));
-						r.getIntValue(CommentHistoryAdapter.HISTORY_POS2_COL),
-						r.getString(CommentHistoryAdapter.HISTORY_STRING_COL));
 				}
-				if (comments == null) {
-					comments = "";
-				}
-				historyList.add(new CommentHistory(addr, commentType, userName, comments, modDate));
-				comments = applyDiffs(comments, diffs);
 
-				int from = list.size() - subList.size();
+				results.add(new CommentHistory(addr, commentType, user, comment, new Date(date)));
-				// remove the subList elements from the list
+				comment = StringDiffer.applyDiffs(comment, diffs);
-				list.subList(from, list.size()).clear();
+
+				records.clear(); // remove the subList elements from the list
 			}
-			CommentHistory[] h = new CommentHistory[historyList.size()];
+
-			return historyList.toArray(h);
+			CommentHistory[] h = new CommentHistory[results.size()];
+			return results.toArray(h);
 		}
 		catch (IOException e) {
 			dbError(e);
@@ -3438,23 +3432,39 @@ public class CodeManager implements ErrorHandler, ManagerDB {
 		return new CommentHistory[0];
 	}
 
-	private List<Record> findHistoryRecords(long date, List<Record> recList) {
+	// note: you must have the lock when calling this method
-		int i;
+	private List<Record> getHistoryRecords(Address addr, int commentType) throws IOException {
-		for (i = recList.size() - 1; i >= 0; i--) {
+		RecordIterator it = historyAdapter.getRecordsByAddress(addr);
-			Record rec = recList.get(i);
+		List<Record> list = new ArrayList<>();
-			if (date != rec.getLongValue(CommentHistoryAdapter.HISTORY_DATE_COL)) {
+		while (it.hasNext()) {
-				break;
+			Record rec = it.next();
+			if (rec.getByteValue(CommentHistoryAdapter.HISTORY_TYPE_COL) == commentType) {
+				list.add(rec);
 			}
 		}
-		return recList.subList(i + 1, recList.size());
+		return list;
 	}
 
-	private String getComments(Address addr, int commentType) throws IOException {
+	// note: records are sorted by date; assume that the date we seek is at the end
+	private List<Record> subListByDate(List<Record> records, long date) {
+
+		for (int i = records.size() - 1; i >= 0; i--) {
+			Record rec = records.get(i);
+			if (date != rec.getLongValue(CommentHistoryAdapter.HISTORY_DATE_COL)) {
+				return records.subList(i + 1, records.size());
+			}
+		}
+
+		// all records have the same date
+		return records.subList(0, records.size());
+	}
+
+	private String getComment(Address addr, int commentType) throws IOException {
 		Record record = commentAdapter.getRecord(addrMap.getKey(addr, false));
 		if (record != null) {
 			return record.getString(commentType);
 		}
-		return null;
+		return "";
 	}
 
 	public void replaceDataTypes(long oldDataTypeID, long newDataTypeID) {
@@ -3631,181 +3641,4 @@ public class CodeManager implements ErrorHandler, ManagerDB {
 		return protoMgr.getPrototype(protoID);
 	}
 
-	/**
-	 * Returns the list of StringDiff objects that if applied to s1 would result in s2;  The
-	 * given text will look only for whole lines using '\n'.
-	 *
-	 * @param s1 the original string
-	 * @param s2 the result string
-	 *        this value, then a completely different string will be returned
-	 * @return an array of StringDiff objects that change s1 into s2;
-	 */
-	private static StringDiff[] getLineDiffs(String s1, String s2) {
-
-		/**
-		 * Minimum size used to determine whether a new StringDiff object will be
-		 * created just using a string (no positions)
-		 * in the <code>getDiffs(String, String)</code> method.
-		 * @see #getLineDiffs(String, String)
-		 */
-		int MINIMUM_DIFF_SIZE = 100;
-		return getLineDiffs(s1, s2, MINIMUM_DIFF_SIZE);
-	}
-
-	/**
-	 * Returns the list of StringDiff objects that if applied to s1 would result in s2;  The
-	 * given text will look only for whole lines using '\n'.
-	 *
-	 * @param s1 the original string
-	 * @param s2 the result string
-	 * @param minimumDiffSize the minimum length of s2 required for a diff; if s2 is less than
-	 *        this value, then a completely different string will be returned
-	 * @return an array of StringDiff objects that change s1 into s2;
-	 */
-	private static StringDiff[] getLineDiffs(String s1, String s2, int minimumDiffSize) {
-		if (s2.length() < minimumDiffSize) {
-			return new StringDiff[] { new StringDiff(s2) };
-		}
-
-		List<StringDiff> list = new LinkedList<>();
-		int pos1 = 0;
-		int pos2 = 0;
-		int len1 = s1.length();
-		int len2 = s2.length();
-		int origPos;
-
-		while (pos1 < len1 || pos2 < len2) {
-			String line1 = getLine(s1, pos1);
-			String line2 = getLine(s2, pos2);
-			if (line1.equals(line2)) {
-				pos1 += line1.length();
-				pos2 += line2.length();
-				continue;
-			}
-			int posInOther1 = findLine(s2, pos2, line1);
-			origPos = pos1;
-			while (posInOther1 < 0) {
-				pos1 += line1.length();
-				line1 = getLine(s1, pos1);
-				posInOther1 = findLine(s2, pos2, line1);
-			}
-			if (pos1 > origPos) {
-				list.add(new StringDiff(origPos, pos1));
-			}
-			int posInOther2 = findLine(s1, pos1, line2);
-			origPos = pos2;
-			while (posInOther2 < 0) {
-				pos2 += line2.length();
-				line2 = getLine(s2, pos2);
-				posInOther2 = findLine(s1, pos1, line2);
-			}
-			if (pos2 > origPos) {
-				list.add(new StringDiff(pos1, s2.substring(origPos, pos2)));
-				continue;
-			}
-			int advance1 = posInOther2 - pos1;
-			int advance2 = posInOther1 - pos2;
-			if (advance1 > advance2) {
-				list.add(new StringDiff(pos1, s2.substring(pos2, posInOther1)));
-				pos2 = posInOther1;
-			}
-			else if (advance2 > advance1) {
-				list.add(new StringDiff(pos1, posInOther2));
-				pos1 = posInOther2;
-			}
-		}
-		return list.toArray(new StringDiff[list.size()]);
-	}
-
-	/**
-	 * Finds a position in s that contains the string line.  The matching string in
-	 * s must be a "complete" line, in other words if pos > 0 then s.charAt(index-1) must be
-	 * a newLine character and s.charAt(index+line.length()) must be a newLine or the end of
-	 * the string.
-	 * @param s the string to scan
-	 * @param pos the position to begin the scan.
-	 * @param line the line to scan for
-	 * @return the position in s containing the line string.
-	 */
-	private static int findLine(String s, int pos, String line) {
-		if (line.length() == 0) {
-			return pos;
-		}
-		while (true) {
-			int index = s.indexOf(line, pos);
-			if (index < 0) {
-				return index;
-			}
-			if (index > 0 && s.charAt(index - 1) != '\n') {
-				pos = index + line.length();
-				continue;
-			}
-			if (line.endsWith("\n")) {
-				return index;
-			}
-			if (index + line.length() == s.length()) {
-				return index;
-			}
-			pos = index + line.length();
-		}
-	}
-
-	/**
-	 * Returns a substring of s beginning at start and ending at either the end of the string or
-	 * the first newLine at or after start.
-	 * @param s the string to scan
-	 * @param start the starting position for the scan
-	 * @return A string that represents a line within s.
-	 */
-	public static String getLine(String s, int start) {
-		int n = s.length();
-		if (start >= n) {
-			return "";
-		}
-		int pos = start;
-		while (pos < n && s.charAt(pos) != '\n') {
-			pos++;
-		}
-		if (pos < n) {
-			pos++;
-		}
-		return s.substring(start, pos);
-	}
-
-	/**
-	 * Applies the array of StringObjects to the string s to produce a new string. Warning - the
-	 * diff objects cannot be applied to an arbitrary string, the String s must be the original
-	 * String used to compute the diffs.
-	 * @param s the original string
-	 * @param diffs the array of StringDiff object to apply
-	 * @return a new String resulting from applying the diffs to s.
-	 */
-	private static String applyDiffs(String s, StringDiff[] diffs) {
-		if (diffs.length == 0) {
-			return s;
-		}
-		if (diffs[0].pos1 < 0) {
-			return diffs[0].insertData;
-		}
-		StringBuffer buf = new StringBuffer(s.length());
-		int pos = 0;
-
-		for (StringDiff element : diffs) {
-			if (element.pos1 > pos) {
-				buf.append(s.substring(pos, element.pos1));
-				pos = element.pos1;
-			}
-			if (element.insertData != null) {
-				buf.append(element.insertData);
-			}
-			else {
-				pos = element.pos2;
-			}
-		}
-		if (pos < s.length()) {
-			buf.append(s.substring(pos));
-		}
-		return buf.toString();
-	}
-
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/CodeUnitDB.java

@@ -20,6 +20,8 @@ import java.math.BigInteger;
 import java.util.ConcurrentModificationException;
 import java.util.Iterator;
 
+import org.apache.commons.lang3.StringUtils;
+
 import db.Record;
 import ghidra.program.database.*;
 import ghidra.program.model.address.Address;
@@ -512,7 +514,7 @@ abstract class CodeUnitDB extends DatabaseObject implements CodeUnit, ProcessorC
 
 	@Override
 	public void setCommentAsArray(int commentType, String[] comment) {
-		setComment(commentType, StringUtilities.convertStringArray(comment));
+		setComment(commentType, StringUtils.join(comment, '\n'));
 	}
 
 	@Override

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/CommentHistoryAdapter.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.code;
 
+import java.io.IOException;
+
+import db.*;
 import ghidra.program.database.map.AddressMap;
 import ghidra.program.model.address.Address;
 import ghidra.util.exception.CancelledException;
 import ghidra.util.exception.VersionException;
 import ghidra.util.task.TaskMonitor;
 
-import java.io.IOException;
-
-import db.*;
-
 /**
  * Adapter for accessing records in the CommentHistory table.
  */
@@ -33,10 +31,10 @@ abstract class CommentHistoryAdapter {
 
 	static final String COMMENT_HISTORY_TABLE_NAME = "Comment History";
 
-	static final Schema COMMENT_HISTORY_SCHEMA = new Schema(0, "Key", new Class[] {
+	static final Schema COMMENT_HISTORY_SCHEMA = new Schema(0, "Key",
-		LongField.class, ByteField.class, IntField.class, IntField.class, StringField.class,
+		new Class[] { LongField.class, ByteField.class, IntField.class, IntField.class,
-		StringField.class, LongField.class }, new String[] { "Address", "Comment Type", "Pos1",
+			StringField.class, StringField.class, LongField.class },
-		"Pos2", "String Data", "User", "Date" });
+		new String[] { "Address", "Comment Type", "Pos1", "Pos2", "String Data", "User", "Date" });
 
 	static final int HISTORY_ADDRESS_COL = 0;
 	static final int HISTORY_TYPE_COL = 1;
@@ -79,14 +77,15 @@ abstract class CommentHistoryAdapter {
 			return new CommentHistoryAdapterV0(handle, addrMap.getOldAddressMap(), false);
 		}
 		catch (VersionException e) {
+			// use the 'no table' below
 		}
 
 		return new CommentHistoryAdapterNoTable();
 	}
 
 	private static CommentHistoryAdapter upgrade(DBHandle dbHandle, AddressMap addrMap,
-			CommentHistoryAdapter oldAdapter, TaskMonitor monitor) throws VersionException,
+			CommentHistoryAdapter oldAdapter, TaskMonitor monitor)
-			IOException, CancelledException {
+			throws VersionException, IOException, CancelledException {
 
 		AddressMap oldAddrMap = addrMap.getOldAddressMap();
 
@@ -128,7 +127,8 @@ abstract class CommentHistoryAdapter {
 	}
 
 	/**
-	 * Returns record count
+	 * Returns the record count
+	 * @return the record count
 	 */
 	abstract int getRecordCount();
 
@@ -139,15 +139,16 @@ abstract class CommentHistoryAdapter {
 	 * @param pos1 position 1 of change
 	 * @param pos2 position 2 of change
 	 * @param data string from the comment change
-	 * @throws IOException
+	 * @param date the date of the history entry
+	 * @throws IOException if there was a problem accessing the database
 	 */
-	abstract void createRecord(long addr, byte commentType, int pos1, int pos2, String data)
+	abstract void createRecord(long addr, byte commentType, int pos1, int pos2, String data,
-			throws IOException;
+			long date) throws IOException;
 
 	/**
 	 * Update record
-	 * @param rec
+	 * @param rec the record to update
-	 * @throws IOException
+	 * @throws IOException if there was a problem accessing the database
 	 */
 	abstract void updateRecord(Record rec) throws IOException;
 
@@ -162,12 +163,15 @@ abstract class CommentHistoryAdapter {
 
 	/**
 	 * Get an iterator over records with the given address.
+	 * @param addr the address for which to get records
+	 * @return the iterator
 	 * @throws IOException if there was a problem accessing the database
 	 */
 	abstract RecordIterator getRecordsByAddress(Address addr) throws IOException;
 
 	/**
-	 * Get an iterator over all records.
+	 * Get an iterator over all records
+	 * @return the iterator
 	 * @throws IOException if there was a problem accessing the database
 	 */
 	abstract RecordIterator getAllRecords() throws IOException;

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/CommentHistoryAdapterNoTable.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,13 +15,12 @@
  */
 package ghidra.program.database.code;
 
-import ghidra.program.database.util.EmptyRecordIterator;
-import ghidra.program.model.address.Address;
-
 import java.io.IOException;
 
 import db.Record;
 import db.RecordIterator;
+import ghidra.program.database.util.EmptyRecordIterator;
+import ghidra.program.model.address.Address;
 
 /**
  * Adapter needed for a read-only version of Program that is not going
@@ -30,53 +28,34 @@ import db.RecordIterator;
  */
 class CommentHistoryAdapterNoTable extends CommentHistoryAdapter {
 
-	/* (non Javadoc)
-	 * @see ghidra.program.database.code.CommentHistoryAdapter#createRecord(long, byte, int, int, java.lang.String)
-	 */
 	@Override
-	public void createRecord(long addr, byte commentType, int pos1, int pos2, String data)
+	public void createRecord(long addr, byte commentType, int pos1, int pos2, String data,
-			throws IOException {
+			long date) throws IOException {
 		throw new UnsupportedOperationException();
 	}
 
-	/**
-	 * @see ghidra.program.database.code.CommentHistoryAdapter#getRecordsByAddress(long)
-	 */
 	@Override
 	public RecordIterator getRecordsByAddress(Address addr) throws IOException {
 		return new EmptyRecordIterator();
 	}
 
-	/**
-	 * @see ghidra.program.database.code.CommentHistoryAdapter#getAllRecords()
-	 */
 	@Override
 	public RecordIterator getAllRecords() throws IOException {
 		return new EmptyRecordIterator();
 	}
 
-	/**
-	 * @see ghidra.program.database.code.CommentHistoryAdapter#updateRecord(db.Record)
-	 */
 	@Override
 	void updateRecord(Record rec) throws IOException {
 		throw new UnsupportedOperationException();
 	}
 
-	/**
-	 * @see ghidra.program.database.code.CommentHistoryAdapter#deleteRecords(ghidra.program.model.address.Address, ghidra.program.model.address.Address)
-	 */
 	@Override
 	boolean deleteRecords(Address start, Address end) throws IOException {
 		throw new UnsupportedOperationException();
 	}
 
-	/**
-	 * @see ghidra.program.database.code.CommentHistoryAdapter#getRecordCount()
-	 */
 	@Override
 	int getRecordCount() {
 		return 0;
 	}
-
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/CommentHistoryAdapterV0.java

@@ -19,13 +19,12 @@ import java.io.IOException;
 
 import db.*;
 import ghidra.program.database.map.*;
-import ghidra.program.database.util.DatabaseVersionException;
 import ghidra.program.model.address.Address;
 import ghidra.util.SystemUtilities;
 import ghidra.util.exception.VersionException;
 
 /**
- * Adapter for Version 0 of the Comment History table.
+ * Adapter for Version 0 of the Comment History table
  */
 class CommentHistoryAdapterV0 extends CommentHistoryAdapter {
 
@@ -36,16 +35,17 @@ class CommentHistoryAdapterV0 extends CommentHistoryAdapter {
 	/**
 	 * Construct a new Version 0 comment history adapter.
 	 * @param handle database handle
-	 * @throws DatabaseVersionException if the table was not found
+	 * @param addrMap the address map used to generate keys for addresses
+	 * @param create true if to create a new table; false to load an existing table
+	 * @throws VersionException if the table was not found
 	 * @throws IOException if an error occurred while accessing the database
 	 */
 	CommentHistoryAdapterV0(DBHandle handle, AddressMap addrMap, boolean create)
 			throws VersionException, IOException {
 		this.addrMap = addrMap;
 		if (create) {
-			table =
+			table = handle.createTable(COMMENT_HISTORY_TABLE_NAME, COMMENT_HISTORY_SCHEMA,
-				handle.createTable(COMMENT_HISTORY_TABLE_NAME, COMMENT_HISTORY_SCHEMA,
+				new int[] { HISTORY_ADDRESS_COL });
-					new int[] { HISTORY_ADDRESS_COL });
 		}
 		else {
 			table = handle.getTable(COMMENT_HISTORY_TABLE_NAME);
@@ -59,11 +59,8 @@ class CommentHistoryAdapterV0 extends CommentHistoryAdapter {
 		userName = SystemUtilities.getUserName();
 	}
 
-	/**
-	 * @see ghidra.program.database.code.CommentHistoryAdapter#createRecord(long, byte, int, int, java.lang.String)
-	 */
 	@Override
-	void createRecord(long addr, byte commentType, int pos1, int pos2, String data)
+	void createRecord(long addr, byte commentType, int pos1, int pos2, String data, long date)
 			throws IOException {
 
 		Record rec = table.getSchema().createRecord(table.getKey());
@@ -73,47 +70,32 @@ class CommentHistoryAdapterV0 extends CommentHistoryAdapter {
 		rec.setIntValue(HISTORY_POS2_COL, pos2);
 		rec.setString(HISTORY_STRING_COL, data);
 		rec.setString(HISTORY_USER_COL, userName);
-		rec.setLongValue(HISTORY_DATE_COL, System.currentTimeMillis() ); //new Date().getTime());
+		rec.setLongValue(HISTORY_DATE_COL, date);
 
 		table.putRecord(rec);
 	}
 
-	/**
-	 * @see ghidra.program.database.code.CommentHistoryAdapter#getRecordsByAddress(long)
-	 */
 	@Override
 	RecordIterator getRecordsByAddress(Address address) throws IOException {
 		LongField field = new LongField(addrMap.getKey(address, false));
 		return table.indexIterator(HISTORY_ADDRESS_COL, field, field, true);
 	}
 
-	/**
-	 * @see ghidra.program.database.code.CommentHistoryAdapter#getAllRecords()
-	 */
 	@Override
 	RecordIterator getAllRecords() throws IOException {
 		return new AddressKeyRecordIterator(table, addrMap);
 	}
 
-	/**
-	 * @see ghidra.program.database.code.CommentHistoryAdapter#updateRecord(db.Record)
-	 */
 	@Override
 	void updateRecord(Record rec) throws IOException {
 		table.putRecord(rec);
 	}
 
-	/**
-	 * @see ghidra.program.database.code.CommentHistoryAdapter#deleteRecords(ghidra.program.model.address.Address, ghidra.program.model.address.Address)
-	 */
 	@Override
 	boolean deleteRecords(Address start, Address end) throws IOException {
 		return AddressRecordDeleter.deleteRecords(table, addrMap, start, end);
 	}
 
-	/**
-	 * @see ghidra.program.database.code.CommentHistoryAdapter#getRecordCount()
-	 */
 	@Override
 	int getRecordCount() {
 		return table.getRecordCount();

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/StringDiff.java

@@ -0,0 +1,132 @@
+/* ###
+ * 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.code;
+
+import com.google.common.base.Objects;
+
+/**
+ * Container object that holds a start and end position within a string. A list of StringDiffs 
+ * is used to keep track of changes made to a string.
+ */
+public class StringDiff {
+
+	/**
+	 * Start position of the string used when text is inserted or replaced
+	 */
+	int start;
+
+	/**
+	 * End position of the string used when part of the string is replaced
+	 */
+	int end;
+
+	/**
+	 * String being inserted.  This can be an insert or a complete replace (the positions will both
+	 * be -1 in a replace; pos1 will be non-negative during an insert).
+	 */
+	public String text;
+
+	/**
+	 * Construct a new StringDiff with pos1 and pos2 are initialized to -1
+	 * 
+	 * @param newText string 
+	 * @return the new diff 
+	 */
+	public static StringDiff allTextReplaced(String newText) {
+		return new StringDiff(-1, -1, newText);
+	}
+
+	/**
+	 * Construct a new StringDiff that indicates text was deleted from pos1 to pos2
+	 * 
+	 * @param start position 1 for the diff
+	 * @param end position 2 for the diff
+	 * @return the new diff
+	 */
+	public static StringDiff textDeleted(int start, int end) {
+		return new StringDiff(start, end, null);
+	}
+
+	/**
+	 * Construct a new StringDiff that indicates that insertData was inserted at the given position
+	 * 
+	 * @param newText inserted string
+	 * @param start position where the text was inserted
+	 * @return the new diff
+	 */
+	public static StringDiff textInserted(String newText, int start) {
+		return new StringDiff(start, -1, newText);
+	}
+
+	// for restoring from saved record
+	public static StringDiff restore(String text, int start, int end) {
+		return new StringDiff(start, end, text);
+	}
+
+	private StringDiff(int start, int end, String text) {
+		this.start = start;
+		this.end = end;
+		this.text = text;
+	}
+
+	@Override
+	public int hashCode() {
+		final int prime = 31;
+		int result = 1;
+		result = prime * result + ((text == null) ? 0 : text.hashCode());
+		result = prime * result + start;
+		result = prime * result + end;
+		return result;
+	}
+
+	@Override
+	public boolean equals(Object obj) {
+		if (this == obj) {
+			return true;
+		}
+		if (obj == null) {
+			return false;
+		}
+		if (getClass() != obj.getClass()) {
+			return false;
+		}
+
+		StringDiff other = (StringDiff) obj;
+		if (!Objects.equal(text, other.text)) {
+			return false;
+		}
+		if (start != other.start) {
+			return false;
+		}
+		if (end != other.end) {
+			return false;
+		}
+		return true;
+	}
+
+	@Override
+	public String toString() {
+		if (text != null) {
+			if (start >= 0) {
+				return "StringDiff: inserted <" + text + "> at " + start;
+			}
+
+			return "StringDiff: replace with <" + text + ">";
+		}
+		return "StringDiff: deleted text from " + start + " to " + end;
+	}
+
+}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/StringDiffer.java

@@ -0,0 +1,261 @@
+/* ###
+ * 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.code;
+
+import java.util.LinkedList;
+import java.util.List;
+
+class StringDiffer {
+
+	/**
+	 * Returns the list of StringDiff objects that if applied to s1 would result in s2;  The
+	 * given text will look only for whole lines using '\n'.
+	 *
+	 * @param s1 the original string
+	 * @param s2 the result string
+	 *        this value, then a completely different string will be returned
+	 * @return an array of StringDiff objects that change s1 into s2;
+	 */
+	static StringDiff[] getLineDiffs(String s1, String s2) {
+
+		/**
+		 * Minimum size used to determine whether a new StringDiff object will be
+		 * created just using a string (no positions)
+		 * in the <code>getDiffs(String, String)</code> method.
+		 * @see #getLineDiffs(String, String)
+		 */
+		int MINIMUM_DIFF_SIZE = 100;
+		return StringDiffer.getLineDiffs(s1, s2, MINIMUM_DIFF_SIZE);
+	}
+
+	/**
+	 * Returns the list of StringDiff objects that if applied to s1 would result in s2;  The
+	 * given text will look only for whole lines using '\n'.
+	 *
+	 * @param s1 the original string
+	 * @param s2 the result string
+	 * @param minimumDiffSize the minimum length of s2 required for a diff; if s2 is less than
+	 *        this value, then a completely different string will be returned
+	 * @return an array of StringDiff objects that change s1 into s2;
+	 */
+	static StringDiff[] getLineDiffs(String s1, String s2, int minimumDiffSize) {
+		if (s2.length() < minimumDiffSize) {
+			return new StringDiff[] { StringDiff.allTextReplaced(s2) };
+		}
+
+		List<StringDiff> results = new LinkedList<>();
+		int cursor1 = 0;
+		int cursor2 = 0;
+		int len1 = s1.length();
+		int len2 = s2.length();
+
+		/*
+		 	-look at each line in 'line' chunks using '\n'
+		 */
+
+		// walk each string until the end...
+		while (cursor1 < len1 || cursor2 < len2) {
+			String line1 = getLine(s1, cursor1);
+			String line2 = getLine(s2, cursor2);
+			if (line1.equals(line2)) {
+				cursor1 += line1.length();
+				cursor2 += line2.length();
+				continue;
+			}
+
+			// look for line1 in s2...
+			int line1PosInOther = findLine(s2, cursor2, line1);
+			int mark = cursor1;
+			while (line1PosInOther < 0) {
+
+				// line1 is not in s2; scan for the next line
+				cursor1 += line1.length();
+				line1 = getLine(s1, cursor1);
+				line1PosInOther = findLine(s2, cursor2, line1);
+			}
+			if (cursor1 > mark) {
+				// the original line1 was not in s2; add all that was different up to current cursor1
+				results.add(StringDiff.textDeleted(mark, cursor1));
+			}
+
+			// now look for line2 in s1
+			int line2PosInOther = findLine(s1, cursor1, line2);
+			mark = cursor2;
+			while (line2PosInOther < 0) {
+
+				// line2 is not in s1; scan for the next line
+				cursor2 += line2.length();
+				line2 = getLine(s2, cursor2);
+				line2PosInOther = findLine(s1, cursor1, line2);
+			}
+			if (cursor2 > mark) {
+				// the original line2 was not in s1; add all that was different up to current cursor2
+				results.add(StringDiff.textInserted(s2.substring(mark, cursor2), cursor1));
+				continue;
+			}
+
+			// move both searches forward
+			int delta1 = line2PosInOther - cursor1;
+			int delta2 = line1PosInOther - cursor2;
+			if (delta1 > delta2) {
+
+				// this can happen when two lines have been rearranged *and* the line length
+				// of the moved line is *longer* than the new line at the replaced position
+				results.add(
+					StringDiff.textInserted(s2.substring(cursor2, line1PosInOther), cursor1));
+				cursor2 = line1PosInOther;
+			}
+			else if (delta2 > delta1) {
+
+				// this can happen when two lines have been rearranged *and* the line length
+				// of the moved line is *shorter* than the new line at the replaced position
+				results.add(StringDiff.textDeleted(cursor1, line2PosInOther));
+				cursor1 = line2PosInOther;
+			}
+			else { // delta1 == delta2
+
+				if (cursor1 != line2PosInOther) {
+					results.add(StringDiff.textDeleted(cursor1, line2PosInOther));
+					cursor1 = line2PosInOther;
+				}
+
+				if (cursor2 != line1PosInOther) {
+					results.add(
+						StringDiff.textInserted(s2.substring(cursor2, line1PosInOther), cursor1));
+					cursor2 = line1PosInOther;
+				}
+			}
+		}
+		return results.toArray(new StringDiff[results.size()]);
+	}
+
+	/**
+	 * Finds a position in s that contains the string line.  The matching string in
+	 * s must be a "complete" line, in other words if pos > 0 then s.charAt(index-1) must be
+	 * a newLine character and s.charAt(index+line.length()) must be a newLine or the end of
+	 * the string.
+	 * @param s the string to scan
+	 * @param pos the position to begin the scan.
+	 * @param line the line to scan for
+	 * @return the position in s containing the line string.
+	 */
+	static int findLine(String s, int pos, String line) {
+
+		if (line.length() == 0) {
+			// this is used as a marker: -1 means not found; non-negative number signals to keep going
+			return pos;  // TODO this is odd; why is this a match??
+		}
+
+		int n = s.length();
+		while (pos < n) {
+			int index = s.indexOf(line, pos);
+			if (index < 0) {
+				return index;
+			}
+
+			if (index > 0 && s.charAt(index - 1) != '\n') {
+				pos = index + line.length(); // line matched, but not a newline in 's'
+				continue;
+			}
+
+			//
+			// Have a match with at start/0 or have a preceding newline
+			//
+
+			if (line.endsWith("\n")) {
+				return index; // the match ends with a newline; found line
+			}
+
+			// no newline for the current match in 's'
+			if (index + line.length() == n) {
+				return index; // at the end exactly; found line
+			}
+
+			// no newline; not at end; keep going
+			pos = index + line.length();
+		}
+
+		return -1;
+	}
+
+	/**
+	 * Returns a substring of s beginning at start and ending at either the end of the string or
+	 * the first newLine at or after start
+	 * 
+	 * @param s the string to scan
+	 * @param start the starting position for the scan
+	 * @return a string that represents a line within s
+	 */
+	private static String getLine(String s, int start) {
+		int n = s.length();
+		if (start >= n) {
+			return "";
+		}
+		int pos = start;
+		while (pos < n && s.charAt(pos) != '\n') {
+			pos++;
+		}
+
+		if (pos < n) {
+			pos++; // not at the end; found newline; include the newline
+		}
+		return s.substring(start, pos);
+	}
+
+	/**
+	 * Applies the array of StringObjects to the string s to produce a new string. Warning - the
+	 * diff objects cannot be applied to an arbitrary string, the Strings must be the original
+	 * String used to compute the diffs.
+	 * @param s the original string
+	 * @param diffs the array of StringDiff object to apply
+	 * @return a new String resulting from applying the diffs to s.
+	 */
+	static String applyDiffs(String s, List<StringDiff> diffs) {
+
+		if (diffs.isEmpty()) {
+			return s;
+		}
+
+		if (diffs.get(0).start < 0) {
+			// all replaced or all deleted
+			String data = diffs.get(0).text;
+			return data == null ? "" : data;
+		}
+
+		int pos = 0;
+		StringBuilder buf = new StringBuilder(s.length());
+		for (StringDiff element : diffs) {
+			if (element.start > pos) {
+				buf.append(s.substring(pos, element.start));
+				pos = element.start;
+			}
+
+			String data = element.text;
+			if (data != null) {
+				buf.append(data);
+			}
+			else {
+				// null data is a delete; move to the end of the delete
+				pos = element.end;
+			}
+		}
+
+		if (pos < s.length()) {
+			buf.append(s.substring(pos));
+		}
+		return buf.toString();
+	}
+}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/CommentHistory.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.
@@ -18,13 +17,15 @@ package ghidra.program.model.listing;
 
 import java.util.Date;
 
+import org.apache.commons.lang3.StringUtils;
+
 import ghidra.program.model.address.Address;
 
 /**
  * Container class for information about changes to a comment.
  */
 public class CommentHistory {
-	
+
 	private Address addr;
 	private int commentType;
 	private Date modificationDate;
@@ -39,8 +40,8 @@ public class CommentHistory {
 	 * @param comments the list of comments.
 	 * @param modificationDate the date the comment was changed.
 	 */
-	public CommentHistory(Address addr, int commentType, String userName, 
+	public CommentHistory(Address addr, int commentType, String userName, String comments,
-							String comments, Date modificationDate) {
+			Date modificationDate) {
 		this.addr = addr;
 		this.commentType = commentType;
 		this.userName = userName;
@@ -49,35 +50,55 @@ public class CommentHistory {
 	}
 
 	/**
-	 * Get address for this label history object.
+	 * Get address for this label history object
+	 * @return address for this label history object.
 	 */
 	public Address getAddress() {
 		return addr;
 	}
 
 	/**
-	 * Get the user that made the change.
+	 * Get the user that made the change
+	 * @return the user that made the change
 	 */
 	public String getUserName() {
 		return userName;
 	}
+
 	/**
-	 * Get the comments for this history object.
+	 * Get the comments for this history object
+	 * @return the comments for this history object
 	 */
 	public String getComments() {
 		return comments;
 	}
+
 	/**
-	 * Get the comment type.
+	 * Get the comment type
+	 * @return the comment type
 	 */
 	public int getCommentType() {
 		return commentType;
 	}
+
 	/**
 	 * Get the modification date
+	 * @return the modification date
 	 */
 	public Date getModificationDate() {
 		return modificationDate;
 	}
-		
+
+	@Override
+	public String toString() {
+
+		//@formatter:off
+		return "{\n" +
+			"\tuser: " + userName + ",\n" +
+			"\tdate: " + modificationDate + ",\n" + 
+			"\taddress: " + addr + ",\n" +
+			"\tcomment: " + StringUtils.abbreviate(comments, 10) + "\n" +
+		"}";		
+		//@formatter:on
+	}
 }

Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/database/code/SringDiffTest.java

@@ -0,0 +1,254 @@
+/* ###
+ * 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.code;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.Arrays;
+
+import org.apache.commons.lang3.StringUtils;
+import org.junit.Test;
+
+public class SringDiffTest {
+
+	/*
+	 	A line match is if the given line to match is contained in the source string and:
+	 	
+	 		1) a) matches in the source string with a '\n' char at the index before the match OR
+	 		   b) is at the beginning *and* the match contains a newline
+	 		2) is at the exact end of the source string
+	 		
+	 		*The empty string matches at the current position
+	 	
+	 	Source String:	 "abcd\nefghi\n"
+	 	Line to Match:	 	
+	 */
+
+	@Test
+	public void testFindLine_FromStart_EmptyLine() {
+
+		String source = "this is a really\nlone line with\n newlines";
+		String line = "";
+		int result = StringDiffer.findLine(source, 0, line);
+		assertEquals(0, result);
+	}
+
+	@Test
+	public void testFindLine_FromStart_NoMatch() {
+
+		String source = "this is a really\nlone line with\n newlines";
+		String line = "coconuts";
+		int result = StringDiffer.findLine(source, 0, line);
+		assertEquals(-1, result);
+	}
+
+	@Test
+	public void testFindLine_FromMiddle_NoMatch() {
+
+		String source = "this is a really\nlone line with\n newlines";
+		String line = "coconuts";
+		int result = StringDiffer.findLine(source, 15, line);
+		assertEquals(-1, result);
+	}
+
+	@Test
+	public void testFindLine_FromEnd_NoMatch() {
+
+		String source = "this is a really\nlone line with\n newlines";
+		String line = "coconuts";
+		int result = StringDiffer.findLine(source, source.length(), line);
+		assertEquals(-1, result);
+	}
+
+	@Test
+	public void testFindLine_FromStart_MatchWithNewline_AtStart() {
+
+		String source = "this is a really\nlone line with\n newlines";
+		String line = "this is a really\n";
+		int result = StringDiffer.findLine(source, 0, line);
+		assertEquals(0, result);
+	}
+
+	@Test
+	public void testFindLine_FromStart_MatchWithNewline_AtMiddle() {
+
+		String source = "this is a really\nlone line with\n newlines";
+		String line = "lone line with\n";
+		int result = StringDiffer.findLine(source, 0, line);
+		assertEquals(17, result);
+	}
+
+	@Test
+	public void testFindLine_FromStart_MatchWithNewline_AtEnd_FailWithoutPrecedingNewline() {
+
+		String source = "this is a really\nlone line with\n newlines\n";
+		String line = "lines\n";
+		int result = StringDiffer.findLine(source, 0, line);
+		assertEquals(-1, result);
+	}
+
+	@Test
+	public void testFindLine_FromStart_MatchWithNewline_AtEnd_PassWithPrecedingNewline() {
+
+		String source = "this is a really\nlone line with\n new\nlines\n";
+		String line = "lines\n";
+		int result = StringDiffer.findLine(source, 0, line);
+		assertEquals(37, result);
+	}
+
+	@Test
+	public void testFindLine_FromStart_MatchWithoutNewline_AtStart() {
+
+		String source = "this is a really\nlone line with\n newlines";
+		String line = "this is a really";
+		int result = StringDiffer.findLine(source, 0, line);
+		assertEquals(-1, result); // match at start must contain a newline
+	}
+
+	@Test
+	public void testGetDiffLines_Insert_AtFront() {
+
+		String[] a1 = new String[] { "This", "is", "four", "friends" };
+		String[] a2 = new String[] { "Inserted", "This", "is", "four", "friends" };
+		String v1 = StringUtils.join(a1, '\n');
+		String v2 = StringUtils.join(a2, '\n');
+
+		StringDiff[] diffs = StringDiffer.getLineDiffs(v1, v2, 1);
+		String restoredV2 = StringDiffer.applyDiffs(v1, Arrays.asList(diffs));
+		assertEquals(v2, restoredV2);
+	}
+
+	@Test
+	public void testGetDiffLines_Insert_AtEnd() {
+
+		String[] a1 = new String[] { "This", "is", "four", "friends" };
+		String[] a2 = new String[] { "This", "is", "four", "friends", "Inserted" };
+		String v1 = StringUtils.join(a1, '\n');
+		String v2 = StringUtils.join(a2, '\n');
+
+		StringDiff[] diffs = StringDiffer.getLineDiffs(v1, v2, 1);
+		String restoredV2 = StringDiffer.applyDiffs(v1, Arrays.asList(diffs));
+		assertEquals(v2, restoredV2);
+	}
+
+	@Test
+	public void testGetDiffLines_Insert_AtMiddle() {
+
+		String[] a1 = new String[] { "This", "is", "four", "friends" };
+		String[] a2 = new String[] { "This", "is", "Inserted", "four", "friends" };
+		String v1 = StringUtils.join(a1, '\n');
+		String v2 = StringUtils.join(a2, '\n');
+
+		StringDiff[] diffs = StringDiffer.getLineDiffs(v1, v2, 1);
+		String restoredV2 = StringDiffer.applyDiffs(v1, Arrays.asList(diffs));
+		assertEquals(v2, restoredV2);
+	}
+
+	@Test
+	public void testGetDiffLines_Delete_AtStart() {
+
+		String[] a1 = new String[] { "DELETED", "This", "is", "the", "best" };
+		String[] a2 = new String[] { "This", "is", "the", "best" };
+		String v1 = StringUtils.join(a1, '\n');
+		String v2 = StringUtils.join(a2, '\n');
+
+		StringDiff[] diffs = StringDiffer.getLineDiffs(v1, v2, 1);
+		String restoredV2 = StringDiffer.applyDiffs(v1, Arrays.asList(diffs));
+		assertEquals(v2, restoredV2);
+	}
+
+	@Test
+	public void testGetDiffLines_Delete_AtEnd() {
+
+		String[] a1 = new String[] { "This", "is", "the", "best", "DELETED" };
+		String[] a2 = new String[] { "This", "is", "the", "best" };
+		String v1 = StringUtils.join(a1, '\n');
+		String v2 = StringUtils.join(a2, '\n');
+
+		StringDiff[] diffs = StringDiffer.getLineDiffs(v1, v2, 1);
+		String restoredV2 = StringDiffer.applyDiffs(v1, Arrays.asList(diffs));
+		assertEquals(v2, restoredV2);
+	}
+
+	@Test
+	public void testGetDiffLines_Delete_AtMiddle() {
+
+		String[] a1 = new String[] { "This", "is", "DELETED", "the", "best" };
+		String[] a2 = new String[] { "This", "is", "the", "best" };
+		String v1 = StringUtils.join(a1, '\n');
+		String v2 = StringUtils.join(a2, '\n');
+
+		StringDiff[] diffs = StringDiffer.getLineDiffs(v1, v2, 1);
+		String restoredV2 = StringDiffer.applyDiffs(v1, Arrays.asList(diffs));
+		assertEquals(v2, restoredV2);
+	}
+
+	@Test
+	public void testGetDiffLines_Delete_MultipleDeletes() {
+
+		String[] a1 = new String[] { "This", "is", "DELETED", "the", "best", "DELETED" };
+		String[] a2 = new String[] { "This", "is", "the", "best" };
+		String v1 = StringUtils.join(a1, '\n');
+		String v2 = StringUtils.join(a2, '\n');
+
+		StringDiff[] diffs = StringDiffer.getLineDiffs(v1, v2, 1);
+		String restoredV2 = StringDiffer.applyDiffs(v1, Arrays.asList(diffs));
+		assertEquals(v2, restoredV2);
+	}
+
+	@Test
+	public void testGetDiffLines_Rearrange_EqualLineLength() {
+
+		// note: this text used to cause an infinite loop bug that tripped when two words were
+		//       swapped at some point in the two strings *and* had the same length
+
+		String[] a1 = new String[] { "This", "is", "best", "four", "friends" };
+		String[] a2 = new String[] { "This", "is", "four", "best", "friends" };
+		String v1 = StringUtils.join(a1, '\n');
+		String v2 = StringUtils.join(a2, '\n');
+
+		StringDiff[] diffs = StringDiffer.getLineDiffs(v1, v2, 1);
+		String restoredV2 = StringDiffer.applyDiffs(v1, Arrays.asList(diffs));
+		assertEquals(v2, restoredV2);
+	}
+
+	@Test
+	public void testGetDiffLines_Rearrange_DifferentLineLength_LongerThanNewSpot() {
+
+		String[] a1 = new String[] { "This", "is", "besties", "four", "friends" };
+		String[] a2 = new String[] { "This", "is", "four", "besties", "friends" };
+		String v1 = StringUtils.join(a1, '\n');
+		String v2 = StringUtils.join(a2, '\n');
+
+		StringDiff[] diffs = StringDiffer.getLineDiffs(v1, v2, 1);
+		String restoredV2 = StringDiffer.applyDiffs(v1, Arrays.asList(diffs));
+		assertEquals(v2, restoredV2);
+	}
+
+	@Test
+	public void testGetDiffLines_Rearrange_DifferentLineLength_ShorterThanNewSpot() {
+
+		String[] a1 = new String[] { "This", "is", "be", "four", "friends" };
+		String[] a2 = new String[] { "This", "is", "four", "be", "friends" };
+		String v1 = StringUtils.join(a1, '\n');
+		String v2 = StringUtils.join(a2, '\n');
+
+		StringDiff[] diffs = StringDiffer.getLineDiffs(v1, v2, 1);
+		String restoredV2 = StringDiffer.applyDiffs(v1, Arrays.asList(diffs));
+		assertEquals(v2, restoredV2);
+	}
+
+}