GP-3726 do not use aligned-length for non-packed structures

Ghidra/Features/Base/src/main/java/ghidra/app/cmd/data/CreateDataInStructureBackgroundCmd.java

@@ -30,6 +30,9 @@ import ghidra.util.task.TaskMonitor;
  */
 public class CreateDataInStructureBackgroundCmd extends BackgroundCommand {
 
+	// TODO: Not sure any of this will work for a packed structure which does not support
+	// offset-based component manipulation (see GP-3740)
+
 	private Address addr;
 	private int length;
 	private int[] startPath;
@@ -133,7 +136,7 @@ public class CreateDataInStructureBackgroundCmd extends BackgroundCommand {
 //			        MemBuffer memBuf = new ProgramStructureProviderContext(program,addr, 
 //    	    	    					struct, struct.getComponent(index).getOffset());
 					DataTypeInstance dti =
-						DataTypeInstance.getDataTypeInstance(newDataType, length, true);
+						DataTypeInstance.getDataTypeInstance(newDataType, length, false);
 					if (dti == null || dti.getLength() > length) {
 						break;
 					}

Ghidra/Features/Base/src/main/java/ghidra/app/cmd/data/CreateDataInStructureCmd.java

@@ -27,6 +27,9 @@ import ghidra.program.model.listing.Program;
  */
 public class CreateDataInStructureCmd implements Command {
 
+	// TODO: Not sure any of this will work for a packed structure which does not support
+	// offset-based component manipulation (see GP-3740)
+
 	private Address addr;
 	private int[] componentPath;
 	private DataType newDataType;
@@ -115,7 +118,7 @@ public class CreateDataInStructureCmd implements Command {
 //			        MemBuffer memBuf = new ProgramStructureProviderContext(program,addr, 
 //	    	        					struct, dataComp.getParentOffset());
 					DataTypeInstance dti =
-						DataTypeInstance.getDataTypeInstance(newDataType, -1, true);
+						DataTypeInstance.getDataTypeInstance(newDataType, -1, false);
 					struct.replace(index, dti.getDataType(), dti.getLength());
 				}
 			}

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/CompEditorModel.java

@@ -663,7 +663,8 @@ public abstract class CompEditorModel extends CompositeEditorModel {
 	 * @throws UsrException if add fails
 	 */
 	public DataTypeComponent replace(int rowIndex, DataType dt) throws UsrException {
-		DataTypeInstance dti = DataTypeHelper.getFixedLength(this, rowIndex, dt);
+		DataTypeInstance dti =
+			DataTypeHelper.getFixedLength(this, rowIndex, dt, usesAlignedLengthComponents());
 		if (dti == null) {
 			return null; // User cancelled from size dialog.
 		}
@@ -1254,8 +1255,7 @@ public abstract class CompEditorModel extends CompositeEditorModel {
 	}
 
 	@Override
-	public boolean setComponentName(int rowIndex, String name)
+	public boolean setComponentName(int rowIndex, String name) throws InvalidNameException {
-			throws InvalidNameException {
 
 		String oldName = getComponent(rowIndex).getFieldName();
 		if (Objects.equals(oldName, name)) {

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/CompositeEditorModel.java

@@ -755,7 +755,8 @@ public abstract class CompositeEditorModel extends CompositeViewerModel implemen
 			int currentIndex = getMinIndexSelected();
 			DataType dt = getNextCycleDataType(cycleGroup);
 			if (dt != null) {
-				DataTypeInstance dti = DataTypeHelper.getFixedLength(this, currentIndex, dt);
+				DataTypeInstance dti = DataTypeHelper.getFixedLength(this, currentIndex, dt,
+					usesAlignedLengthComponents());
 				if (dti == null) {
 					return;
 				}
@@ -1162,7 +1163,7 @@ public abstract class CompositeEditorModel extends CompositeViewerModel implemen
 			dtName = dt.getDisplayName();
 			if (dtString.equals(dtName)) {
 				return DataTypeInstance.getDataTypeInstance(element.getDataType(),
-					element.getLength(), true);
+					element.getLength(), usesAlignedLengthComponents());
 			}
 		}
 
@@ -1194,7 +1195,8 @@ public abstract class CompositeEditorModel extends CompositeViewerModel implemen
 		if (maxLength > 0 && newLength > maxLength) {
 			throw new UsrException(newDt.getDisplayName() + " doesn't fit.");
 		}
-		return DataTypeInstance.getDataTypeInstance(newDt, newLength, true);
+		return DataTypeInstance.getDataTypeInstance(newDt, newLength,
+			usesAlignedLengthComponents());
 	}
 
 	@SuppressWarnings("unused") // the exception is thrown by subclasses

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/CompositeViewerModel.java

@@ -572,7 +572,7 @@ abstract class CompositeViewerModel extends AbstractTableModel
 			DataType dt = dtc.getDataType();
 			int dtLen = dt.getLength();
 			return DataTypeInstance.getDataTypeInstance(dt, (dtLen > 0) ? dtLen : dtc.getLength(),
-				true);
+				usesAlignedLengthComponents());
 		}
 		else if (columnIndex == getNameColumn()) {
 			value = dtc.getFieldName();
@@ -1346,6 +1346,14 @@ abstract class CompositeViewerModel extends AbstractTableModel
 		return originalCompositeId;
 	}
 
+	/**
+	 * Determine if {@link DataType#getAlignedLength() aligned-length} components should be used. 
+	 * @return true if aligned-length components should be used, else false
+	 */
+	protected boolean usesAlignedLengthComponents() {
+		return viewComposite.isPackingEnabled();
+	}
+
 	@Override
 	public void programArchitectureChanged(DataTypeManager dataTypeManager) {
 		// don't care

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/DataTypeHelper.java

@@ -146,7 +146,8 @@ public class DataTypeHelper {
 			throw new InvalidDataTypeException(
 				"Data type " + dt.getDisplayName() + " has no size and is not allowed.");
 		}
-		return DataTypeInstance.getDataTypeInstance(dt, dtLen, true);
+		return DataTypeInstance.getDataTypeInstance(dt, dtLen,
+			provider.editorModel.usesAlignedLengthComponents());
 	}
 
 	public static int requestDtSize(CompositeEditorProvider provider, String dtName,
@@ -177,12 +178,14 @@ public class DataTypeHelper {
 	 *
 	 * @param index the component index of where to add the data type.
 	 * @param dt the data type to add
-	 *
+	 * @param useAlignedLength if true a fixed-length primitive data type will use its 
+	 * {@link DataType#getAlignedLength() aligned-length}, otherwise it will use its
+	 * {@link DataType#getLength() raw length}.
 	 * @return the data type and its size or null if the user canceled when 
 	 * prompted for a size.
 	 */
 	public static DataTypeInstance getFixedLength(CompositeEditorModel model, int index,
-			DataType dt) {
+			DataType dt, boolean useAlignedLength) {
 		if (dt instanceof FactoryDataType) {
 			model.setStatus("Factory data types are not allowed in a composite data type.");
 			return null;
@@ -203,7 +206,7 @@ public class DataTypeHelper {
 			int maxBytes = model.getMaxReplaceLength(index);
 			return requestBytes(model, dt, maxBytes);
 		}
-		return DataTypeInstance.getDataTypeInstance(dt, length, true);
+		return DataTypeInstance.getDataTypeInstance(dt, length, useAlignedLength);
 	}
 
 	public static DataTypeInstance requestBytes(CompositeEditorModel model, DataType dt,
@@ -228,7 +231,8 @@ public class DataTypeHelper {
 
 		if (size >= 1) {
 			model.setLastNumBytes(size);
-			return DataTypeInstance.getDataTypeInstance(dt, size, true);
+			return DataTypeInstance.getDataTypeInstance(dt, size,
+				model.usesAlignedLengthComponents());
 		}
 		return null;
 	}

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/StructureEditorModel.java

@@ -162,7 +162,7 @@ class StructureEditorModel extends CompEditorModel {
 			DataType dt = dtc.getDataType();
 			int dtLen = dt.getLength();
 			return DataTypeInstance.getDataTypeInstance(dt, (dtLen > 0) ? dtLen : dtc.getLength(),
-				true);
+				usesAlignedLengthComponents());
 		}
 		else if (columnIndex == getNameColumn()) {
 			value = dtc.getFieldName();
@@ -1154,8 +1154,7 @@ class StructureEditorModel extends CompEditorModel {
 	}
 
 	private void doCreateInternalStructure(DataTypeManager dtm, CategoryPath categoryPath,
-			String name, TaskMonitor monitor)
+			String name, TaskMonitor monitor) throws InvalidDataTypeException, UsrException {
-			throws InvalidDataTypeException, UsrException {
 
 		int length = 0;
 		StructureDataType structureDataType =
@@ -1250,9 +1249,8 @@ class StructureEditorModel extends CompEditorModel {
 		};
 
 		String title = "Specify the Structure's Name";
-		InputDialog nameStructureDialog =
+		InputDialog nameStructureDialog = new InputDialog(title,
-			new InputDialog(title, new String[] { "New Structure's Name: " },
+			new String[] { "New Structure's Name: " }, new String[] { defaultName }, listener);
-				new String[] { defaultName }, listener);
 
 		provider.getPlugin().getTool().showDialog(nameStructureDialog);
 

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/data/CreateArrayAction.java

@@ -141,6 +141,7 @@ class CreateArrayAction extends ListingContextAction {
 		}
 
 		int length = sel.getByteLength();
+		// Arrays currently use aligned-length only
 		int numElements = length / dt.getAlignedLength();
 
 		Command cmd = new CreateArrayInStructureCmd(from.getAddress(), numElements, dt,
@@ -161,6 +162,7 @@ class CreateArrayAction extends ListingContextAction {
 			}
 			length += dtc.getLength();
 		}
+		// Arrays currently use aligned-length only
 		return length / dt.getAlignedLength();
 	}
 
@@ -171,6 +173,7 @@ class CreateArrayAction extends ListingContextAction {
 			DataTypeComponent dtc = struct.getComponent(index++);
 			length += dtc.getLength();
 		}
+		// Arrays currently use aligned-length only
 		return length / dt.getAlignedLength();
 	}
 

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/stackeditor/StackEditorModel.java

@@ -203,7 +203,7 @@ public class StackEditorModel extends CompositeEditorModel {
 				dt = element.getDataType();
 				dtLen = dt.getLength();
 				return DataTypeInstance.getDataTypeInstance(dt,
-					(dtLen > 0) ? dtLen : element.getLength(), true);
+					(dtLen > 0) ? dtLen : element.getLength(), usesAlignedLengthComponents());
 			case NAME:
 				String fieldName = getFieldNameAtRow(rowIndex, (StackFrameDataType) viewComposite);
 				if (fieldName == null) {
@@ -813,8 +813,7 @@ public class StackEditorModel extends CompositeEditorModel {
 	}
 
 	@Override
-	public boolean setComponentName(int rowIndex, String newName)
+	public boolean setComponentName(int rowIndex, String newName) throws InvalidNameException {
-			throws InvalidNameException {
 
 		if (newName.trim().length() == 0) {
 			newName = null;
@@ -1125,8 +1124,9 @@ public class StackEditorModel extends CompositeEditorModel {
 		OffsetPairs offsetSelection = getRelOffsetSelection();
 		int transID = startTransaction("Apply Data Type \"" + dt.getName() + "\"");
 		try {
-			fieldEdited(DataTypeInstance.getDataTypeInstance(dt, dtLength, true), index,
+			fieldEdited(
-				getDataTypeColumn());
+				DataTypeInstance.getDataTypeInstance(dt, dtLength, usesAlignedLengthComponents()),
+				index, getDataTypeColumn());
 			setRelOffsetSelection(offsetSelection);
 		}
 		finally {
@@ -1155,6 +1155,7 @@ public class StackEditorModel extends CompositeEditorModel {
 		if (max == Integer.MAX_VALUE) {
 			return Integer.MAX_VALUE;
 		}
+		// Arrays currently use aligned-length only
 		return max / dtc.getDataType().getAlignedLength();
 	}
 
@@ -1318,7 +1319,7 @@ public class StackEditorModel extends CompositeEditorModel {
 			dtName = dt.getDisplayName();
 			if (dtString.equals(dtName)) {
 				return DataTypeInstance.getDataTypeInstance(element.getDataType(),
-					element.getLength(), true);
+					element.getLength(), usesAlignedLengthComponents());
 			}
 		}
 
@@ -1344,7 +1345,8 @@ public class StackEditorModel extends CompositeEditorModel {
 		if (maxLength > 0 && newLength > maxLength) {
 			throw new UsrException(newDt.getDisplayName() + " doesn't fit.");
 		}
-		return DataTypeInstance.getDataTypeInstance(newDt, newLength, true);
+		return DataTypeInstance.getDataTypeInstance(newDt, newLength,
+			usesAlignedLengthComponents());
 	}
 
 	@Override

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/core/data/ProgramProviderContext.java

@@ -58,7 +58,7 @@ public class ProgramProviderContext implements DataTypeProviderContext {
 
 		DataType dt = data.getDataType();
 		int length = DataTypeComponentImpl.getPreferredComponentLength(dt,
-			Math.max(data.getLength(), dt.getAlignedLength()));
+			Math.max(data.getLength(), dt.getLength()));
 		String label = null;
 		Symbol symbol = data.getPrimarySymbol();
 		if (symbol != null && !symbol.isDynamic()) {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/core/data/ProgramStructureProviderContext.java

@@ -15,6 +15,8 @@
  */
 package ghidra.app.plugin.core.data;
 
+import java.util.ArrayList;
+
 import ghidra.program.model.address.Address;
 import ghidra.program.model.data.*;
 import ghidra.program.model.lang.DataTypeProviderContext;
@@ -22,8 +24,6 @@ import ghidra.program.model.listing.Data;
 import ghidra.program.model.listing.Program;
 import ghidra.program.util.ProgramLocation;
 
-import java.util.ArrayList;
-
 public class ProgramStructureProviderContext implements DataTypeProviderContext {
 	Program program;
 	Address addr;

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

@@ -65,27 +65,74 @@ abstract class CompositeDB extends DataTypeDB implements CompositeInternal {
 	}
 
 	/**
-	 * Get the preferred length for a new component. For Unions and packed
+	 * Get the preferred length for a new component. If type is dynamic length must be specified
-	 * structures the preferred component length for a fixed-length dataType
+	 * (assuming {@link Dynamic#canSpecifyLength()} is true).  Otherwise, when packing is enabled
-	 * will be the length of that dataType. Otherwise the length returned will be no
+	 * the {@link DataType#getAlignedLength()} is returned; when packing disabled for Union
-	 * larger than the specified length.
+	 * use of fixed-length type size is forced. Otherwise the decision
+	 * is deferred to {@link DataTypeComponentImpl#getPreferredComponentLength(DataType, int)}.
+	 * During packing the actual component length may be changed.
 	 * 
 	 * @param dataType new component datatype
-	 * @param length   constrained length or -1 to force use of dataType size.
+	 * @param length   constrained length or -1 to force use of dataType size for non-packing.
-	 *                 Dynamic types such as string must have a positive length
+	 *                 Dynamic types such as string must have a positive length specified.
+	 *                 This value is ignored for fixed-length types if maxLength has been 
 	 *                 specified.
+	 * @param maxLength applies to non-packed structures only to indicate available space for
+	 *                 fixed-length types using {@link DataType#getLength()}. Specify -1
+	 *                 to ignore this value.
 	 * @return preferred component length
+	 * @throws IllegalArgumentException if length not specified for a {@link Dynamic} dataType.
 	 */
-	protected int getPreferredComponentLength(DataType dataType, int length) {
+	protected int getPreferredComponentLength(DataType dataType, int length, int maxLength) {
+
 		if (DataTypeComponent.usesZeroLengthComponent(dataType)) {
 			return 0;
 		}
-		if ((isPackingEnabled() || (this instanceof Union)) && !(dataType instanceof Dynamic)) {
+
-			length = -1; // force use of datatype size
+		if (!(dataType instanceof Dynamic)) {
+			if (isPackingEnabled()) {
+				length = dataType.getAlignedLength();
+				if (length > 0) {
+					return length;
+				}
+			}
+			else if (this instanceof Union) {
+				// enforce Union component size for fixed-length types
+				length = dataType.getLength();
+				if (length > 0) {
+					return length;
+				}
+			}
+			else if (maxLength >= 0) {
+				// length determined by datatype but must not exceed maxLength
+				length = Math.min(dataType.getLength(), maxLength);
+				if (length > 0) {
+					return length;
+				}
+			}
 		}
+
 		return DataTypeComponentImpl.getPreferredComponentLength(dataType, length);
 	}
-	
+
+	/**
+	 * Get the preferred length for a new component. If type is dynamic length must be specified
+	 * (assuming {@link Dynamic#canSpecifyLength()} is true).  Otherwise, when packing is enabled
+	 * the {@link DataType#getAlignedLength()} is returned; when packing disabled for Union
+	 * use of fixed-length type size is forced. Otherwise the decision
+	 * is deferred to {@link DataTypeComponentImpl#getPreferredComponentLength(DataType, int)}.
+	 * During packing the actual component length may be changed.
+	 * 
+	 * @param dataType new component datatype
+	 * @param length   constrained length or -1 to force use of dataType size for non-packing.
+	 *                 Dynamic types such as string must have a positive length specified.
+	 * @return preferred component length
+	 * @throws IllegalArgumentException if length not specified for a {@link Dynamic} dataType.
+	 */
+	protected int getPreferredComponentLength(DataType dataType, int length) {
+		return getPreferredComponentLength(dataType, length, -1);
+	}
+
 	@Override
 	protected String doGetName() {
 		return record.getString(CompositeDBAdapter.COMPOSITE_NAME_COL);
@@ -193,7 +240,7 @@ abstract class CompositeDB extends DataTypeDB implements CompositeInternal {
 
 	@Override
 	public abstract boolean hasLanguageDependantLength();
-	
+
 	/**
 	 * Determine if this composite should be treated as undefined.
 	 * <p>
@@ -283,8 +330,7 @@ abstract class CompositeDB extends DataTypeDB implements CompositeInternal {
 		}
 	}
 
-	protected DataType doCheckedResolve(DataType dt)
+	protected DataType doCheckedResolve(DataType dt) throws DataTypeDependencyException {
-			throws DataTypeDependencyException {
 		if (dt instanceof Pointer) {
 			pointerPostResolveRequired = true;
 			return resolve(((Pointer) dt).newPointer(DataType.DEFAULT));
@@ -681,7 +727,7 @@ abstract class CompositeDB extends DataTypeDB implements CompositeInternal {
 	public String toString() {
 		return CompositeInternal.toString(this);
 	}
-	
+
 	/**
 	 * Perform any neccessary component adjustments based on sizes of components differing from 
 	 * their specification which may be influenced by the data organization.  This method

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

@@ -129,18 +129,16 @@ class StructureDB extends CompositeDB implements StructureInternal {
 			dt = dataMgr.resolve(dt, null); // use zero-length array
 			oldFlexArrayRecord.setIntValue(ComponentDBAdapter.COMPONENT_ORDINAL_COL,
 				numComponents++);
-			oldFlexArrayRecord.setIntValue(ComponentDBAdapter.COMPONENT_OFFSET_COL,
+			oldFlexArrayRecord.setIntValue(ComponentDBAdapter.COMPONENT_OFFSET_COL, structLength);
-				structLength);
 			oldFlexArrayRecord.setLongValue(ComponentDBAdapter.COMPONENT_DT_ID_COL,
 				dataMgr.getID(dt));
 			componentAdapter.updateRecord(oldFlexArrayRecord);
 
-			component = new DataTypeComponentDB(dataMgr, componentAdapter, this,
+			component =
-				oldFlexArrayRecord);
+				new DataTypeComponentDB(dataMgr, componentAdapter, this, oldFlexArrayRecord);
 
 			// Update component count (flex-array had been excluded prviously)
-			record.setIntValue(CompositeDBAdapter.COMPOSITE_NUM_COMPONENTS_COL,
+			record.setIntValue(CompositeDBAdapter.COMPOSITE_NUM_COMPONENTS_COL, numComponents);
-				numComponents);
 			compositeAdapter.updateRecord(record, false);
 			repack = isPackingEnabled();
 		}
@@ -148,10 +146,9 @@ class StructureDB extends CompositeDB implements StructureInternal {
 			// read-only mode must use proxy component with zero-length array
 			String fieldName =
 				oldFlexArrayRecord.getString(ComponentDBAdapter.COMPONENT_FIELD_NAME_COL);
-			String comment =
+			String comment = oldFlexArrayRecord.getString(ComponentDBAdapter.COMPONENT_COMMENT_COL);
-				oldFlexArrayRecord.getString(ComponentDBAdapter.COMPONENT_COMMENT_COL);
+			component = new DataTypeProxyComponentDB(dataMgr, this, numComponents++, structLength,
-			component = new DataTypeProxyComponentDB(dataMgr, this, numComponents++,
+				dt, 0, fieldName, comment);
-				structLength, dt, 0, fieldName, comment);
 		}
 
 		components.add(component);
@@ -179,7 +176,7 @@ class StructureDB extends CompositeDB implements StructureInternal {
 			throw new IllegalArgumentException(e.getMessage(), e);
 		}
 	}
-	
+
 	private DataTypeComponent doAdd(DataType dataType, int length, String name, String comment,
 			boolean validatePackAndNotify)
 			throws DataTypeDependencyException, IllegalArgumentException {
@@ -202,9 +199,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 				}
 				else {
 					int componentLength = getPreferredComponentLength(dataType, length);
-					DBRecord rec =
+					DBRecord rec = componentAdapter.createRecord(dataMgr.getResolvedID(dataType),
-						componentAdapter.createRecord(dataMgr.getResolvedID(dataType), key,
+						key, componentLength, numComponents, structLength, name, comment);
-							componentLength, numComponents, structLength, name, comment);
 					dtc = new DataTypeComponentDB(dataMgr, componentAdapter, this, rec);
 					dataType.addParent(this);
 					components.add(dtc);
@@ -321,9 +317,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 			length = getPreferredComponentLength(dataType, length);
 
 			int offset = getComponent(ordinal).getOffset();
-			DBRecord rec =
+			DBRecord rec = componentAdapter.createRecord(dataMgr.getResolvedID(dataType), key,
-				componentAdapter.createRecord(dataMgr.getResolvedID(dataType), key, length,
+				length, ordinal, offset, name, comment);
-					ordinal, offset, name, comment);
 			DataTypeComponentDB dtc = new DataTypeComponentDB(dataMgr, componentAdapter, this, rec);
 			dataType.addParent(this);
 			shiftOffsets(idx, 1, dtc.getLength());
@@ -736,9 +731,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 			if (isPackingEnabled()) {
 				return components.get(ordinal);
 			}
-			int idx =
+			int idx = Collections.binarySearch(components, Integer.valueOf(ordinal),
-				Collections.binarySearch(components, Integer.valueOf(ordinal),
+				OrdinalComparator.INSTANCE);
-					OrdinalComparator.INSTANCE);
 			if (idx >= 0) {
 				return components.get(idx);
 			}
@@ -984,9 +978,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 				return;
 			}
 
-			int index =
+			int index = Collections.binarySearch(components, Integer.valueOf(offset),
-				Collections.binarySearch(components, Integer.valueOf(offset),
+				OffsetComparator.INSTANCE);
-					OffsetComparator.INSTANCE);
 
 			if (index < 0) {
 				if (offset == structLength) {
@@ -1027,9 +1020,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 				return;
 			}
 
-			int index =
+			int index = Collections.binarySearch(components, Integer.valueOf(offset),
-				Collections.binarySearch(components, Integer.valueOf(offset),
+				OffsetComparator.INSTANCE);
-					OffsetComparator.INSTANCE);
 			if (index < 0) {
 				return;
 			}
@@ -1088,9 +1080,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 			if (offset > structLength || offset < 0) {
 				return null;
 			}
-			int index =
+			int index = Collections.binarySearch(components, Integer.valueOf(offset),
-				Collections.binarySearch(components, Integer.valueOf(offset),
+				OffsetComparator.INSTANCE);
-					OffsetComparator.INSTANCE);
 			if (index >= 0) {
 				// return first matching defined component containing offset
 				DataTypeComponent dtc = components.get(index);
@@ -1122,9 +1113,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 			if (offset > structLength || offset < 0) {
 				return list;
 			}
-			int index =
+			int index = Collections.binarySearch(components, Integer.valueOf(offset),
-				Collections.binarySearch(components, Integer.valueOf(offset),
+				OffsetComparator.INSTANCE);
-					OffsetComparator.INSTANCE);
 			boolean hasSizedComponent = false;
 			if (index >= 0) {
 				// collect matching defined components containing offset
@@ -1250,9 +1240,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 				structLength = offset;
 			}
 
-			int index =
+			int index = Collections.binarySearch(components, Integer.valueOf(offset),
-				Collections.binarySearch(components, Integer.valueOf(offset),
+				OffsetComparator.INSTANCE);
-					OffsetComparator.INSTANCE);
 
 			int additionalShift = 0;
 			if (index >= 0) {
@@ -1280,9 +1269,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 
 			length = getPreferredComponentLength(dataType, length);
 
-			DBRecord rec =
+			DBRecord rec = componentAdapter.createRecord(dataMgr.getResolvedID(dataType), key,
-				componentAdapter.createRecord(dataMgr.getResolvedID(dataType), key, length,
+				length, ordinal, offset, name, comment);
-					ordinal, offset, name, comment);
 			dataType.addParent(this);
 			DataTypeComponentDB dtc = new DataTypeComponentDB(dataMgr, componentAdapter, this, rec);
 			shiftOffsets(index, 1 + additionalShift, length + additionalShift);
@@ -1316,8 +1304,7 @@ class StructureDB extends CompositeDB implements StructureInternal {
 	 */
 	private DataTypeComponent doComponentReplacement(
 			LinkedList<DataTypeComponentDB> replacedComponents, int offset, DataType dataType,
-			int length, String componentName, String comment)
+			int length, String componentName, String comment) throws IOException {
-			throws IOException {
 
 		// Attempt quick update of a single defined component if possible.
 		// A quick update requires that component characteristics including length, offset, 
@@ -1325,11 +1312,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 		// repack.
 		DataTypeComponentDB oldComponent = replacedComponents.get(0);
 		DataType oldDt = oldComponent.getDataType();
-		if (replacedComponents.size() == 1 &&
+		if (replacedComponents.size() == 1 && oldDt != DEFAULT && dataType != DEFAULT &&
-			oldDt != DEFAULT &&
+			length == oldComponent.getLength() && offset == oldComponent.getOffset() &&
-			dataType != DEFAULT &&
-			length == oldComponent.getLength() &&
-			offset == oldComponent.getOffset() &&
 			(!isPackingEnabled() || dataType.getAlignment() == oldDt.getAlignment())) {
 
 			oldComponent.update(componentName, dataType, comment);
@@ -1339,8 +1323,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 			return oldComponent;
 		}
 
-		DataTypeComponent replaceComponent = replaceComponents(replacedComponents, dataType,
+		DataTypeComponent replaceComponent =
-			offset, length, componentName, comment);
+			replaceComponents(replacedComponents, dataType, offset, length, componentName, comment);
 
 		repack(false, false);
 		record.setIntValue(CompositeDBAdapter.COMPOSITE_LENGTH_COL, structLength);
@@ -1357,7 +1341,7 @@ class StructureDB extends CompositeDB implements StructureInternal {
 	}
 
 	@Override
-	public DataTypeComponent replace(int ordinal, DataType dataType, int length, 
+	public DataTypeComponent replace(int ordinal, DataType dataType, int length,
 			String componentName, String comment) {
 		lock.acquire();
 		try {
@@ -1494,7 +1478,7 @@ class StructureDB extends CompositeDB implements StructureInternal {
 				// defined component found - advance to last one containing offset
 				index = advanceToLastComponentContainingOffset(index, offset);
 				origDtc = components.get(index);
-				
+
 				// case 1: only defined component(s) at offset are zero-length
 				if (origDtc.getLength() == 0) {
 					if (isPackingEnabled()) {
@@ -1551,7 +1535,7 @@ class StructureDB extends CompositeDB implements StructureInternal {
 			}
 
 			length = getPreferredComponentLength(dataType, length);
-			
+
 			DataTypeComponent replaceComponent = doComponentReplacement(replacedComponents, offset,
 				dataType, length, componentName, comment);
 
@@ -1707,15 +1691,15 @@ class StructureDB extends CompositeDB implements StructureInternal {
 		DataTypeComponent[] otherComponents = struct.getDefinedComponents();
 		for (int i = 0; i < otherComponents.length; i++) {
 			DataTypeComponent dtc = otherComponents[i];
-
 			DataType dt = resolvedDts[i]; // ancestry check already performed by caller
-			int length = DataTypeComponent.usesZeroLengthComponent(dt) ? 0 : dt.getAlignedLength();
+
-			if (length < 0 || dtc.isBitFieldComponent()) {
+			int length;
+			if (dtc.isBitFieldComponent() || (dt instanceof Dynamic)) {
 				// TODO: bitfield truncation/expansion may be an issue if data organization changes
 				length = dtc.getLength();
 			}
 			else {
-				// do not exceed available space
+				// determine maxLength for fixed-length types
 				int maxOffset;
 				int nextIndex = i + 1;
 				if (nextIndex < otherComponents.length) {
@@ -1724,9 +1708,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 				else {
 					maxOffset = structLength;
 				}
-				if (length > 0) {
+				int maxLength = maxOffset - dtc.getOffset();
-					length = Math.min(length, maxOffset - dtc.getOffset());
+				length = getPreferredComponentLength(dt, -1, maxLength);
-				}
 			}
 
 			DBRecord rec = componentAdapter.createRecord(dataMgr.getResolvedID(dt), key, length,
@@ -1752,7 +1735,7 @@ class StructureDB extends CompositeDB implements StructureInternal {
 			DataType dt = resolvedDts[i]; // ancestry check already performed by caller
 			DBRecord rec =
 				componentAdapter.createRecord(dataMgr.getResolvedID(dt), key, dtc.getLength(),
-				dtc.getOrdinal(), dtc.getOffset(), dtc.getFieldName(), dtc.getComment());
+					dtc.getOrdinal(), dtc.getOffset(), dtc.getFieldName(), dtc.getComment());
 			dt.addParent(this);
 			DataTypeComponentDB newDtc =
 				new DataTypeComponentDB(dataMgr, componentAdapter, this, rec);
@@ -1776,7 +1759,7 @@ class StructureDB extends CompositeDB implements StructureInternal {
 				}
 				if (removeBitFieldComponent || dtc.getDataType() == dt) {
 					doDelete(i);
-// FIXME: Consider replacing with undefined type instead of removing (don't remove bitfield)
+					// for non-packed offsets of remaining components will not change
 					shiftOffsets(i, dtc.getLength() - 1, 0); // ordinals only
 					--numComponents; // may be revised by repack
 					changed = true;
@@ -1794,6 +1777,30 @@ class StructureDB extends CompositeDB implements StructureInternal {
 		}
 	}
 
+	/**
+	 * Get the available space for an existing defined component in relation to the next defined 
+	 * component or the end of the structure.  Method should be used in conjunction with
+	 * {@link #consumeBytesAfter(int, int)} and/or {@link #shiftOffsets(int, int, int)} for
+	 * non-packed structure use.  This method is intended to supplt the maxLength parameter
+	 * for the {@link #getPreferredComponentLength(DataType, int, int)} method call.
+	 * 
+	 * @param index defined components index
+	 * @return available space for this component (i.e., maxLength). {@link Integer#MAX_VALUE} 
+	 * is returned if last component in non-packed structure, or -1 if structure is packed.
+	 */
+	private int getAvailableComponentSpace(int index) {
+		if (isPackingEnabled()) {
+			return -1;
+		}
+		// determine maximum component space available
+		int nextIndex = index + 1;
+		if (nextIndex < components.size()) {
+			DataTypeComponentDB dtc = components.get(index);
+			return components.get(nextIndex).getOffset() - dtc.getOffset();
+		}
+		return Integer.MAX_VALUE;
+	}
+
 	@Override
 	public void dataTypeSizeChanged(DataType dt) {
 		if (dt instanceof BitFieldDataType) {
@@ -1817,11 +1824,10 @@ class StructureDB extends CompositeDB implements StructureInternal {
 				if (dtc.getDataType() == dt) {
 					// assume no impact to bitfields since base types should not change size
 					int dtcLen = dtc.getLength();
+
 					int length =
-						DataTypeComponent.usesZeroLengthComponent(dt) ? 0 : dt.getAlignedLength();
+						getPreferredComponentLength(dt, dtcLen, getAvailableComponentSpace(i));
-					if (length < 0) {
+
-						length = dtcLen;
-					}
 					if (length < dtcLen) {
 						dtc.setLength(length, true);
 						shiftOffsets(i + 1, dtcLen - length, 0); // updates structure record and last modified time
@@ -1876,15 +1882,15 @@ class StructureDB extends CompositeDB implements StructureInternal {
 				forceRepack |= isPacked;
 				continue;
 			}
-			int length = DataTypeComponent.usesZeroLengthComponent(dt) ? 0 : dt.getAlignedLength();
+			int dtcLen = dtc.getLength();
+			int length = getPreferredComponentLength(dt, dtcLen, getAvailableComponentSpace(i));
 			if (length < 0) {
 				continue; // illegal condition - skip
 			}
-			int dtcLen = dtc.getLength();
 			if (dtcLen != length) {
 				if (isPacked) {
-					dtc.setLength(length, true);
 					changed = true;
+					break;
 				}
 				else if (length < dtcLen) {
 					dtc.setLength(length, true);
@@ -2162,9 +2168,8 @@ class StructureDB extends CompositeDB implements StructureInternal {
 			index = newOrdinal;
 		}
 		else {
-			index =
+			index = Collections.binarySearch(components, Integer.valueOf(origFirstOrdinal),
-				Collections.binarySearch(components, Integer.valueOf(origFirstOrdinal),
+				OrdinalComparator.INSTANCE);
-					OrdinalComparator.INSTANCE);
 		}
 		if (index < 0) {
 			index = -index - 1; // undefined component replacement
@@ -2252,7 +2257,6 @@ class StructureDB extends CompositeDB implements StructureInternal {
 			for (int i = components.size() - 1; i >= 0; i--) {
 
 				DataTypeComponentDB comp = components.get(i);
-				int nextIndex = i + 1;
 
 				boolean remove = false;
 				if (comp.isBitFieldComponent()) {
@@ -2276,7 +2280,7 @@ class StructureDB extends CompositeDB implements StructureInternal {
 						remove = true;
 					}
 					else {
-						setComponentDataType(comp, replacementDt, nextIndex);
+						setComponentDataType(comp, replacementDt, i);
 						changed = true;
 					}
 				}
@@ -2301,48 +2305,30 @@ class StructureDB extends CompositeDB implements StructureInternal {
 		}
 	}
 
-	private void setComponentDataType(DataTypeComponentDB comp, DataType newDt,
+	private void setComponentDataType(DataTypeComponentDB comp, DataType newDt, int index)
-			int nextIndex) throws IOException {
+			throws IOException {
-
-		int oldLen = comp.getLength();
-		int len = DataTypeComponent.usesZeroLengthComponent(newDt) ? 0 : newDt.getAlignedLength();
-		if (len < 0) {
-			len = oldLen;
-		}
 
+		dataMgr.getSettingsAdapter().removeAllSettingsRecords(comp.getKey());
 		comp.getDataType().removeParent(this);
 
-		if (isPackingEnabled()) {
-			comp.setLength(len, false); // do before record save below
-		}
 		comp.setDataType(newDt); // saves component record
-		dataMgr.getSettingsAdapter().removeAllSettingsRecords(comp.getKey());
 		newDt.addParent(this);
 
 		if (isPackingEnabled()) {
 			return;
 		}
 
-		if (len < oldLen) {
+		int oldLen = comp.getLength();
-			comp.setLength(len, true);
+		int length = getPreferredComponentLength(newDt, oldLen, getAvailableComponentSpace(index));
-			shiftOffsets(nextIndex, oldLen - len, 0);
+
+		if (length < oldLen) {
+			comp.setLength(length, true);
+			shiftOffsets(index + 1, oldLen - length, 0); // updates structure record and last modified time
 		}
-		else if (len > oldLen) {
+		else if (length > oldLen) {
-			int bytesAvailable = getNumUndefinedBytes(comp.getOrdinal() + 1);
+			int consumed = consumeBytesAfter(index, length - oldLen); // updates component record
-			int bytesNeeded = len - oldLen;
+			if (consumed > 0) {
-			if (bytesNeeded <= bytesAvailable) {
+				shiftOffsets(index + 1, -consumed, 0); // updates structure record and last modified time
-				comp.setLength(len, true);
-				shiftOffsets(nextIndex, -bytesNeeded, 0);
-			}
-			else if (comp.getOrdinal() == getLastDefinedComponentOrdinal()) {
-				// we are the last defined component, grow structure
-				doGrowStructure(bytesNeeded - bytesAvailable);
-				comp.setLength(len, true);
-				shiftOffsets(nextIndex, -bytesNeeded, 0);
-			}
-			else {
-				comp.setLength(oldLen + bytesAvailable, true);
-				shiftOffsets(nextIndex, -bytesAvailable, 0);
 			}
 		}
 	}

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

@@ -487,7 +487,7 @@ class UnionDB extends CompositeDB implements UnionInternal {
 			if (dt instanceof BitFieldDataType) {
 				dt = adjustBitField(dt); // in case base type changed
 			}
-			int length = DataTypeComponent.usesZeroLengthComponent(dt) ? 0 : dt.getAlignedLength();
+			int length = getPreferredComponentLength(dt, -1);
 			if (length < 0) {
 				continue; // illegal condition - skip
 			}
@@ -533,9 +533,8 @@ class UnionDB extends CompositeDB implements UnionInternal {
 			boolean changed = false;
 			for (DataTypeComponentDB dtc : components) {
 				if (dtc.getDataType() == dt) {
-					int length =
+					int length = getPreferredComponentLength(dt, dtc.getLength());
-						DataTypeComponent.usesZeroLengthComponent(dt) ? 0 : dt.getAlignedLength();
+					if (length != dtc.getLength()) {
-					if (length >= 0 && length != dtc.getLength()) {
 						dtc.setLength(length, true);
 						changed = true;
 					}
@@ -824,13 +823,9 @@ class UnionDB extends CompositeDB implements UnionInternal {
 						remove = true;
 					}
 					else {
-						int len = DataTypeComponent.usesZeroLengthComponent(newDt) ? 0
+						int len = getPreferredComponentLength(newDt, dtc.getLength());
-								: newDt.getAlignedLength();
-						if (len < 0) {
-							len = dtc.getLength();
-						}
-						oldDt.removeParent(this);
 						dtc.setLength(len, false);
+						oldDt.removeParent(this);
 						dtc.setDataType(replacementDt); // updates record
 						dataMgr.getSettingsAdapter().removeAllSettingsRecords(dtc.getKey());
 						replacementDt.addParent(this);

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

@@ -23,6 +23,10 @@ import ghidra.program.model.mem.MemBuffer;
  */
 public interface Array extends DataType {
 
+	// TODO: May need to handle both packed and non-packed arrays where packed would use
+	// DataType.getAlignedLength() while non-packed would not.  At present, arrays are 
+	// always are treated as packed using getAlignedLength.
+
 	public static final String ARRAY_LABEL_PREFIX = "ARRAY";
 
 	/**
@@ -111,8 +115,8 @@ public interface Array extends DataType {
 		ArrayStringable stringableElementType = ArrayStringable.getArrayStringable(getDataType());
 		String value =
 			(stringableElementType != null && stringableElementType.hasStringValue(settings))
-					? new StringDataInstance(stringableElementType, settings, buf, length,
+					? new StringDataInstance(stringableElementType, settings, buf, length, true)
-						true).getStringRepresentation()
+							.getStringRepresentation()
 					: null;
 		return (value != null) ? value : "";
 	}

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

@@ -83,27 +83,74 @@ public abstract class CompositeDataTypeImpl extends GenericDataType implements C
 	}
 
 	/**
-	 * Get the preferred length for a new component. For Unions and internally
+	 * Get the preferred length for a new component. If type is dynamic length must be specified
-	 * aligned structures the preferred component length for a fixed-length dataType
+	 * (assuming {@link Dynamic#canSpecifyLength()} is true).  Otherwise, when packing is enabled
-	 * will be the length of that dataType. Otherwise the length returned will be no
+	 * the {@link DataType#getAlignedLength()} is returned; when packing disabled for Union
-	 * larger than the specified length.
+	 * use of fixed-length type size is forced. Otherwise the decision
+	 * is deferred to {@link DataTypeComponentImpl#getPreferredComponentLength(DataType, int)}.
+	 * During packing the actual component length may be changed.
 	 * 
 	 * @param dataType new component datatype
-	 * @param length   constrained length or -1 to force use of dataType size.
+	 * @param length   constrained length or -1 to force use of dataType size for non-packing.
-	 *                 Dynamic types such as string must have a positive length
+	 *                 Dynamic types such as string must have a positive length specified.
+	 *                 This value is ignored for fixed-length types if maxLength has been 
 	 *                 specified.
+	 * @param maxLength applies to non-packed structures only to indicate available space for
+	 *                 fixed-length types using {@link DataType#getLength()}. Specify -1
+	 *                 to ignore this value.
 	 * @return preferred component length
+	 * @throws IllegalArgumentException if length not specified for a {@link Dynamic} dataType.
 	 */
-	protected int getPreferredComponentLength(DataType dataType, int length) {
+	protected int getPreferredComponentLength(DataType dataType, int length, int maxLength) {
+
 		if (DataTypeComponent.usesZeroLengthComponent(dataType)) {
 			return 0;
 		}
-		if ((isPackingEnabled() || (this instanceof Union)) && !(dataType instanceof Dynamic)) {
+
-			length = -1; // force use of datatype size
+		if (!(dataType instanceof Dynamic)) {
+			if (isPackingEnabled()) {
+				length = dataType.getAlignedLength();
+				if (length > 0) {
+					return length;
+				}
+			}
+			else if (this instanceof Union) {
+				// enforce Union component size for fixed-length types
+				length = dataType.getLength();
+				if (length > 0) {
+					return length;
+				}
+			}
+			else if (maxLength >= 0) {
+				// length determined by datatype but must not exceed maxLength
+				length = Math.min(dataType.getLength(), maxLength);
+				if (length > 0) {
+					return length;
+				}
+			}
 		}
+
 		return DataTypeComponentImpl.getPreferredComponentLength(dataType, length);
 	}
 
+	/**
+	 * Get the preferred length for a new component. If type is dynamic length must be specified
+	 * (assuming {@link Dynamic#canSpecifyLength()} is true).  Otherwise, when packing is enabled
+	 * the {@link DataType#getAlignedLength()} is returned; when packing disabled for Union
+	 * use of fixed-length type size is forced. Otherwise the decision
+	 * is deferred to {@link DataTypeComponentImpl#getPreferredComponentLength(DataType, int)}.
+	 * During packing the actual component length may be changed.
+	 * 
+	 * @param dataType new component datatype
+	 * @param length   constrained length or -1 to force use of dataType size for non-packing.
+	 *                 Dynamic types such as string must have a positive length specified.
+	 * @return preferred component length
+	 * @throws IllegalArgumentException if length not specified for a {@link Dynamic} dataType.
+	 */
+	protected int getPreferredComponentLength(DataType dataType, int length) {
+		return getPreferredComponentLength(dataType, length, -1);
+	}
+
 	@Override
 	public abstract boolean hasLanguageDependantLength();
 

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

@@ -24,6 +24,9 @@ import ghidra.util.exception.DuplicateNameException;
  */
 public interface DataTypeComponent {
 
+	// TODO: known issue accessing big-endian data when component-length differs from 
+	// datatype length.
+
 	/** The default prefix for the name of a component. */
 	public final static String DEFAULT_FIELD_NAME_PREFIX = "field";
 

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

@@ -356,16 +356,17 @@ public class DataTypeComponentImpl implements InternalDataTypeComponent, Seriali
 	 *                 Dynamic types such as string must have a positive length
 	 *                 specified.
 	 * @return preferred component length
+	 * @throws IllegalArgumentException if length not specified for a {@link Dynamic} dataType.
 	 */
 	public static int getPreferredComponentLength(DataType dataType, int length) {
 		if (DataTypeComponent.usesZeroLengthComponent(dataType)) {
 			return 0;
 		}
-		int dtLength = dataType.getAlignedLength();
+		int dtLength = dataType.getLength();
 		if (length <= 0) {
 			length = dtLength;
 		}
-		else if (dtLength > 0 && dtLength < length) {
+		else if (dtLength >= 0 && dtLength < length) { // constrain fixed-length type
 			length = dtLength;
 		}
 		if (length <= 0) {

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

@@ -88,9 +88,8 @@ public abstract class FactoryStructureDataType extends BuiltIn implements Factor
 	protected Structure setCategoryPath(Structure struct, MemBuffer buf) {
 		CategoryPath path = CategoryPath.ROOT;
 		try {
-			path =
+			path = new CategoryPath(new CategoryPath(CategoryPath.ROOT, getName()),
-				new CategoryPath(new CategoryPath(CategoryPath.ROOT, getName()), "" +
+				"" + buf.getAddress());
-					buf.getAddress());
 		}
 		catch (Exception e) {
 		}
@@ -136,8 +135,12 @@ public abstract class FactoryStructureDataType extends BuiltIn implements Factor
 	}
 
 	protected DataTypeComponent addComponent(Structure es, DataType dt, String componentName) {
+		return es.add(dt, dt.getLength(), componentName, null);
+	}
 
-		return es.add(dt, dt.getAlignedLength(), componentName, null);
+	protected DataTypeComponent addComponent(Structure es, DataType dt, int length,
+			String componentName) {
+		return es.add(dt, length, componentName, null);
 	}
 
 	protected abstract void populateDynamicStructure(MemBuffer buf, Structure es);

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

@@ -195,16 +195,15 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 		}
 		return null;
 	}
-	
+
 	@Override
 	public List<DataTypeComponent> getComponentsContaining(int offset) {
 		ArrayList<DataTypeComponent> list = new ArrayList<>();
 		if (offset > structLength || offset < 0) {
 			return list;
 		}
-		int index =
+		int index = Collections.binarySearch(components, Integer.valueOf(offset),
-			Collections.binarySearch(components, Integer.valueOf(offset),
+			OffsetComparator.INSTANCE);
-				OffsetComparator.INSTANCE);
 
 		boolean hasSizedComponent = false;
 		if (index >= 0) {
@@ -571,8 +570,7 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	 *             this composite data type or an invalid length is specified.
 	 */
 	private DataTypeComponentImpl doAdd(DataType dataType, int length, String componentName,
-			String comment, boolean packAndNotify)
+			String comment, boolean packAndNotify) throws IllegalArgumentException {
-			throws IllegalArgumentException {
 
 		dataType = validateDataType(dataType);
 
@@ -621,8 +619,9 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	}
 
 	@Override
-	public DataTypeComponent insert(int ordinal, DataType dataType, int length, String componentName,
+	public DataTypeComponent insert(int ordinal, DataType dataType, int length,
-			String comment) throws IndexOutOfBoundsException, IllegalArgumentException {
+			String componentName, String comment)
+			throws IndexOutOfBoundsException, IllegalArgumentException {
 		if (ordinal < 0 || ordinal > numComponents) {
 			throw new IndexOutOfBoundsException(ordinal);
 		}
@@ -666,8 +665,8 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 		length = getPreferredComponentLength(dataType, length);
 
 		int offset = (getComponent(ordinal)).getOffset();
-		DataTypeComponentImpl dtc = new DataTypeComponentImpl(dataType, this, length, ordinal, offset,
+		DataTypeComponentImpl dtc = new DataTypeComponentImpl(dataType, this, length, ordinal,
-			componentName, comment);
+			offset, componentName, comment);
 		dataType.addParent(this);
 		shiftOffsets(idx, 1, dtc.getLength());
 		components.add(idx, dtc);
@@ -988,6 +987,30 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 		return true;
 	}
 
+	/**
+	 * Get the available space for an existing defined component in relation to the next defined 
+	 * component or the end of the structure.  Method should be used in conjunction with
+	 * {@link #consumeBytesAfter(int, int)} and/or {@link #shiftOffsets(int, int, int)} for
+	 * non-packed structure use.  This method is intended to supplt the maxLength parameter
+	 * for the {@link #getPreferredComponentLength(DataType, int, int)} method call.
+	 * 
+	 * @param index defined components index
+	 * @return available space for this component (i.e., maxLength). {@link Integer#MAX_VALUE} 
+	 * is returned if last component in non-packed structure, or -1 if structure is packed.
+	 */
+	private int getAvailableComponentSpace(int index) {
+		if (isPackingEnabled()) {
+			return -1;
+		}
+		// determine maximum component space available
+		int nextIndex = index + 1;
+		if (nextIndex < components.size()) {
+			DataTypeComponentImpl dtc = components.get(index);
+			return components.get(nextIndex).getOffset() - dtc.getOffset();
+		}
+		return Integer.MAX_VALUE;
+	}
+
 	@Override
 	public void dataTypeSizeChanged(DataType dt) {
 		if (dt instanceof BitFieldDataType) {
@@ -1003,22 +1026,20 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 		for (int i = 0; i < n; i++) {
 			DataTypeComponentImpl dtc = components.get(i);
 			if (dtc.getDataType() == dt) {
-				// assume no impact to bitfields since base types
+				// assume no impact to bitfields since base types should not change size
-				// should not change size
 				int dtcLen = dtc.getLength();
-				int length = DataTypeComponent.usesZeroLengthComponent(dt) ? 0 : dt.getLength();
+
-				if (length < 0) {
+				int length = getPreferredComponentLength(dt, dtcLen, getAvailableComponentSpace(i));
-					length = dtcLen;
+
-				}
 				if (length < dtcLen) {
 					dtc.setLength(length);
-					shiftOffsets(i + 1, dtcLen - length, 0);
+					shiftOffsets(i + 1, dtcLen - length, 0); // updates structure record and last modified time
 					changed = true;
 				}
 				else if (length > dtcLen) {
-					int consumed = consumeBytesAfter(i, length - dtcLen);
+					int consumed = consumeBytesAfter(i, length - dtcLen); // updates component record
 					if (consumed > 0) {
-						shiftOffsets(i + 1, 0 - consumed, 0);
+						shiftOffsets(i + 1, -consumed, 0); // updates structure record and last modified time
 						changed = true;
 					}
 				}
@@ -1195,13 +1216,13 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 			DataType dt = dtc.getDataType().clone(dataMgr);
 			checkAncestry(dt);
 
-			int length = DataTypeComponent.usesZeroLengthComponent(dt) ? 0 : dt.getLength();
+			int length;
-			if (length < 0 || dtc.isBitFieldComponent()) {
+			if (dtc.isBitFieldComponent() || (dt instanceof Dynamic)) {
-				// TODO: bitfield truncation/expansion may be an issues if data organization changes
+				// TODO: bitfield truncation/expansion may be an issue if data organization changes
 				length = dtc.getLength();
 			}
 			else {
-				// do not exceed available space
+				// determine maxLength for fixed-length types
 				int maxOffset;
 				int nextIndex = i + 1;
 				if (nextIndex < otherComponents.length) {
@@ -1210,9 +1231,8 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 				else {
 					maxOffset = structLength;
 				}
-				if (length > 0) {
+				int maxLength = maxOffset - dtc.getOffset();
-					length = Math.min(length, maxOffset - dtc.getOffset());
+				length = getPreferredComponentLength(dt, -1, maxLength);
-				}
 			}
 
 			components.add(new DataTypeComponentImpl(dt, this, length, dtc.getOrdinal(),
@@ -1265,7 +1285,6 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 		for (int i = components.size() - 1; i >= 0; i--) {
 
 			DataTypeComponentImpl comp = components.get(i);
-			int nextIndex = i + 1;
 
 			boolean remove = false;
 			if (comp.isBitFieldComponent()) {
@@ -1288,7 +1307,7 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 					remove = true;
 				}
 				else {
-					setComponentDataType(comp, replacementDt, nextIndex);
+					setComponentDataType(comp, replacementDt, i);
 					changed = true;
 				}
 			}
@@ -1307,44 +1326,28 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 		}
 	}
 
-	private void setComponentDataType(DataTypeComponentImpl comp, DataType newDt, int nextIndex) {
+	private void setComponentDataType(DataTypeComponentImpl comp, DataType newDt, int index) {
-
-		int oldLen = comp.getLength();
-		int len = DataTypeComponent.usesZeroLengthComponent(newDt) ? 0 : newDt.getLength();
-		if (len < 0) {
-			len = oldLen;
-		}
 
 		comp.getDataType().removeParent(this);
-		comp.setDataType(newDt);
+
-		comp.invalidateSettings();
+		comp.setDataType(newDt); // saves component record
 		newDt.addParent(this);
 
 		if (isPackingEnabled()) {
-			comp.setLength(len);
 			return;
 		}
 
-		if (len < oldLen) {
+		int oldLen = comp.getLength();
-			comp.setLength(len);
+		int length = getPreferredComponentLength(newDt, oldLen, getAvailableComponentSpace(index));
-			shiftOffsets(nextIndex, oldLen - len, 0);
+
+		if (length < oldLen) {
+			comp.setLength(length);
+			shiftOffsets(index + 1, oldLen - length, 0); // updates structure record and last modified time
 		}
-		else if (len > oldLen) {
+		else if (length > oldLen) {
-			int bytesAvailable = getNumUndefinedBytes(comp.getOrdinal() + 1);
+			int consumed = consumeBytesAfter(index, length - oldLen); // updates component record
-			int bytesNeeded = len - oldLen;
+			if (consumed > 0) {
-			if (bytesNeeded <= bytesAvailable) {
+				shiftOffsets(index + 1, -consumed, 0); // updates structure record and last modified time
-				comp.setLength(len);
-				shiftOffsets(nextIndex, -bytesNeeded, 0);
-			}
-			else if (comp.getOrdinal() == getLastDefinedComponentOrdinal()) {
-				// we are the last defined component, grow structure
-				doGrowStructure(bytesNeeded - bytesAvailable);
-				comp.setLength(len);
-				shiftOffsets(nextIndex, -bytesNeeded, 0);
-			}
-			else {
-				comp.setLength(oldLen + bytesAvailable);
-				shiftOffsets(nextIndex, -bytesAvailable, 0);
 			}
 		}
 	}
@@ -1383,19 +1386,16 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 		// repack.
 		DataTypeComponentImpl oldComponent = replacedComponents.get(0);
 		DataType oldDt = oldComponent.getDataType();
-		if (replacedComponents.size() == 1 &&
+		if (replacedComponents.size() == 1 && oldDt != DEFAULT && dataType != DEFAULT &&
-			oldDt != DEFAULT &&
+			length == oldComponent.getLength() && offset == oldComponent.getOffset() &&
-			dataType != DEFAULT &&
-			length == oldComponent.getLength() &&
-			offset == oldComponent.getOffset() &&
 			(!isPackingEnabled() || dataType.getAlignment() == oldDt.getAlignment())) {
 
 			oldComponent.update(componentName, dataType, comment);
 			return oldComponent;
 		}
 
-		DataTypeComponent replaceComponent = replaceComponents(replacedComponents, dataType,
+		DataTypeComponent replaceComponent =
-			offset, length, componentName, comment);
+			replaceComponents(replacedComponents, dataType, offset, length, componentName, comment);
 
 		repack(false);
 		notifySizeChanged();
@@ -1407,10 +1407,11 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	public final DataTypeComponent replace(int index, DataType dataType, int length) {
 		return replace(index, dataType, length, null, null);
 	}
-	
+
 	@Override
-	public DataTypeComponent replace(int ordinal, DataType dataType, int length, String componentName,
+	public DataTypeComponent replace(int ordinal, DataType dataType, int length,
-			String comment) throws IndexOutOfBoundsException, IllegalArgumentException {
+			String componentName, String comment)
+			throws IndexOutOfBoundsException, IllegalArgumentException {
 		if (ordinal < 0 || ordinal >= numComponents) {
 			throw new IndexOutOfBoundsException(ordinal);
 		}
@@ -1434,7 +1435,7 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 			// defined component
 			DataTypeComponentImpl origDtc = components.get(index);
 			offset = origDtc.getOffset();
-			
+
 			if (isPackingEnabled() || length == 0) {
 				// case 1: packed structure or zero-length replacement - do 1-for-1 replacement
 				replacedComponents.add(origDtc);
@@ -1676,8 +1677,7 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 
 		if (!clearOnly && !isPackingEnabled()) {
 			int bytesNeeded = length - origLength + leadingUnusedBytes;
-			checkUndefinedSpaceAvailabilityAfter(origLastOrdinal, bytesNeeded, dataType,
+			checkUndefinedSpaceAvailabilityAfter(origLastOrdinal, bytesNeeded, dataType, newOffset);
-				newOffset);
 		}
 
 		// determine defined component list insertion point, remove old components
@@ -1705,8 +1705,8 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 		DataTypeComponentImpl newDtc = null;
 		if (!clearOnly) {
 			// insert new component
-			newDtc = new DataTypeComponentImpl(dataType, this, length, newOrdinal,
+			newDtc = new DataTypeComponentImpl(dataType, this, length, newOrdinal, newOffset, name,
-				newOffset, name, comment);
+				comment);
 			components.add(index, newDtc);
 		}
 
@@ -1789,7 +1789,7 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 			changed = packResult.componentsChanged;
 			changed |= (structLength != packResult.structureLength) ||
 				(structAlignment != packResult.alignment) ||
-					(numComponents != packResult.numComponents);
+				(numComponents != packResult.numComponents);
 			structLength = packResult.structureLength;
 			structAlignment = packResult.alignment;
 			numComponents = components.size();

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

@@ -126,14 +126,6 @@ public class UnionDataType extends CompositeDataTypeImpl implements UnionInterna
 		return components.size();
 	}
 
-	@Override
-	protected int getPreferredComponentLength(DataType dataType, int length) {
-		if (!(dataType instanceof Dynamic)) {
-			length = -1;
-		}
-		return super.getPreferredComponentLength(dataType, length);
-	}
-
 	@Override
 	public DataTypeComponent add(DataType dataType, int length, String componentName,
 			String comment) throws IllegalArgumentException {
@@ -400,7 +392,7 @@ public class UnionDataType extends CompositeDataTypeImpl implements UnionInterna
 			}
 			unionLength = Math.max(length, unionLength);
 		}
-		
+
 		unionAlignment = -1; // force recompute of unionAlignment
 		getAlignment();
 
@@ -490,8 +482,8 @@ public class UnionDataType extends CompositeDataTypeImpl implements UnionInterna
 		boolean changed = false;
 		for (DataTypeComponentImpl dtc : components) {
 			if (dtc.getDataType() == dt) {
-				int length = DataTypeComponent.usesZeroLengthComponent(dt) ? 0 : dt.getLength();
+				int length = getPreferredComponentLength(dt, dtc.getLength());
-				if (length >= 0 && length != dtc.getLength()) {
+				if (length != dtc.getLength()) {
 					dtc.setLength(length);
 					changed = true;
 				}
@@ -545,14 +537,10 @@ public class UnionDataType extends CompositeDataTypeImpl implements UnionInterna
 					remove = true;
 				}
 				else {
-					int len =
+					int len = getPreferredComponentLength(newDt, dtc.getLength());
-						DataTypeComponent.usesZeroLengthComponent(newDt) ? 0 : newDt.getLength();
-					if (len < 0) {
-						len = dtc.getLength();
-					}
 					oldDt.removeParent(this);
 					dtc.setLength(len);
-					dtc.setDataType(replacementDt); 
+					dtc.setDataType(replacementDt);
 					dtc.invalidateSettings();
 					replacementDt.addParent(this);
 					changed = true;
@@ -604,9 +592,7 @@ public class UnionDataType extends CompositeDataTypeImpl implements UnionInterna
 
 		UnionInternal union = (UnionInternal) dataType;
 
-		Iterator<DataTypeComponentImpl> it = components.iterator();
+		for (DataTypeComponent dtc : components) {
-		while (it.hasNext()) {
-			DataTypeComponent dtc = it.next();
 			dtc.getDataType().removeParent(this);
 		}
 		components.clear();