From 6d2df2281f4a1c0a022a873bb6a1b871e99db75d Mon Sep 17 00:00:00 2001 From: ghidra1 Date: Mon, 26 Aug 2019 16:44:24 -0400 Subject: [PATCH] GT-2922 cleanup per review comments --- .../merge/datatypes/DataTypeMergeManager.java | 5 +--- .../database/data/FunctionDefinitionDB.java | 16 ++++++------- .../database/data/ParameterDefinitionDB.java | 2 +- .../model/data/FunctionDefinition.java | 2 +- .../data/FunctionDefinitionDataType.java | 4 ++-- .../model/data/ParameterDefinition.java | 16 ++++++------- .../model/data/ParameterDefinitionImpl.java | 23 ++++++++++++++----- 7 files changed, 38 insertions(+), 30 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/merge/datatypes/DataTypeMergeManager.java b/Ghidra/Features/Base/src/main/java/ghidra/app/merge/datatypes/DataTypeMergeManager.java index c79b631489..01caf5eff8 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/merge/datatypes/DataTypeMergeManager.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/merge/datatypes/DataTypeMergeManager.java @@ -2839,12 +2839,9 @@ public class DataTypeMergeManager implements MergeResolver { try { SwingUtilities.invokeAndWait(() -> Msg.showInfo(getClass(), null, title, msg)); } - catch (InterruptedException e) { + catch (InterruptedException | InvocationTargetException e) { // ignore } - catch (InvocationTargetException e) { - e.printStackTrace(); - } } @Override diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/FunctionDefinitionDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/FunctionDefinitionDB.java index e3825ad114..ca1a143ef0 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/FunctionDefinitionDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/FunctionDefinitionDB.java @@ -41,7 +41,7 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { super(dataMgr, cache, record); this.funDefAdapter = adapter; this.paramAdapter = paramAdapter; - getParameters(); + loadParameters(); } @Override @@ -54,7 +54,7 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { return record.getLongValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_CAT_ID_COL); } - private void getParameters() { + private void loadParameters() { parameters = new ArrayList<>(); try { long[] ids = paramAdapter.getParameterIdsInFunctionDef(key); @@ -75,7 +75,7 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { Record rec = funDefAdapter.getRecord(key); if (rec != null) { record = rec; - getParameters(); + loadParameters(); return super.refresh(); } } @@ -250,13 +250,13 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { parameters.clear(); for (int i = 0; i < args.length; i++) { DataType type = - ParameterDefinitionImpl.checkDataType(args[i].getDataType(), dataMgr, false); + ParameterDefinitionImpl.validateDataType(args[i].getDataType(), dataMgr, false); DataType resolvedDt = resolve(type); paramAdapter.createRecord(dataMgr.getID(resolvedDt), key, i, args[i].getName(), args[i].getComment(), args[i].getLength()); resolvedDt.addParent(this); } - getParameters(); + loadParameters(); funDefAdapter.updateRecord(record, true); // update last change time dataMgr.dataTypeChanged(this); } @@ -270,7 +270,7 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { @Override public void setReturnType(DataType type) { - type = ParameterDefinitionImpl.checkDataType(type, dataMgr, true); + type = ParameterDefinitionImpl.validateDataType(type, dataMgr, true); lock.acquire(); try { checkDeleted(); @@ -396,7 +396,7 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { try { checkDeleted(); if (newDt == this) { - // TODO: document why this is neccessary + // avoid creating circular dependency newDt = DataType.DEFAULT; } DataType retType = getReturnType(); @@ -454,7 +454,7 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { rdt.addParent(this); paramAdapter.createRecord(dataMgr.getID(rdt), key, ordinal, name, comment, dt.getLength()); - getParameters(); + loadParameters(); } catch (IOException e) { dataMgr.dbError(e); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParameterDefinitionDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParameterDefinitionDB.java index 530c719da9..267f92d9fa 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParameterDefinitionDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParameterDefinitionDB.java @@ -64,7 +64,7 @@ final class ParameterDefinitionDB implements ParameterDefinition { @Override public void setDataType(DataType type) { - type = ParameterDefinitionImpl.checkDataType(type, dataMgr, false); + type = ParameterDefinitionImpl.validateDataType(type, dataMgr, false); getDataType().removeParent(parent); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinition.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinition.java index 5a5b05795a..c1c90571c2 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinition.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinition.java @@ -34,7 +34,7 @@ public interface FunctionDefinition extends DataType, FunctionSignature { * @param type the return datatype to be set. * @throws IllegalArgumentException if data type is not a fixed length type */ - public void setReturnType(DataType type); + public void setReturnType(DataType type) throws IllegalArgumentException; /** * Set the function comment diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinitionDataType.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinitionDataType.java index 73f88b7968..685ba74f9b 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinitionDataType.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/FunctionDefinitionDataType.java @@ -158,7 +158,7 @@ public class FunctionDefinitionDataType extends GenericDataType implements Funct @Override public void setReturnType(DataType type) { - returnType = ParameterDefinitionImpl.checkDataType(type, dataMgr, true); + returnType = ParameterDefinitionImpl.validateDataType(type, dataMgr, true); } @Override @@ -337,7 +337,7 @@ public class FunctionDefinitionDataType extends GenericDataType implements Funct public void dataTypeReplaced(DataType oldDt, DataType newDt) { if (newDt == this) { - // TODO: document why this is neccessary + // avoid creating circular dependency newDt = DataType.DEFAULT; } DataType retType = getReturnType(); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinition.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinition.java index e99cda139f..da39a60a99 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinition.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinition.java @@ -39,11 +39,11 @@ public interface ParameterDefinition extends Comparable { public DataType getDataType(); /** - * Set the Data Type of this variable. The given dataType must have a fixed length. - * @param type the data type - * @throws IllegalArgumentException if data type is not a fixed length type + * Set the Data Type of this variable. + * @param type dataType the fixed-length datatype of the parameter + * @throws IllegalArgumentException if invalid parameter datatype specified */ - public void setDataType(DataType type); + public void setDataType(DataType type) throws IllegalArgumentException; /** * Get the Name of this variable. @@ -79,9 +79,9 @@ public interface ParameterDefinition extends Comparable { public void setComment(String comment); /** - * Determine if a variable corresponds to a parameter which is equivelent to + * Determine if a variable corresponds to a parameter which is equivalent to * this parameter definition by both ordinal and datatype. Name is not considered - * relavent. + * relevant. * @param variable variable to be compared with this parameter definition. * @return true if the specified variable represents the same parameter by ordinal * and dataType. False will always be returned if specified variable is @@ -90,8 +90,8 @@ public interface ParameterDefinition extends Comparable { public boolean isEquivalent(Variable variable); /** - * Determine if parm is equivelent to this parameter definition by both ordinal - * and datatype. Name is not considered relavent. + * Determine if parm is equivalent to this parameter definition by both ordinal + * and datatype. Name is not considered relevant. * @param parm parameter definition to be compared with this parameter definition. * @return true if the specified parameter definition represents the same parameter * by ordinal and dataType. diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinitionImpl.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinitionImpl.java index ce1497bce6..ceef49e2a5 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinitionImpl.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ParameterDefinitionImpl.java @@ -31,8 +31,9 @@ public class ParameterDefinitionImpl implements ParameterDefinition { * Constructs a new ParameterImp with an unassigned ordinal. The ordinal will be * established by the function definition. * @param name the name of the parameter. - * @param dataType the datatype of the parameter + * @param dataType the fixed-length datatype of the parameter * @param comment the comment to store about this parameter. + * @throws IllegalArgumentException if invalid parameter datatype specified */ public ParameterDefinitionImpl(String name, DataType dataType, String comment) { this(name, dataType, comment, Parameter.UNASSIGNED_ORDINAL); @@ -41,19 +42,29 @@ public class ParameterDefinitionImpl implements ParameterDefinition { /** * Constructs a new ParameterImp * @param name the name of the parameter. - * @param dataType the datatype of the parameter + * @param dataType the fixed-length datatype of the parameter * @param comment the comment to store about this parameter. * @param ordinal the index of this parameter within the function signature. + * @throws IllegalArgumentException if invalid parameter datatype specified */ protected ParameterDefinitionImpl(String name, DataType dataType, String comment, int ordinal) { - this.dataType = checkDataType(dataType, null, false); + this.dataType = validateDataType(dataType, null, false); this.name = name; this.comment = comment; this.ordinal = ordinal; } - public static DataType checkDataType(DataType dataType, DataTypeManager dtMgr, boolean isReturn) - throws IllegalArgumentException { + /** + * Validate the specified datatype based upon its' use as a parameter or return type. + * Ensure that the datatype has been cloned to the specified datatype manager (dtMgr). + * @param dataType datatype to be validated + * @param dtMgr target datatype manager + * @param isReturn true if checking return datatype, false if parameter datatype. + * @return datatype suitable for use within the target {@link FunctionDefinition}. + * @throws IllegalArgumentException if invalid datatype specified + */ + public static DataType validateDataType(DataType dataType, DataTypeManager dtMgr, + boolean isReturn) throws IllegalArgumentException { String kind = isReturn ? "Return" : "Parameter"; if (dataType == null) { dataType = DataType.DEFAULT; @@ -131,7 +142,7 @@ public class ParameterDefinitionImpl implements ParameterDefinition { @Override public void setDataType(DataType type) { - this.dataType = checkDataType(type, dataType.getDataTypeManager(), false); + this.dataType = validateDataType(type, dataType.getDataTypeManager(), false); } @Override