From f58a9035f77bebf92d64faa2b74b2489ab483069 Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Mon, 30 Jan 2023 17:39:56 -0500 Subject: [PATCH 1/2] GP-3069 - Refactored the 'Create Structure from Selection' action to work around a focus issue seen by some users --- .../CreateInternalStructureAction.java | 32 +++++----- .../compositeeditor/StructureEditorModel.java | 58 +++++++++++++------ .../docking/action/ShowContextMenuAction.java | 9 ++- 3 files changed, 60 insertions(+), 39 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/CreateInternalStructureAction.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/CreateInternalStructureAction.java index f845f33353..9410befc0a 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/CreateInternalStructureAction.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/CreateInternalStructureAction.java @@ -21,10 +21,7 @@ import javax.swing.ImageIcon; import docking.ActionContext; import ghidra.util.Swing; -import ghidra.util.exception.CancelledException; import ghidra.util.exception.UsrException; -import ghidra.util.task.TaskLauncher; -import ghidra.util.task.TaskMonitor; import resources.ResourceManager; /** @@ -51,16 +48,20 @@ public class CreateInternalStructureAction extends CompositeEditorTableAction { public void actionPerformed(ActionContext context) { int[] selectedComponentRows = model.getSelectedComponentRows(); boolean hasComponentSelection = model.hasComponentSelection(); - boolean contiguousComponentSelection = model.isContiguousComponentSelection(); - if (hasComponentSelection && contiguousComponentSelection && - (selectedComponentRows.length > 0)) { + boolean hasContiguousSelection = model.isContiguousComponentSelection(); + if (selectedComponentRows.length == 0) { + return; + } - Arrays.sort(selectedComponentRows); - int numComponents = model.getNumComponents(); - int maxRow = selectedComponentRows[selectedComponentRows.length - 1]; - if (maxRow < numComponents) { - TaskLauncher.launchModal(getName(), this::doCreate); - } + if (!hasComponentSelection || !hasContiguousSelection) { + return; + } + + Arrays.sort(selectedComponentRows); + int numComponents = model.getNumComponents(); + int maxRow = selectedComponentRows[selectedComponentRows.length - 1]; + if (maxRow < numComponents) { + createStructure(); } requestTableFocus(); @@ -72,12 +73,9 @@ public class CreateInternalStructureAction extends CompositeEditorTableAction { }); } - private void doCreate(TaskMonitor monitor) { + private void createStructure() { try { - ((StructureEditorModel) model).createInternalStructure(monitor); - } - catch (CancelledException e) { - // user cancelled + ((StructureEditorModel) model).createInternalStructure(); } catch (UsrException e) { model.setStatus(e.getMessage(), true); diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/StructureEditorModel.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/StructureEditorModel.java index 18cff461bc..362a1813ed 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/StructureEditorModel.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/StructureEditorModel.java @@ -30,6 +30,7 @@ import ghidra.program.model.data.*; import ghidra.program.model.lang.InsufficientBytesException; import ghidra.util.Msg; import ghidra.util.exception.*; +import ghidra.util.task.TaskLauncher; import ghidra.util.task.TaskMonitor; class StructureEditorModel extends CompEditorModel { @@ -687,7 +688,7 @@ class StructureEditorModel extends CompEditorModel { } FieldRange currentRange = getSelectedRangeContaining(currentIndex); - // if the index isn't in the selection or is in a range of only + // if the index isn't in the selection or is in a range of only // one row then we want to handle it the same. boolean isOneComponent = (currentRange == null) || (currentRange.getStart().getIndex().intValue() + @@ -786,10 +787,10 @@ class StructureEditorModel extends CompEditorModel { /** * Gets the maximum number of bytes available for a data type that is added at the indicated - * index. This can vary based on whether or not it is in a selection. - *
In unlocked mode, the size is unrestricted when no selection or single row selection. + * index. This can vary based on whether or not it is in a selection. + *
In unlocked mode, the size is unrestricted when no selection or single row selection. * Multi-row selection always limits the size. - *
In locked mode, single row selection is limited to selected row plus undefined bytes + *
In locked mode, single row selection is limited to selected row plus undefined bytes * following it that can be absorbed. * * @param rowIndex index of the row in the editor's composite data type table. @@ -803,7 +804,7 @@ class StructureEditorModel extends CompEditorModel { } DataTypeComponent comp = getComponent(rowIndex); FieldRange currentRange = getSelectedRangeContaining(rowIndex); - // if the index isn't in the selection or is in a range of only + // if the index isn't in the selection or is in a range of only // one row then we want to handle it the same. boolean isOneComponent = (currentRange == null) || (currentRange.getStart().getIndex().intValue() + @@ -825,9 +826,9 @@ class StructureEditorModel extends CompEditorModel { } /** - * Gets the maximum number of bytes available for a new data type that + * Gets the maximum number of bytes available for a new data type that * will replace the current data type at the indicated index. - * If there isn't a component with the indicated index, the max length + * If there isn't a component with the indicated index, the max length * will be determined by the lock mode. * * @param currentIndex index of the component in the structure. @@ -1096,12 +1097,12 @@ class StructureEditorModel extends CompEditorModel { return !viewComposite.isPackingEnabled(); } - public void createInternalStructure(TaskMonitor monitor) - throws InvalidDataTypeException, UsrException { + void createInternalStructure() throws UsrException { if (selection.getNumRanges() != 1) { throw new UsrException("Can only create structure on a contiguous selection."); } + FieldRange fieldRange = selection.getFieldRange(0); int minRow = fieldRange.getStart().getIndex().intValue(); int maxRow = fieldRange.getEnd().getIndex().intValue(); @@ -1120,29 +1121,44 @@ class StructureEditorModel extends CompEditorModel { if (isEditingField()) { endFieldEditing(); } - DataTypeManager originalDTM = getOriginalDataTypeManager(); + + DataTypeManager originalDtm = getOriginalDataTypeManager(); String baseName = "struct"; CategoryPath originalCategoryPath = getOriginalCategoryPath(); String uniqueName = viewDTM.getUniqueName(originalCategoryPath, baseName); - DataType conflictingDt = originalDTM.getDataType(originalCategoryPath, uniqueName); + DataType conflictingDt = originalDtm.getDataType(originalCategoryPath, uniqueName); while (conflictingDt != null) { // pull the data type into the view data type manager with the conflicting name. viewDTM.resolve(conflictingDt, DataTypeConflictHandler.DEFAULT_HANDLER); // Try to get another unique name. uniqueName = viewDTM.getUniqueName(originalCategoryPath, baseName); - conflictingDt = originalDTM.getDataType(originalCategoryPath, uniqueName); + conflictingDt = originalDtm.getDataType(originalCategoryPath, uniqueName); } String specifiedName = - showNameDialog(uniqueName, originalCategoryPath, viewComposite.getName(), originalDTM); + showNameDialog(uniqueName, originalCategoryPath, viewComposite.getName(), originalDtm); if (specifiedName == null) { return; } - uniqueName = specifiedName; + + TaskLauncher.launchModal("Create Structure", monitor -> { + try { + doCreateInternalStructure(originalDtm, originalCategoryPath, specifiedName, + monitor); + } + catch (UsrException e) { + setStatus(e.getMessage(), true); + } + }); + } + + private void doCreateInternalStructure(DataTypeManager dtm, CategoryPath categoryPath, + String name, TaskMonitor monitor) + throws InvalidDataTypeException, UsrException { int length = 0; - final StructureDataType structureDataType = - new StructureDataType(originalCategoryPath, uniqueName, length, originalDTM); + StructureDataType structureDataType = + new StructureDataType(categoryPath, name, length, dtm); // adopt pack setting from current structure structureDataType.setPackingEnabled(isPackingEnabled()); @@ -1150,6 +1166,10 @@ class StructureEditorModel extends CompEditorModel { structureDataType.setExplicitPackingValue(getExplicitPackingValue()); } + FieldRange fieldRange = selection.getFieldRange(0); + int minRow = fieldRange.getStart().getIndex().intValue(); + int maxRow = fieldRange.getEnd().getIndex().intValue(); + // Get data type components to make into structure. DataTypeComponent firstDtc = null; DataTypeComponent lastDtc = null; @@ -1165,7 +1185,6 @@ class StructureEditorModel extends CompEditorModel { DataType dt = component.getDataType(); int compLength = component.getLength(); - length += compLength; if (!structureDataType.isPackingEnabled() && component.isBitFieldComponent()) { @@ -1182,6 +1201,7 @@ class StructureEditorModel extends CompEditorModel { lastDtc = component; } + DataType addedDataType = createDataTypeInOriginalDTM(structureDataType); if (viewComposite.isPackingEnabled()) { deleteSelectedComponents(); @@ -1207,7 +1227,7 @@ class StructureEditorModel extends CompEditorModel { } } - public String showNameDialog(final String defaultName, final CategoryPath catPath, + private String showNameDialog(final String defaultName, final CategoryPath catPath, final String parentStructureName, final DataTypeManager applyDTM) { InputDialogListener listener = dialog -> { String name = dialog.getValue(); @@ -1259,7 +1279,7 @@ class StructureEditorModel extends CompEditorModel { /** * Unpackage the selected component in the structure or array. This means replace the structure - * with the data types for its component parts. For an array replace the array with the data type + * with the data types for its component parts. For an array replace the array with the data type * for each array element. * If the component isn't a structure or union then returns false. * @param rowIndex the row diff --git a/Ghidra/Framework/Docking/src/main/java/docking/action/ShowContextMenuAction.java b/Ghidra/Framework/Docking/src/main/java/docking/action/ShowContextMenuAction.java index 469e521057..8f0fa24a56 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/action/ShowContextMenuAction.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/action/ShowContextMenuAction.java @@ -21,9 +21,10 @@ import javax.swing.KeyStroke; import docking.ActionContext; import docking.DockingWindowManager; +import ghidra.util.Swing; /** - * An action to trigger a context menu over the focus owner. This allows context menus to be + * An action to trigger a context menu over the focus owner. This allows context menus to be * triggered from the keyboard. */ public class ShowContextMenuAction extends DockingAction { @@ -42,10 +43,12 @@ public class ShowContextMenuAction extends DockingAction { return; } - // use the focused component to determine what should get the context menu + // use the focused component to determine what should get the context menu Component focusOwner = kfm.getFocusOwner(); if (focusOwner != null) { - DockingWindowManager.showContextMenu(focusOwner); + Swing.runLater(() -> { + DockingWindowManager.showContextMenu(focusOwner); + }); } } From 0ac639f504308dcc6f306c06e3c71259f501ccfc Mon Sep 17 00:00:00 2001 From: ghidra1 Date: Tue, 31 Jan 2023 22:05:13 +0000 Subject: [PATCH 2/2] GP-1453 corrected XML export of custom properties such as Analysis Times --- .../ghidra/app/util/xml/PropertiesXmlMgr.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/xml/PropertiesXmlMgr.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/xml/PropertiesXmlMgr.java index 9d2cb81b83..72d9097c10 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/xml/PropertiesXmlMgr.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/xml/PropertiesXmlMgr.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,15 @@ */ package ghidra.app.util.xml; +import java.awt.Color; +import java.awt.Font; +import java.io.File; +import java.util.*; + +import javax.swing.KeyStroke; + +import org.xml.sax.SAXParseException; + import ghidra.app.util.importer.MessageLog; import ghidra.framework.options.*; import ghidra.program.model.address.*; @@ -28,15 +36,6 @@ import ghidra.util.xml.*; import ghidra.xml.XmlElement; import ghidra.xml.XmlPullParser; -import java.awt.Color; -import java.awt.Font; -import java.io.File; -import java.util.*; - -import javax.swing.KeyStroke; - -import org.xml.sax.SAXParseException; - class PropertiesXmlMgr { private final static String PROPERTY_LIST_CATEGORY_DELIMITER = Options.DELIMITER_STRING; @@ -406,7 +405,7 @@ class PropertiesXmlMgr { case CUSTOM_TYPE: attrs.addAttribute("TYPE", "custom"); CustomOption custom = propList.getCustomOption(name, null); - xmlString = OptionType.KEYSTROKE_TYPE.convertObjectToString(custom); + xmlString = OptionType.CUSTOM_TYPE.convertObjectToString(custom); attrs.addAttribute("VALUE", XmlUtilities.escapeElementEntities(xmlString)); break; case BYTE_ARRAY_TYPE: