Merged revisions 708385,708996,709054,709217,709221,709235 via svnmerge from

https://svn.apache.org/repos/asf/poi/trunk

........
  r708385 | josh | 2008-10-27 16:44:44 -0700 (Mon, 27 Oct 2008) | 1 line
  
  Small fix for bug in RecordInputStream.readAllContinuedRemainder()  introduced in r707778.  It seems like only BiffViewer was affected.
........
  r708996 | josh | 2008-10-29 13:13:58 -0700 (Wed, 29 Oct 2008) | 1 line
  
  Allowed for quad-byte padding alignment on ObjRecord
........
  r709054 | josh | 2008-10-29 17:21:24 -0700 (Wed, 29 Oct 2008) | 1 line
  
  removed obsolete methods
........
  r709217 | josh | 2008-10-30 10:56:34 -0700 (Thu, 30 Oct 2008) | 1 line
  
  Fixed compiler warnings / simplified code
........
  r709221 | josh | 2008-10-30 11:33:35 -0700 (Thu, 30 Oct 2008) | 1 line
  
  Optimised slow test case (after reviewing original purpose)
........
  r709235 | josh | 2008-10-30 13:17:04 -0700 (Thu, 30 Oct 2008) | 1 line
  
  Fix for bug 15716 - - converted Ptg arrays into Formula objects to optimise memory usage
........


git-svn-id: https://svn.apache.org/repos/asf/poi/branches/ooxml@709262 13f79535-47bb-0310-9956-ffa450edef68

src/documentation/content/xdocs/changes.xml

@@ -37,6 +37,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.5-beta4" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">15716 - memory usage optimisation - converted Ptg arrays into Formula objects</action>
            <action dev="POI-DEVELOPERS" type="add">46065 - added implementation for VALUE function</action>
            <action dev="POI-DEVELOPERS" type="add">45966 - added implementation for FIND function</action>
            <action dev="POI-DEVELOPERS" type="fix">45778 - fixed ObjRecord to read ftLbsData properly</action>

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.5-beta4" date="2008-??-??">
+           <action dev="POI-DEVELOPERS" type="fix">15716 - memory usage optimisation - converted Ptg arrays into Formula objects</action>
            <action dev="POI-DEVELOPERS" type="add">46065 - added implementation for VALUE function</action>
            <action dev="POI-DEVELOPERS" type="add">45966 - added implementation for FIND function</action>
            <action dev="POI-DEVELOPERS" type="fix">45778 - fixed ObjRecord to read ftLbsData properly</action>

src/java/org/apache/poi/hssf/record/ArrayRecord.java

@@ -18,8 +18,9 @@
 package org.apache.poi.hssf.record;
 
 import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.ss.formula.Formula;
 import org.apache.poi.util.HexDump;
-import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.LittleEndianOutput;
 
 /**
  * ARRAY (0x0221)<p/>
@@ -36,14 +37,15 @@ public final class ArrayRecord extends SharedValueRecordBase {
 	
 	private int	_options;
 	private int _field3notUsed;
-	private Ptg[] _formulaTokens;
+	private Formula _formula;
 
 	public ArrayRecord(RecordInputStream in) {
 		super(in);
 		_options = in.readUShort();
 		_field3notUsed = in.readInt();
-		int formulaLen = in.readUShort();
+		int formulaTokenLen = in.readUShort();
-		_formulaTokens = Ptg.readTokens(formulaLen, in);
+		int totalFormulaLen = in.available();
+		_formula = Formula.read(formulaTokenLen, in, totalFormulaLen);
 	}
 
 	public boolean isAlwaysRecalculate() {
@@ -55,18 +57,12 @@ public final class ArrayRecord extends SharedValueRecordBase {
 
 	protected int getExtraDataSize() {
 		return 2 + 4
-			+ 2 + Ptg.getEncodedSize(_formulaTokens);
+			+ _formula.getEncodedSize();
 	}
-	protected void serializeExtraData(int offset, byte[] data) {
+	protected void serializeExtraData(LittleEndianOutput out) {
-		int pos = offset;
+		out.writeShort(_options);
-		LittleEndian.putUShort(data, pos, _options);
+		out.writeInt(_field3notUsed);
-		pos+=2;
+		_formula.serialize(out);
-		LittleEndian.putInt(data, pos, _field3notUsed);
-		pos+=4;
-		int tokenSize = Ptg.getEncodedSizeWithoutArrayData(_formulaTokens);
-		LittleEndian.putUShort(data, pos, tokenSize);
-		pos+=2;
-		Ptg.serializePtgs(_formulaTokens, data, pos);
 	}
 
 	public short getSid() {
@@ -80,8 +76,9 @@ public final class ArrayRecord extends SharedValueRecordBase {
 		sb.append(" options=").append(HexDump.shortToHex(_options)).append("\n");
 		sb.append(" notUsed=").append(HexDump.intToHex(_field3notUsed)).append("\n");
 		sb.append(" formula:").append("\n");
-		for (int i = 0; i < _formulaTokens.length; i++) {
+		Ptg[] ptgs = _formula.getTokens();
-			Ptg ptg = _formulaTokens[i];
+		for (int i = 0; i < ptgs.length; i++) {
+			Ptg ptg = ptgs[i];
 			sb.append(ptg.toString()).append(ptg.getRVAType()).append("\n");
 		}
 		sb.append("]");

src/java/org/apache/poi/hssf/record/CFRuleRecord.java

@@ -23,9 +23,10 @@ import org.apache.poi.hssf.record.cf.FontFormatting;
 import org.apache.poi.hssf.record.cf.PatternFormatting;
 import org.apache.poi.hssf.record.formula.Ptg;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+import org.apache.poi.ss.formula.Formula;
 import org.apache.poi.util.BitField;
 import org.apache.poi.util.BitFieldFactory;
-import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.LittleEndianByteArrayOutputStream;
 
 /**
  * Conditional Formatting Rule Record.
@@ -94,18 +95,12 @@ public final class CFRuleRecord extends Record {
 
 	private FontFormatting fontFormatting;
 
-	private byte  field_8_align_text_break;
-	private byte  field_9_align_text_rotation_angle;
-	private short field_10_align_indentation;
-	private short field_11_relative_indentation;
-	private short field_12_not_used;
-	
 	private BorderFormatting borderFormatting;
 
 	private PatternFormatting patternFormatting;
 	
-	private Ptg[] field_17_formula1;
+	private Formula field_17_formula1;
-	private Ptg[] field_18_formula2;
+	private Formula field_18_formula2;
 	
 	/** Creates new CFRuleRecord */
 	private CFRuleRecord(byte conditionType, byte comparisonOperation)
@@ -121,23 +116,18 @@ public final class CFRuleRecord extends Record {
 
 		field_6_not_used = (short)0x8002; // Excel seems to write this value, but it doesn't seem to care what it reads
 		fontFormatting=null;
-		field_8_align_text_break = 0;
-		field_9_align_text_rotation_angle = 0;
-		field_10_align_indentation = 0;
-		field_11_relative_indentation = 0;
-		field_12_not_used = 0;
 		borderFormatting=null;
 		patternFormatting=null;
-		field_17_formula1=null;
+		field_17_formula1=Formula.create(Ptg.EMPTY_PTG_ARRAY);
-		field_18_formula2=null;
+		field_18_formula2=Formula.create(Ptg.EMPTY_PTG_ARRAY);
 	}
 	
 	private CFRuleRecord(byte conditionType, byte comparisonOperation, Ptg[] formula1, Ptg[] formula2) {
 		this(conditionType, comparisonOperation); 
 		field_1_condition_type = CONDITION_TYPE_CELL_VALUE_IS;
 		field_2_comparison_operator = comparisonOperation;
-		field_17_formula1 = formula1;
+		field_17_formula1 = Formula.create(formula1);
-		field_18_formula2 = formula2;
+		field_18_formula2 = Formula.create(formula2);
 	}
 
 	/**
@@ -178,12 +168,9 @@ public final class CFRuleRecord extends Record {
 			patternFormatting = new PatternFormatting(in);
 		}
 
-		if (field_3_formula1_len > 0) {
+		// "You may not use unions, intersections or array constants in Conditional Formatting criteria"
-			field_17_formula1 = Ptg.readTokens(field_3_formula1_len, in);
+		field_17_formula1 = Formula.read(field_3_formula1_len, in);
-		}
+		field_18_formula2 = Formula.read(field_4_formula2_len, in);
-		if (field_4_formula2_len > 0) {
-			field_18_formula2 = Ptg.readTokens(field_4_formula2_len, in);
-		}
 	}
 
 	public byte getConditionType()
@@ -414,33 +401,22 @@ public final class CFRuleRecord extends Record {
 
 	public Ptg[] getParsedExpression1()
 	{
-		return field_17_formula1;
+		return field_17_formula1.getTokens();
 	}
 	public void setParsedExpression1(Ptg[] ptgs) {
-		field_17_formula1 = safeClone(ptgs);
+		field_17_formula1 = Formula.create(ptgs);
-	}
-	private static Ptg[] safeClone(Ptg[] ptgs) {
-		if (ptgs == null) {
-			return null;
-		}
-		return (Ptg[]) ptgs.clone();
 	}
 
 	/**
 	 * get the stack of the 2nd expression as a list
 	 *
-	 * @return list of tokens (casts stack to a list and returns it!)
+	 * @return array of {@link Ptg}s, possibly <code>null</code>
-	 * this method can return null is we are unable to create Ptgs from 
-	 *	 existing excel file
-	 * callers should check for null!
 	 */
-
+	public Ptg[] getParsedExpression2() {
-	public Ptg[] getParsedExpression2()
+		return Formula.getTokens(field_18_formula2);
-	{
-		return field_18_formula2;
 	}
 	public void setParsedExpression2(Ptg[] ptgs) {
-		field_18_formula2 = safeClone(ptgs);
+		field_18_formula2 = Formula.create(ptgs);
 	}
 
 	public short getSid()
@@ -449,14 +425,11 @@ public final class CFRuleRecord extends Record {
 	}
 
 	/**
-	 * @param ptgs may be <code>null</code>
+	 * @param ptgs must not be <code>null</code>
-	 * @return encoded size of the formula
+	 * @return encoded size of the formula tokens (does not include 2 bytes for ushort length)
 	 */
-	private static int getFormulaSize(Ptg[] ptgs) {
+	private static int getFormulaSize(Formula formula) {
-		if (ptgs == null) {
+		return formula.getEncodedTokenSize();
-			return 0;
-		}
-		return Ptg.getEncodedSize(ptgs);
 	}
 	
 	/**
@@ -468,51 +441,43 @@ public final class CFRuleRecord extends Record {
 	 * @param data byte array containing instance data
 	 * @return number of bytes written
 	 */
-	public int serialize(int pOffset, byte [] data)
+	public int serialize(int pOffset, byte [] data) {
-	{
 		
 		int formula1Len=getFormulaSize(field_17_formula1);
 		int formula2Len=getFormulaSize(field_18_formula2);
 		
-		int offset = pOffset;
 		int recordsize = getRecordSize();
-		LittleEndian.putShort(data, 0 + offset, sid);
-		LittleEndian.putShort(data, 2 + offset, (short)(recordsize-4));
-		data[4 + offset] = field_1_condition_type;
-		data[5 + offset] = field_2_comparison_operator;
-		LittleEndian.putUShort(data, 6 + offset, formula1Len);
-		LittleEndian.putUShort(data, 8 + offset, formula2Len);
-		LittleEndian.putInt(data,  10 + offset, field_5_options);
-		LittleEndian.putShort(data,14 + offset, field_6_not_used);
 		
-		offset += 16;
+		LittleEndianByteArrayOutputStream out = new LittleEndianByteArrayOutputStream(data, pOffset, recordsize);
 		
-		if( containsFontFormattingBlock() )
+		out.writeShort(sid);
-		{
+		out.writeShort(recordsize-4);
+		out.writeByte(field_1_condition_type);
+		out.writeByte(field_2_comparison_operator);
+		out.writeShort(formula1Len);
+		out.writeShort(formula2Len);
+		out.writeInt(field_5_options);
+		out.writeShort(field_6_not_used);
+		
+		if (containsFontFormattingBlock()) {
 			byte[] fontFormattingRawRecord  = fontFormatting.getRawRecord();
-			System.arraycopy(fontFormattingRawRecord, 0, data, offset, fontFormattingRawRecord.length);
+			out.write(fontFormattingRawRecord);
-			offset += fontFormattingRawRecord.length;
 		}
 		
-		if( containsBorderFormattingBlock())
+		if (containsBorderFormattingBlock()) {
-		{
+			borderFormatting.serialize(out);
-			offset += borderFormatting.serialize(offset, data);
 		}
 		
-		if( containsPatternFormattingBlock() )
+		if (containsPatternFormattingBlock()) {
-		{
+			patternFormatting.serialize(out);
-			offset += patternFormatting.serialize(offset, data);
 		}
 		
-		if (field_17_formula1 != null) {
+		field_17_formula1.serializeTokens(out);
-			offset += Ptg.serializePtgs(field_17_formula1, data, offset);
+		field_18_formula2.serializeTokens(out);
-		}
+		
-
+		if(out.getWriteIndex() - pOffset != recordsize) {
-		if (field_18_formula2 != null) {
+			throw new IllegalStateException("write mismatch (" 
-			offset += Ptg.serializePtgs(field_18_formula2, data, offset);
+					+ (out.getWriteIndex() - pOffset) + "!=" + recordsize + ")");
-		}
-		if(offset - pOffset != recordsize) {
-			throw new IllegalStateException("write mismatch (" + (offset - pOffset) + "!=" + recordsize + ")");
 		}
 		return recordsize;
 	}
@@ -531,25 +496,22 @@ public final class CFRuleRecord extends Record {
 	}
 
 
-	public String toString()
+	public String toString() {
-	{
 		StringBuffer buffer = new StringBuffer();
 		buffer.append("[CFRULE]\n");
 		buffer.append("    OPTION FLAGS=0x"+Integer.toHexString(getOptions()));
-		/*
+		if (false) {
-		if( containsFontFormattingBlock())
+			if (containsFontFormattingBlock()) {
-		{
+				buffer.append(fontFormatting.toString());
-			buffer.append(fontFormatting.toString());
+			}
+			if (containsBorderFormattingBlock()) {
+				buffer.append(borderFormatting.toString());
+			}
+			if (containsPatternFormattingBlock()) {
+				buffer.append(patternFormatting.toString());
+			}
+			buffer.append("[/CFRULE]\n");
 		}
-		if( containsBorderFormattingBlock())
-		{
-			buffer.append(borderFormatting.toString());
-		}
-		if( containsPatternFormattingBlock())
-		{
-			buffer.append(patternFormatting.toString());
-		}
-		buffer.append("[/CFRULE]\n");*/
 		return buffer.toString();
 	}
 	
@@ -566,12 +528,8 @@ public final class CFRuleRecord extends Record {
 		if (containsPatternFormattingBlock()) {
 			rec.patternFormatting = (PatternFormatting) patternFormatting.clone();
 		}
-		if (field_17_formula1 != null) {
+		rec.field_17_formula1 = field_17_formula1.copy();
-			rec.field_17_formula1 = (Ptg[]) field_17_formula1.clone();
+		rec.field_18_formula2 = field_17_formula1.copy();
-		}
-		if (field_18_formula2 != null) {
-			rec.field_18_formula2 = (Ptg[]) field_18_formula2.clone();
-		}
 
 		return rec;
 	}

src/java/org/apache/poi/hssf/record/DVRecord.java

@@ -16,14 +16,16 @@
 
 package org.apache.poi.hssf.record;
 
-import org.apache.poi.hssf.record.UnicodeString.UnicodeRecordStats;
 import org.apache.poi.hssf.record.formula.Ptg;
 import org.apache.poi.hssf.usermodel.DVConstraint;
 import org.apache.poi.hssf.usermodel.HSSFDataValidation;
 import org.apache.poi.ss.util.CellRangeAddress;
 import org.apache.poi.ss.util.CellRangeAddressList;
+import org.apache.poi.ss.formula.Formula;
 import org.apache.poi.util.BitField;
-import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.LittleEndianByteArrayOutputStream;
+import org.apache.poi.util.LittleEndianOutput;
+import org.apache.poi.util.StringUtil;
 
 /**
  * Title:        DATAVALIDATION Record (0x01BE)<p/>
@@ -53,11 +55,11 @@ public final class DVRecord extends Record {
 	/** Not used - Excel seems to always write 0x3FE0 */
 	private short _not_used_1 = 0x3FE0;
 	/** Formula data for first condition (RPN token array without size field) */
-	private Ptg[] _formula1;
+	private Formula _formula1;
 	/** Not used - Excel seems to always write 0x0000 */
 	private short _not_used_2 = 0x0000;
 	/** Formula data for second condition (RPN token array without size field) */
-	private Ptg[] _formula2;
+	private Formula _formula2;
 	/** Cell range address list with all affected ranges */
 	private CellRangeAddressList _regions;
 
@@ -96,34 +98,36 @@ public final class DVRecord extends Record {
 		_promptText = resolveTitleText(promptText);
 		_errorTitle = resolveTitleText(errorTitle);
 		_errorText = resolveTitleText(errorText);
-		_formula1 = formula1;
+		_formula1 = Formula.create(formula1);
-		_formula2 = formula2;
+		_formula2 = Formula.create(formula2);
 		_regions = regions;
 	}
 
 	public DVRecord(RecordInputStream in) {
-		
-	   _option_flags = in.readInt();
-	   
-	   _promptTitle = readUnicodeString(in);
-	   _errorTitle = readUnicodeString(in);
-	   _promptText = readUnicodeString(in);
-	   _errorText = readUnicodeString(in);
 
-	   int field_size_first_formula = in.readUShort(); 
+		_option_flags = in.readInt();
-	   _not_used_1 = in.readShort();
 
-	   //read first formula data condition
+		_promptTitle = readUnicodeString(in);
-	   _formula1 = Ptg.readTokens(field_size_first_formula, in);
+		_errorTitle = readUnicodeString(in);
+		_promptText = readUnicodeString(in);
+		_errorText = readUnicodeString(in);
 
-	   int field_size_sec_formula = in.readUShort(); 
+		int field_size_first_formula = in.readUShort();
-	   _not_used_2 = in.readShort();
+		_not_used_1 = in.readShort();
 
-	   //read sec formula data condition
+		// "You may not use unions, intersections or array constants in Data Validation criteria"
-	   _formula2 = Ptg.readTokens(field_size_sec_formula, in);
 
-	   //read cell range address list with all affected ranges
+		// read first formula data condition
-	   _regions = new org.apache.poi.hssf.util.CellRangeAddressList(in);
+		_formula1 = Formula.read(field_size_first_formula, in);
+
+		int field_size_sec_formula = in.readUShort();
+		_not_used_2 = in.readShort();
+
+		// read sec formula data condition
+		_formula2 = Formula.read(field_size_sec_formula, in);
+
+		// read cell range address list with all affected ranges
+		_regions = new CellRangeAddressList(in);
 	}
 
 	// --> start option flags
@@ -235,45 +239,43 @@ public final class DVRecord extends Record {
 		return str;
 	}
 
-	private void appendFormula(StringBuffer sb, String label, Ptg[] ptgs) {
+	private static void appendFormula(StringBuffer sb, String label, Formula f) {
 		sb.append(label);
-		if (ptgs.length < 1) {
+		
+		if (f == null) {
 			sb.append("<empty>\n");
 			return;
 		}
-		sb.append("\n");
+		Ptg[] ptgs = f.getTokens();
+		sb.append('\n');
 		for (int i = 0; i < ptgs.length; i++) {
 			sb.append('\t').append(ptgs[i].toString()).append('\n');
 		}
 	}
 
 	public int serialize(int offset, byte [] data) {
-		int size = this.getRecordSize();
+		int recSize = getRecordSize();
-		LittleEndian.putShort(data, 0 + offset, sid);
+		LittleEndianOutput out = new LittleEndianByteArrayOutputStream(data, offset, recSize);
-		LittleEndian.putShort(data, 2 + offset, ( short ) (size-4));
-
-		int pos = 4;
-		LittleEndian.putInt(data, pos + offset, _option_flags);
-		pos += 4;
 		
-		pos += serializeUnicodeString(_promptTitle, pos+offset, data);
+		out.writeShort(sid);
-		pos += serializeUnicodeString(_errorTitle, pos+offset, data);
+		out.writeShort(recSize-4);
-		pos += serializeUnicodeString(_promptText, pos+offset, data);
-		pos += serializeUnicodeString(_errorText, pos+offset, data);
-		LittleEndian.putUShort(data, offset+pos, Ptg.getEncodedSize(_formula1));
-		pos += 2;
-		LittleEndian.putUShort(data, offset+pos, _not_used_1);
-		pos += 2;
 
-		pos += Ptg.serializePtgs(_formula1, data, pos+offset);
+		out.writeInt(_option_flags);
-
+		
-		LittleEndian.putUShort(data, offset+pos, Ptg.getEncodedSize(_formula2));
+		serializeUnicodeString(_promptTitle, out);
-		pos += 2;
+		serializeUnicodeString(_errorTitle, out);
-		LittleEndian.putShort(data, offset+pos, _not_used_2);
+		serializeUnicodeString(_promptText, out);
-		pos += 2;
+		serializeUnicodeString(_errorText, out);
-		pos += Ptg.serializePtgs(_formula2, data, pos+offset);
+		out.writeShort(_formula1.getEncodedTokenSize());
-		_regions.serialize(pos+offset, data);
+		out.writeShort(_not_used_1);
-		return size;
+		_formula1.serializeTokens(out);
+		
+		out.writeShort(_formula2.getEncodedTokenSize());
+		out.writeShort(_not_used_2);
+		_formula2.serializeTokens(out);
+		
+		_regions.serialize(out);
+		return recSize;
 	}
 
 	/**
@@ -293,13 +295,12 @@ public final class DVRecord extends Record {
 		return new UnicodeString(in);
 	}
 
-	private static int serializeUnicodeString(UnicodeString us, int offset, byte[] data) {
+	private static void serializeUnicodeString(UnicodeString us, LittleEndianOutput out) {
-		UnicodeRecordStats urs = new UnicodeRecordStats();
+		StringUtil.writeUnicodeString(out, us.getString());
-		us.serialize(urs, offset, data);
-		return urs.recordSize;
 	}
-	private static int getUnicodeStringSize(UnicodeString str) {
+	private static int getUnicodeStringSize(UnicodeString us) {
-		return 3 + str.getString().length();
+		String str = us.getString();
+		return 3 + str.length() * (StringUtil.hasMultibyte(str) ? 2 : 1);
 	}
 
 	public int getRecordSize()  {
@@ -308,8 +309,8 @@ public final class DVRecord extends Record {
 		size += getUnicodeStringSize(_errorTitle);
 		size += getUnicodeStringSize(_promptText);
 		size += getUnicodeStringSize(_errorText);
-		size += Ptg.getEncodedSize(_formula1);
+		size += _formula1.getEncodedTokenSize();
-		size += Ptg.getEncodedSize(_formula2);
+		size += _formula2.getEncodedTokenSize();
 		size += _regions.getSize();
 		return size;
 	}

src/java/org/apache/poi/hssf/record/ExternalNameRecord.java

@@ -17,20 +17,19 @@
 
 package org.apache.poi.hssf.record;
 
-import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.ss.formula.Formula;
-import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.LittleEndianByteArrayOutputStream;
+import org.apache.poi.util.LittleEndianOutput;
 import org.apache.poi.util.StringUtil;
 
 /**
- * EXTERNALNAME<p/>
+ * EXTERNALNAME (0x0023)<p/>
  * 
  * @author Josh Micich
  */
 public final class ExternalNameRecord extends Record {
 
-	private static final Ptg[] EMPTY_PTG_ARRAY = { };
+	public final static short sid = 0x0023; // as per BIFF8. (some old versions used 0x223)
-
-	public final static short sid = 0x23; // as per BIFF8. (some old versions used 0x223)
 
 	private static final int OPT_BUILTIN_NAME          = 0x0001;
 	private static final int OPT_AUTOMATIC_LINK        = 0x0002; // m$ doc calls this fWantAdvise 
@@ -45,7 +44,7 @@ public final class ExternalNameRecord extends Record {
 	private short  field_2_index;
 	private short  field_3_not_used;
 	private String field_4_name;
-	private Ptg[]  field_5_name_definition;
+	private Formula  field_5_name_definition;
 
 	/**
 	 * Convenience Function to determine if the name is a built-in name
@@ -88,7 +87,7 @@ public final class ExternalNameRecord extends Record {
 		int result = 3 * 2  // 3 short fields
 			+ 2 + field_4_name.length(); // nameLen and name
 		if(hasFormula()) {
-			result += 2 + getNameDefinitionSize(); // nameDefLen and nameDef
+			result += field_5_name_definition.getEncodedSize();
 		}
 		return result;
 	}
@@ -104,28 +103,23 @@ public final class ExternalNameRecord extends Record {
 	 */
 	public int serialize( int offset, byte[] data ) {
 		int dataSize = getDataSize();
+		int recSize = dataSize + 4;
+		LittleEndianOutput out = new LittleEndianByteArrayOutputStream(data, offset, recSize);
 
-		LittleEndian.putShort( data, 0 + offset, sid );
+		out.writeShort(sid);
-		LittleEndian.putShort( data, 2 + offset, (short) dataSize );
+		out.writeShort(dataSize);
-		LittleEndian.putShort( data, 4 + offset, field_1_option_flag );
+		out.writeShort(field_1_option_flag);
-		LittleEndian.putShort( data, 6 + offset, field_2_index );
+		out.writeShort(field_2_index);
-		LittleEndian.putShort( data, 8 + offset, field_3_not_used );
+		out.writeShort(field_3_not_used);
 		int nameLen = field_4_name.length();
-		LittleEndian.putUShort( data, 10 + offset, nameLen );
+		out.writeShort(nameLen);
-		StringUtil.putCompressedUnicode( field_4_name, data, 12 + offset );
+		StringUtil.putCompressedUnicode(field_4_name, out);
-		if(hasFormula()) {
+		if (hasFormula()) {
-			int defLen = getNameDefinitionSize();
+			field_5_name_definition.serialize(out);
-			LittleEndian.putUShort( data, 12 + nameLen + offset, defLen );
-			Ptg.serializePtgs(field_5_name_definition, data, 14 + nameLen + offset );
 		}
-		return dataSize + 4;
+		return recSize;
 	}
 
-	private int getNameDefinitionSize() {
-		return Ptg.getEncodedSize(field_5_name_definition);
-	}
-
-
 	public int getRecordSize(){
 		return 4 + getDataSize();
 	}
@@ -141,14 +135,16 @@ public final class ExternalNameRecord extends Record {
 			if(in.remaining() > 0) {
 				throw readFail("Some unread data (is formula present?)");
 			}
-			field_5_name_definition = EMPTY_PTG_ARRAY;
+			field_5_name_definition = null;
 			return;
 		}
-		if(in.remaining() <= 0) {
+		int nBytesRemaining = in.available();
+		if(nBytesRemaining <= 0) {
 			throw readFail("Ran out of record data trying to read formula.");
 		}
-		short formulaLen = in.readShort();
+		int formulaLen = in.readUShort();
-		field_5_name_definition = Ptg.readTokens(formulaLen, in);
+		nBytesRemaining -=2;
+		field_5_name_definition = Formula.read(formulaLen, in, nBytesRemaining);
 	}
 	/*
 	 * Makes better error messages (while hasFormula() is not reliable) 
@@ -157,7 +153,7 @@ public final class ExternalNameRecord extends Record {
 	private RuntimeException readFail(String msg) {
 		String fullMsg = msg + " fields: (option=" + field_1_option_flag + " index=" + field_2_index 
 		+ " not_used=" + field_3_not_used + " name='" + field_4_name + "')";
-		return new RuntimeException(fullMsg);
+		return new RecordFormatException(fullMsg);
 	}
 
 	private boolean hasFormula() {

src/java/org/apache/poi/hssf/record/FormulaRecord.java

@@ -20,10 +20,13 @@ package org.apache.poi.hssf.record;
 import org.apache.poi.hssf.record.formula.Ptg;
 import org.apache.poi.hssf.record.formula.eval.ErrorEval;
 import org.apache.poi.hssf.usermodel.HSSFCell;
+import org.apache.poi.ss.formula.Formula;
 import org.apache.poi.util.BitField;
 import org.apache.poi.util.BitFieldFactory;
 import org.apache.poi.util.HexDump;
-import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.LittleEndianByteArrayOutputStream;
+import org.apache.poi.util.LittleEndianInput;
+import org.apache.poi.util.LittleEndianOutput;
 
 /**
  * Formula Record (0x0006).
@@ -35,7 +38,7 @@ import org.apache.poi.util.LittleEndian;
 public final class FormulaRecord extends Record implements CellValueRecordInterface {
 
 	public static final short sid = 0x0006;   // docs say 406...because of a bug Microsoft support site article #Q184647)
-	private static int FIXED_SIZE = 22;
+	private static int FIXED_SIZE = 20;
 
 	private static final BitField alwaysCalc = BitFieldFactory.getInstance(0x0001);
 	private static final BitField calcOnLoad = BitFieldFactory.getInstance(0x0002);
@@ -92,9 +95,9 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 			}
 			return new SpecialCachedValue(result);
 		}
-		public void serialize(byte[] data, int offset) {
+		public void serialize(LittleEndianOutput out) {
-			System.arraycopy(_variableData, 0, data, offset, VARIABLE_DATA_LENGTH);
+			out.write(_variableData);
-			LittleEndian.putUShort(data, offset+VARIABLE_DATA_LENGTH, 0xFFFF);
+			out.writeShort(0xFFFF);
 		}
 		public String formatDebugString() {
 			return formatValue() + ' ' + HexDump.toHex(_variableData);
@@ -172,8 +175,13 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 	private short  field_3_xf;
 	private double field_4_value;
 	private short  field_5_options;
-	private int    field_6_zero;
+	/**
-	private Ptg[]  field_8_parsed_expr;
+	 * Unused field.  As it turns out this field is often not zero..
+	 * According to Microsoft Excel Developer's Kit Page 318:
+	 * when writing the chn field (offset 20), it's supposed to be 0 but ignored on read
+	 */
+	private int field_6_zero;
+	private Formula field_8_parsed_expr;
 
 	/**
 	 * Since the NaN support seems sketchy (different constants) we'll store and spit it out directly
@@ -183,13 +191,14 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 	/** Creates new FormulaRecord */
 
 	public FormulaRecord() {
-		field_8_parsed_expr = Ptg.EMPTY_PTG_ARRAY;
+		field_8_parsed_expr = Formula.create(Ptg.EMPTY_PTG_ARRAY);
 	}
 
-	public FormulaRecord(RecordInputStream in) {
+	public FormulaRecord(RecordInputStream ris) {
-		field_1_row	 = in.readUShort();
+		LittleEndianInput in = ris;
-		field_2_column  = in.readShort();
+		field_1_row = in.readUShort();
-		field_3_xf	  = in.readShort();
+		field_2_column = in.readShort();
+		field_3_xf = in.readShort();
 		long valueLongBits  = in.readLong();
 		field_5_options = in.readShort();
 		specialCachedValue = SpecialCachedValue.create(valueLongBits);
@@ -197,14 +206,11 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 			field_4_value = Double.longBitsToDouble(valueLongBits);
 		}
 
-		field_6_zero		   = in.readInt();
+		field_6_zero = in.readInt();
+
 		int field_7_expression_len = in.readShort(); // this length does not include any extra array data
-		field_8_parsed_expr = Ptg.readTokens(field_7_expression_len, in);
+		int nBytesAvailable = in.available();
-		if (in.remaining() == 10) {
+		field_8_parsed_expr = Formula.read(field_7_expression_len, in, nBytesAvailable);
-			// TODO - this seems to occur when IntersectionPtg is present
-			// 10 extra bytes are just 0x01 and 0x00
-			// This causes POI stderr: "WARN. Unread 10 bytes of record 0x6"
-		}
 	}
 
 
@@ -336,11 +342,11 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 	 * @return the formula tokens. never <code>null</code>
 	 */
 	public Ptg[] getParsedExpression() {
-		return (Ptg[]) field_8_parsed_expr.clone();
+		return field_8_parsed_expr.getTokens();
 	}
 
 	public void setParsedExpression(Ptg[] ptgs) {
-		field_8_parsed_expr = ptgs;
+		field_8_parsed_expr = Formula.create(ptgs);
 	}
 
 	public short getSid() {
@@ -348,33 +354,30 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 	}
 
 	private int getDataSize() {
-		return FIXED_SIZE + Ptg.getEncodedSize(field_8_parsed_expr);
+		return FIXED_SIZE + field_8_parsed_expr.getEncodedSize();
 	}
 	public int serialize(int offset, byte [] data) {
 
 		int dataSize = getDataSize();
-
+		int recSize = 4 + dataSize;
-		LittleEndian.putShort(data, 0 + offset, sid);
+		LittleEndianOutput out = new LittleEndianByteArrayOutputStream(data, offset, recSize);
-		LittleEndian.putUShort(data, 2 + offset, dataSize);
+		out.writeShort(sid);
-		LittleEndian.putUShort(data, 4 + offset, getRow());
+		out.writeShort(dataSize);
-		LittleEndian.putShort(data, 6 + offset, getColumn());
+		out.writeShort(getRow());
-		LittleEndian.putShort(data, 8 + offset, getXFIndex());
+		out.writeShort(getColumn());
+		out.writeShort(getXFIndex());
 
 		if (specialCachedValue == null) {
-			LittleEndian.putDouble(data, 10 + offset, field_4_value);
+			out.writeDouble(field_4_value);
 		} else {
-			specialCachedValue.serialize(data, 10+offset);
+			specialCachedValue.serialize(out);
 		}
 
-		LittleEndian.putShort(data, 18 + offset, getOptions());
+		out.writeShort(getOptions());
 
-		//when writing the chn field (offset 20), it's supposed to be 0 but ignored on read
+		out.writeInt(field_6_zero); // may as well write original data back so as to minimise differences from original
-		//Microsoft Excel Developer's Kit Page 318
+		field_8_parsed_expr.serialize(out);
-		LittleEndian.putInt(data, 20 + offset, 0);
+		return recSize;
-		int formulaTokensSize = Ptg.getEncodedSizeWithoutArrayData(field_8_parsed_expr);
-		LittleEndian.putUShort(data, 24 + offset, formulaTokensSize);
-		Ptg.serializePtgs(field_8_parsed_expr, data, 26+offset);
-		return 4 + dataSize;
 	}
 
 	public int getRecordSize() {
@@ -385,24 +388,25 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 
 		StringBuffer sb = new StringBuffer();
 		sb.append("[FORMULA]\n");
-		sb.append("	.row	   = ").append(HexDump.shortToHex(getRow())).append("\n");
+		sb.append("  .row	   = ").append(HexDump.shortToHex(getRow())).append("\n");
-		sb.append("	.column	= ").append(HexDump.shortToHex(getColumn())).append("\n");
+		sb.append("  .column	= ").append(HexDump.shortToHex(getColumn())).append("\n");
-		sb.append("	.xf		= ").append(HexDump.shortToHex(getXFIndex())).append("\n");
+		sb.append("  .xf		= ").append(HexDump.shortToHex(getXFIndex())).append("\n");
-		sb.append("	.value	 = ");
+		sb.append("  .value	 = ");
 		if (specialCachedValue == null) {
 			sb.append(field_4_value).append("\n");
 		} else {
 			sb.append(specialCachedValue.formatDebugString()).append("\n");
 		}
-		sb.append("	.options   = ").append(HexDump.shortToHex(getOptions())).append("\n");
+		sb.append("  .options   = ").append(HexDump.shortToHex(getOptions())).append("\n");
-		sb.append("	.alwaysCalc= ").append(alwaysCalc.isSet(getOptions())).append("\n");
+		sb.append("    .alwaysCalc= ").append(isAlwaysCalc()).append("\n");
-		sb.append("	.calcOnLoad= ").append(calcOnLoad.isSet(getOptions())).append("\n");
+		sb.append("    .calcOnLoad= ").append(isCalcOnLoad()).append("\n");
-		sb.append("	.shared	= ").append(sharedFormula.isSet(getOptions())).append("\n");
+		sb.append("    .shared    = ").append(isSharedFormula()).append("\n");
-		sb.append("	.zero	  = ").append(HexDump.intToHex(field_6_zero)).append("\n");
+		sb.append("  .zero      = ").append(HexDump.intToHex(field_6_zero)).append("\n");
 
-		for (int k = 0; k < field_8_parsed_expr.length; k++ ) {
+		Ptg[] ptgs = field_8_parsed_expr.getTokens();
-			sb.append("	 Ptg[").append(k).append("]=");
+		for (int k = 0; k < ptgs.length; k++ ) {
-			Ptg ptg = field_8_parsed_expr[k];
+			sb.append("    Ptg[").append(k).append("]=");
+			Ptg ptg = ptgs[k];
 			sb.append(ptg.toString()).append(ptg.getRVAType()).append("\n");
 		}
 		sb.append("[/FORMULA]\n");
@@ -417,12 +421,7 @@ public final class FormulaRecord extends Record implements CellValueRecordInterf
 		rec.field_4_value = field_4_value;
 		rec.field_5_options = field_5_options;
 		rec.field_6_zero = field_6_zero;
-		int nTokens = field_8_parsed_expr.length;
+		rec.field_8_parsed_expr = field_8_parsed_expr;
-		Ptg[] ptgs = new Ptg[nTokens];
-		for (int i = 0; i < nTokens; i++) {
-			ptgs[i] = field_8_parsed_expr[i].copy();
-		}
-		rec.field_8_parsed_expr = ptgs;
 		rec.specialCachedValue = specialCachedValue;
 		return rec;
 	}

src/java/org/apache/poi/hssf/record/NameRecord.java

@@ -27,9 +27,11 @@ import org.apache.poi.hssf.record.formula.Ref3DPtg;
 import org.apache.poi.hssf.record.formula.UnionPtg;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
 import org.apache.poi.hssf.util.RangeAddress;
+import org.apache.poi.ss.formula.Formula;
 import org.apache.poi.ss.util.AreaReference;
 import org.apache.poi.util.HexDump;
-import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.LittleEndianByteArrayOutputStream;
+import org.apache.poi.util.LittleEndianInput;
 import org.apache.poi.util.StringUtil;
 
 /**
@@ -89,7 +91,7 @@ public final class NameRecord extends Record {
 	private boolean           field_11_nameIsMultibyte;
 	private byte              field_12_built_in_code;
 	private String            field_12_name_text;
-	private Ptg[]             field_13_name_definition;
+	private Formula           field_13_name_definition;
 	private String            field_14_custom_menu_text;
 	private String            field_15_description_text;
 	private String            field_16_help_topic_text;
@@ -98,7 +100,7 @@ public final class NameRecord extends Record {
 
 	/** Creates new NameRecord */
 	public NameRecord() {
-		field_13_name_definition = Ptg.EMPTY_PTG_ARRAY;
+		field_13_name_definition = Formula.create(Ptg.EMPTY_PTG_ARRAY);
 
 		field_12_name_text = "";
 		field_14_custom_menu_text = "";
@@ -245,7 +247,7 @@ public final class NameRecord extends Record {
 	 * @return <code>true</code> if name has a formula (named range or defined value)
 	 */
 	public boolean hasFormula() {
-		return field_1_option_flag == 0 && field_13_name_definition.length > 0;
+		return field_1_option_flag == 0 && field_13_name_definition.getEncodedTokenSize() > 0;
 	}
 
 	/**
@@ -296,11 +298,11 @@ public final class NameRecord extends Record {
 	 * @return the name formula. never <code>null</code>
 	 */
 	public Ptg[] getNameDefinition() {
-		return (Ptg[]) field_13_name_definition.clone();
+		return field_13_name_definition.getTokens();
 	}
 
 	public void setNameDefinition(Ptg[] ptgs) {
-		field_13_name_definition = (Ptg[]) ptgs.clone();
+		field_13_name_definition = Formula.create(ptgs);
 	}
 
 	/** get the custom menu text
@@ -346,9 +348,9 @@ public final class NameRecord extends Record {
 		int field_9_length_help_topic_text = field_16_help_topic_text.length();
 		int field_10_length_status_bar_text = field_17_status_bar_text.length();
 		int rawNameSize = getNameRawSize();
-		
+
-		int formulaTotalSize = Ptg.getEncodedSize(field_13_name_definition);
+		int formulaTotalSize = field_13_name_definition.getEncodedSize();
-		int dataSize = 15 // 4 shorts + 7 bytes
+		int dataSize = 13 // 3 shorts + 7 bytes
 			+ rawNameSize
 			+ field_7_length_custom_menu
 			+ field_8_length_description_text
@@ -356,48 +358,45 @@ public final class NameRecord extends Record {
 			+ field_10_length_status_bar_text
 			+ formulaTotalSize;
 		
-		LittleEndian.putShort(data, 0 + offset, sid);
+		int recSize = 4 + dataSize;
-		LittleEndian.putUShort(data, 2 + offset, dataSize);
+		LittleEndianByteArrayOutputStream out = new LittleEndianByteArrayOutputStream(data, offset, recSize);
+		
+		out.writeShort(sid);
+		out.writeShort(dataSize);
 		// size defined below
-		LittleEndian.putShort(data, 4 + offset, getOptionFlag());
+		out.writeShort(getOptionFlag());
-		LittleEndian.putByte(data, 6 + offset, getKeyboardShortcut());
+		out.writeByte(getKeyboardShortcut());
-		LittleEndian.putByte(data, 7 + offset, getNameTextLength());
+		out.writeByte(getNameTextLength());
-		// Note -
+		// Note - formula size is not immediately before encoded formula, and does not include any array constant data
-		LittleEndian.putUShort(data, 8 + offset, Ptg.getEncodedSizeWithoutArrayData(field_13_name_definition));
+		out.writeShort(field_13_name_definition.getEncodedTokenSize());
-		LittleEndian.putUShort(data, 10 + offset, field_5_externSheetIndex_plus1);
+		out.writeShort(field_5_externSheetIndex_plus1);
-		LittleEndian.putUShort(data, 12 + offset, field_6_sheetNumber);
+		out.writeShort(field_6_sheetNumber);
-		LittleEndian.putByte(data, 14 + offset, field_7_length_custom_menu);
+		out.writeByte(field_7_length_custom_menu);
-		LittleEndian.putByte(data, 15 + offset, field_8_length_description_text);
+		out.writeByte(field_8_length_description_text);
-		LittleEndian.putByte(data, 16 + offset, field_9_length_help_topic_text);
+		out.writeByte(field_9_length_help_topic_text);
-		LittleEndian.putByte(data, 17 + offset, field_10_length_status_bar_text);
+		out.writeByte(field_10_length_status_bar_text);
-		LittleEndian.putByte(data, 18 + offset, field_11_nameIsMultibyte ? 1 : 0);
+		out.writeByte(field_11_nameIsMultibyte ? 1 : 0);
-		int pos = 19 + offset;
 
 		if (isBuiltInName()) {
 			//can send the builtin name directly in
-			LittleEndian.putByte(data, pos,  field_12_built_in_code);
+			out.writeByte(field_12_built_in_code);
 		} else {
 			String nameText = field_12_name_text;
 			if (field_11_nameIsMultibyte) {
-    			StringUtil.putUnicodeLE(nameText, data, pos);
+				StringUtil.putUnicodeLE(nameText, out);
-     		} else {
+			} else {
-    			StringUtil.putCompressedUnicode(nameText, data, pos);
+				StringUtil.putCompressedUnicode(nameText, out);
-    		}
+			}
 		}
-		pos += rawNameSize;
+		field_13_name_definition.serializeTokens(out);
-
+		field_13_name_definition.serializeArrayConstantData(out);
-		Ptg.serializePtgs(field_13_name_definition,  data, pos);
-		pos += formulaTotalSize;
 		
-		StringUtil.putCompressedUnicode( getCustomMenuText(), data, pos);
+		StringUtil.putCompressedUnicode( getCustomMenuText(), out);
-		pos += field_7_length_custom_menu;
+		StringUtil.putCompressedUnicode( getDescriptionText(), out);
-		StringUtil.putCompressedUnicode( getDescriptionText(), data, pos);
+		StringUtil.putCompressedUnicode( getHelpTopicText(), out);
-		pos += field_8_length_description_text;
+		StringUtil.putCompressedUnicode( getStatusBarText(), out);
-		StringUtil.putCompressedUnicode( getHelpTopicText(), data, pos);
-		pos += field_9_length_help_topic_text;
-		StringUtil.putCompressedUnicode( getStatusBarText(), data, pos);
 
-		return 4 + dataSize;
+		return recSize;
 	}
 	private int getNameRawSize() {
 		if (isBuiltInName()) {
@@ -412,23 +411,23 @@ public final class NameRecord extends Record {
 
 	public int getRecordSize(){
 		return 4 // sid + size
-			+ 15 // 4 shorts + 7 bytes
+			+ 13 // 3 shorts + 7 bytes
 			+ getNameRawSize()
 			+ field_14_custom_menu_text.length()
 			+ field_15_description_text.length()
 			+ field_16_help_topic_text.length()
 			+ field_17_status_bar_text.length()
-			+ Ptg.getEncodedSize(field_13_name_definition);
+			+ field_13_name_definition.getEncodedSize();
 	}
 
 	/** gets the extern sheet number
 	 * @return extern sheet index
 	 */
 	public int getExternSheetNumber(){
-		if (field_13_name_definition.length < 1) {
+		if (field_13_name_definition.getEncodedSize() < 1) {
 			return 0;
 		}
-		Ptg ptg = field_13_name_definition[0];
+		Ptg ptg = field_13_name_definition.getTokens()[0];
 
 		if (ptg.getClass() == Area3DPtg.class){
 			return ((Area3DPtg) ptg).getExternSheetIndex();
@@ -444,15 +443,14 @@ public final class NameRecord extends Record {
 	 * @param externSheetNumber extern sheet number
 	 */
 	public void setExternSheetNumber(short externSheetNumber){
+		Ptg[] ptgs = field_13_name_definition.getTokens();
 		Ptg ptg;
 
-		if (field_13_name_definition.length < 1){
+		if (ptgs.length < 1){
 			ptg = createNewPtg();
-			field_13_name_definition = new Ptg[] {
+			ptgs = new Ptg[] { ptg, };
-				ptg,
-			};
 		} else {
-			ptg = field_13_name_definition[0];
+			ptg = ptgs[0];
 		}
 
 		if (ptg.getClass() == Area3DPtg.class){
@@ -461,7 +459,7 @@ public final class NameRecord extends Record {
 		} else if (ptg.getClass() == Ref3DPtg.class){
 			((Ref3DPtg) ptg).setExternSheetIndex(externSheetNumber);
 		}
-
+		field_13_name_definition = Formula.create(ptgs);
 	}
 
 	private static Ptg createNewPtg(){
@@ -472,7 +470,7 @@ public final class NameRecord extends Record {
 	 * @return area reference
 	 */
 	public String getAreaReference(HSSFWorkbook book){
-		return HSSFFormulaParser.toFormulaString(book, field_13_name_definition);
+		return HSSFFormulaParser.toFormulaString(book, field_13_name_definition.getTokens());
 	}
 
 	/** sets the reference , the area only (range)
@@ -483,11 +481,11 @@ public final class NameRecord extends Record {
 		RangeAddress ra = new RangeAddress(ref);
 		Ptg oldPtg;
 
-		if (field_13_name_definition.length < 1){
+		if (field_13_name_definition.getEncodedTokenSize() < 1){
 			oldPtg = createNewPtg();
 		} else {
 			//Trying to find extern sheet index
-			oldPtg = field_13_name_definition[0];
+			oldPtg = field_13_name_definition.getTokens()[0];
 		}
 		List temp = new ArrayList();
 		int externSheetIndex = 0;
@@ -519,7 +517,7 @@ public final class NameRecord extends Record {
 		}
 		Ptg[] ptgs = new Ptg[temp.size()];
 		temp.toArray(ptgs);
-		field_13_name_definition = ptgs;
+		field_13_name_definition = Formula.create(ptgs);
 	}
 
 	/**
@@ -528,17 +526,18 @@ public final class NameRecord extends Record {
 	 *
 	 * @param in the RecordInputstream to read the record from
 	 */
-	public NameRecord(RecordInputStream in) {
+	public NameRecord(RecordInputStream ris) {
+		LittleEndianInput in = ris;
 		field_1_option_flag                 = in.readShort();
 		field_2_keyboard_shortcut           = in.readByte();
 		int field_3_length_name_text        = in.readByte();
 		int field_4_length_name_definition  = in.readShort();
 		field_5_externSheetIndex_plus1      = in.readShort();
 		field_6_sheetNumber                 = in.readUShort();
-		int field_7_length_custom_menu      = in.readUByte();
+		int f7_customMenuLen      = in.readUByte();
-		int field_8_length_description_text = in.readUByte();
+		int f8_descriptionTextLen = in.readUByte();
-		int field_9_length_help_topic_text  = in.readUByte();
+		int f9_helpTopicTextLen  = in.readUByte();
-		int field_10_length_status_bar_text = in.readUByte();
+		int f10_statusBarTextLen = in.readUByte();
 
 		//store the name in byte form if it's a built-in name
 		field_11_nameIsMultibyte = (in.readByte() != 0);
@@ -546,19 +545,21 @@ public final class NameRecord extends Record {
 			field_12_built_in_code = in.readByte();
 		} else {
 			if (field_11_nameIsMultibyte) {
-				field_12_name_text = in.readUnicodeLEString(field_3_length_name_text);
+				field_12_name_text = StringUtil.readUnicodeLE(in, field_3_length_name_text);
 			} else {
-				field_12_name_text = in.readCompressedUnicode(field_3_length_name_text);
+				field_12_name_text = StringUtil.readCompressedUnicode(in, field_3_length_name_text);
 			}
 		}
 
-		field_13_name_definition = Ptg.readTokens(field_4_length_name_definition, in);
+		int nBytesAvailable = in.available() - (f7_customMenuLen 
+				+ f8_descriptionTextLen + f9_helpTopicTextLen + f10_statusBarTextLen);
+		field_13_name_definition = Formula.read(field_4_length_name_definition, in, nBytesAvailable);
 
 		//Who says that this can only ever be compressed unicode???
-		field_14_custom_menu_text = in.readCompressedUnicode(field_7_length_custom_menu);
+		field_14_custom_menu_text = StringUtil.readCompressedUnicode(in, f7_customMenuLen);
-		field_15_description_text = in.readCompressedUnicode(field_8_length_description_text);
+		field_15_description_text = StringUtil.readCompressedUnicode(in, f8_descriptionTextLen);
-		field_16_help_topic_text  = in.readCompressedUnicode(field_9_length_help_topic_text);
+		field_16_help_topic_text  = StringUtil.readCompressedUnicode(in, f9_helpTopicTextLen);
-		field_17_status_bar_text  = in.readCompressedUnicode(field_10_length_status_bar_text);
+		field_17_status_bar_text  = StringUtil.readCompressedUnicode(in, f10_statusBarTextLen);
 	}
 
 	/**
@@ -633,9 +634,10 @@ public final class NameRecord extends Record {
 		sb.append("    .Status bar text length = ").append(field_17_status_bar_text.length()).append("\n");
 		sb.append("    .NameIsMultibyte        = ").append(field_11_nameIsMultibyte).append("\n");
 		sb.append("    .Name (Unicode text)    = ").append( getNameText() ).append("\n");
-		sb.append("    .Formula (nTokens=").append(field_13_name_definition.length).append("):") .append("\n");
+		Ptg[] ptgs = field_13_name_definition.getTokens();
-		for (int i = 0; i < field_13_name_definition.length; i++) {
+		sb.append("    .Formula (nTokens=").append(ptgs.length).append("):") .append("\n");
-			Ptg ptg = field_13_name_definition[i];
+		for (int i = 0; i < ptgs.length; i++) {
+			Ptg ptg = ptgs[i];
 			sb.append("       " + ptg.toString()).append(ptg.getRVAType()).append("\n");
 		}
 

src/java/org/apache/poi/hssf/record/ObjRecord.java

@@ -18,15 +18,13 @@
 package org.apache.poi.hssf.record;
 
 import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
 import java.util.ArrayList;
 import java.util.List;
 
 import org.apache.poi.util.HexDump;
 import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.LittleEndianByteArrayOutputStream;
 import org.apache.poi.util.LittleEndianInputStream;
-import org.apache.poi.util.LittleEndianOutput;
-import org.apache.poi.util.LittleEndianOutputStream;
 
 /**
  * OBJRECORD (0x005D)<p/>
@@ -37,10 +35,17 @@ import org.apache.poi.util.LittleEndianOutputStream;
  */
 public final class ObjRecord extends Record {
 	public final static short sid = 0x005D;
+
+	private static final int NORMAL_PAD_ALIGNMENT = 2;
+	private static int MAX_PAD_ALIGNMENT = 4;
 	
 	private List subrecords;
 	/** used when POI has no idea what is going on */
 	private byte[] _uninterpretedData;
+	/**
+	 * Excel seems to tolerate padding to quad or double byte length
+	 */
+	private boolean _isPaddedToQuadByteMultiple;
 
 	//00000000 15 00 12 00 01 00 01 00 11 60 00 00 00 00 00 0D .........`......
 	//00000010 26 01 00 00 00 00 00 00 00 00                   &.........
@@ -71,6 +76,10 @@ public final class ObjRecord extends Record {
 			_uninterpretedData = subRecordData;
 			return;
 		}
+		if (subRecordData.length % 2 != 0) {
+			String msg = "Unexpected length of subRecordData : " + HexDump.toHex(subRecordData);
+			throw new RecordFormatException(msg);
+		}
 
 //		System.out.println(HexDump.toHex(subRecordData));
 
@@ -84,12 +93,17 @@ public final class ObjRecord extends Record {
 				break;
 			}
 		}
-		if (bais.available() > 0) {
+		int nRemainingBytes = bais.available();
-			// earlier versions of the code had allowances for padding
+		if (nRemainingBytes > 0) {
-			// At present (Oct-2008), no unit test samples exhibit such padding
+			// At present (Oct-2008), most unit test samples have (subRecordData.length % 2 == 0)
-			String msg = "Leftover " + bais.available() 
+			_isPaddedToQuadByteMultiple = subRecordData.length % MAX_PAD_ALIGNMENT == 0;
+			if (nRemainingBytes >= (_isPaddedToQuadByteMultiple ? MAX_PAD_ALIGNMENT : NORMAL_PAD_ALIGNMENT)) {
+				String msg = "Leftover " + nRemainingBytes 
 				+ " bytes in subrecord data " + HexDump.toHex(subRecordData);
-			throw new RecordFormatException(msg);
+				throw new RecordFormatException(msg);
+			}
+		} else {
+			_isPaddedToQuadByteMultiple = false;
 		}
 	}
 
@@ -114,34 +128,41 @@ public final class ObjRecord extends Record {
 			SubRecord record = (SubRecord) subrecords.get(i);
 			size += record.getDataSize()+4;
 		}
+		if (_isPaddedToQuadByteMultiple) {
+			while (size % MAX_PAD_ALIGNMENT != 0) {
+				size++;
+			}
+		} else {
+			while (size % NORMAL_PAD_ALIGNMENT != 0) {
+				size++;
+			}
+		}
 		return size;
 	}
 
 	public int serialize(int offset, byte[] data) {
 		int dataSize = getDataSize();
+		int recSize = 4 + dataSize;
+		LittleEndianByteArrayOutputStream out = new LittleEndianByteArrayOutputStream(data, offset, recSize);
 
-		LittleEndian.putUShort(data, 0 + offset, sid);
+		out.writeShort(sid);
-		LittleEndian.putUShort(data, 2 + offset, dataSize);
+		out.writeShort(dataSize);
 
-		byte[] subRecordBytes;
 		if (_uninterpretedData == null) {
-			ByteArrayOutputStream baos = new ByteArrayOutputStream(dataSize);
-			LittleEndianOutput leo = new LittleEndianOutputStream(baos);
 
 			for (int i = 0; i < subrecords.size(); i++) {
 				SubRecord record = (SubRecord) subrecords.get(i);
-				record.serialize(leo);
+				record.serialize(out);
 			}
+			int expectedEndIx = offset+dataSize;
 			// padding
-			while (baos.size() < dataSize) {
+			while (out.getWriteIndex() < expectedEndIx) {
-				baos.write(0);
+				out.writeByte(0);
 			}
-			subRecordBytes = baos.toByteArray();
 		} else {
-			subRecordBytes = _uninterpretedData;
+			out.write(_uninterpretedData);
 		}
-		System.arraycopy(subRecordBytes, 0, data, offset + 4, dataSize);
+		return recSize;
-		return 4 + dataSize;
 	}
 
 	public int getRecordSize() {

src/java/org/apache/poi/hssf/record/RecordInputStream.java

@@ -355,14 +355,14 @@ public final class RecordInputStream extends InputStream implements LittleEndian
     //growable array of the data.
     ByteArrayOutputStream out = new ByteArrayOutputStream(2*MAX_RECORD_DATA_SIZE);
 
-    while (isContinueNext()) {
+    while (true) {
       byte[] b = readRemainder();
       out.write(b, 0, b.length);
+      if (!isContinueNext()) {
+          break;
+      }
       nextRecord();
     }
-    byte[] b = readRemainder();
-    out.write(b, 0, b.length);
-
     return out.toByteArray();
   }
 

src/java/org/apache/poi/hssf/record/SharedFormulaRecord.java

@@ -22,7 +22,10 @@ import org.apache.poi.hssf.record.formula.AreaPtg;
 import org.apache.poi.hssf.record.formula.Ptg;
 import org.apache.poi.hssf.record.formula.RefNPtg;
 import org.apache.poi.hssf.record.formula.RefPtg;
+import org.apache.poi.hssf.util.CellRangeAddress8Bit;
+import org.apache.poi.ss.formula.Formula;
 import org.apache.poi.util.HexDump;
+import org.apache.poi.util.LittleEndianOutput;
 
 /**
  * Title:        SHAREDFMLA (0x04BC) SharedFormulaRecord
@@ -39,10 +42,15 @@ public final class SharedFormulaRecord extends SharedValueRecordBase {
     public final static short   sid = 0x04BC;
 
     private int field_5_reserved;
-    private Ptg[] field_7_parsed_expr;
+    private Formula field_7_parsed_expr;
 
+    // for testing only
     public SharedFormulaRecord() {
-        field_7_parsed_expr = Ptg.EMPTY_PTG_ARRAY;
+        this(new CellRangeAddress8Bit(0,0,0,0));
+    }
+    private SharedFormulaRecord(CellRangeAddress8Bit range) {
+        super(range);
+        field_7_parsed_expr = Formula.create(Ptg.EMPTY_PTG_ARRAY);
     }
 
     /**
@@ -52,17 +60,17 @@ public final class SharedFormulaRecord extends SharedValueRecordBase {
         super(in);
         field_5_reserved        = in.readShort();
         int field_6_expression_len = in.readShort();
-        field_7_parsed_expr = Ptg.readTokens(field_6_expression_len, in);
+        int nAvailableBytes = in.available();
+        field_7_parsed_expr = Formula.read(field_6_expression_len, in, nAvailableBytes);
     }
-    protected void serializeExtraData(int offset, byte[] data) {
+
-        //Because this record is converted to individual Formula records, this method is not required.
+    protected void serializeExtraData(LittleEndianOutput out) {
-        throw new UnsupportedOperationException("Cannot serialize a SharedFormulaRecord");
+        out.writeShort(field_5_reserved);
+        field_7_parsed_expr.serialize(out);
     }
     
     protected int getExtraDataSize() {
-        //Because this record is converted to individual Formula records, this method is not required.
+        return 2 + field_7_parsed_expr.getEncodedSize();
-        throw new UnsupportedOperationException("Cannot get the size for a SharedFormulaRecord");
-
     }
 
     /**
@@ -77,9 +85,10 @@ public final class SharedFormulaRecord extends SharedValueRecordBase {
         buffer.append("    .range      = ").append(getRange().toString()).append("\n");
         buffer.append("    .reserved    = ").append(HexDump.shortToHex(field_5_reserved)).append("\n");
 
-        for (int k = 0; k < field_7_parsed_expr.length; k++ ) {
+        Ptg[] ptgs = field_7_parsed_expr.getTokens();
+        for (int k = 0; k < ptgs.length; k++ ) {
            buffer.append("Formula[").append(k).append("]");
-           Ptg ptg = field_7_parsed_expr[k];
+           Ptg ptg = ptgs[k];
            buffer.append(ptg.toString()).append(ptg.getRVAType()).append("\n");
         }
 
@@ -92,20 +101,13 @@ public final class SharedFormulaRecord extends SharedValueRecordBase {
     }
 
     /**
-     * Creates a non shared formula from the shared formula
+     * Creates a non shared formula from the shared formula counterpart<br/>
-     * counter part
+     * 
+     * Perhaps this functionality could be implemented in terms of the raw 
+     * byte array inside {@link Formula}.
      */
-    protected static Ptg[] convertSharedFormulas(Ptg[] ptgs, int formulaRow, int formulaColumn) {
+    static Ptg[] convertSharedFormulas(Ptg[] ptgs, int formulaRow, int formulaColumn) {
-        if(false) {
+
-            /*
-             * TODO - (May-2008) Stop converting relative ref Ptgs in shared formula records.
-             * If/when POI writes out the workbook, this conversion makes an unnecessary diff in the BIFF records.
-             * Disabling this code breaks one existing junit.
-             * Some fix-up will be required to make Ptg.toFormulaString(HSSFWorkbook) work properly.
-             * That method will need 2 extra params: rowIx and colIx.
-             */
-            return ptgs;
-        }
         Ptg[] newPtgStack = new Ptg[ptgs.length];
 
         for (int k = 0; k < ptgs.length; k++) {
@@ -145,10 +147,9 @@ public final class SharedFormulaRecord extends SharedValueRecordBase {
     }
 
     /**
-     * Creates a non shared formula from the shared formula
+     * @return the equivalent {@link Ptg} array that the formula would have, were it not shared.
-     * counter part
      */
-    public void convertSharedFormulaRecord(FormulaRecord formula) {
+    public Ptg[] getFormulaTokens(FormulaRecord formula) {
         int formulaRow = formula.getRow();
         int formulaColumn = formula.getColumn();
         //Sanity checks
@@ -156,10 +157,7 @@ public final class SharedFormulaRecord extends SharedValueRecordBase {
             throw new RuntimeException("Shared Formula Conversion: Coding Error");
         }
 
-        Ptg[] ptgs = convertSharedFormulas(field_7_parsed_expr, formulaRow, formulaColumn);
+        return convertSharedFormulas(field_7_parsed_expr.getTokens(), formulaRow, formulaColumn);
-        formula.setParsedExpression(ptgs);
-        //Now its not shared!
-        formula.setSharedFormula(false);
     }
 
     private static int fixupRelativeColumn(int currentcolumn, int column, boolean relative) {
@@ -179,7 +177,9 @@ public final class SharedFormulaRecord extends SharedValueRecordBase {
     }
 
     public Object clone() {
-        //Because this record is converted to individual Formula records, this method is not required.
+        SharedFormulaRecord result = new SharedFormulaRecord(getRange());
-        throw new UnsupportedOperationException("Cannot clone a SharedFormulaRecord");
+        result.field_5_reserved = field_5_reserved;
+        result.field_7_parsed_expr = field_7_parsed_expr.copy();
+        return result;
     }
 }

src/java/org/apache/poi/hssf/record/SharedValueRecordBase.java

@@ -18,7 +18,9 @@
 package org.apache.poi.hssf.record;
 
 import org.apache.poi.hssf.util.CellRangeAddress8Bit;
-import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.LittleEndianByteArrayOutputStream;
+import org.apache.poi.util.LittleEndianInput;
+import org.apache.poi.util.LittleEndianOutput;
 
 /**
  * Common base class for {@link SharedFormulaRecord}, {@link ArrayRecord} and
@@ -41,7 +43,7 @@ public abstract class SharedValueRecordBase extends Record {
 	/**
 	 * reads only the range (1 {@link CellRangeAddress8Bit}) from the stream
 	 */
-	public SharedValueRecordBase(RecordInputStream in) {
+	public SharedValueRecordBase(LittleEndianInput in) {
 		_range = new CellRangeAddress8Bit(in);
 	}
 
@@ -71,19 +73,19 @@ public abstract class SharedValueRecordBase extends Record {
 
 	protected abstract int getExtraDataSize();
 
-	protected abstract void serializeExtraData(int offset, byte[] data);
+	protected abstract void serializeExtraData(LittleEndianOutput out);
 
 	public final int serialize(int offset, byte[] data) {
 		int dataSize = CellRangeAddress8Bit.ENCODED_SIZE + getExtraDataSize();
+		
+		int totalRecSize = dataSize + 4;
+		LittleEndianOutput out = new LittleEndianByteArrayOutputStream(data, offset, totalRecSize);
+		out.writeShort(getSid());
+		out.writeShort(dataSize);
 
-		LittleEndian.putShort(data, 0 + offset, getSid());
+		_range.serialize(out);
-		LittleEndian.putUShort(data, 2 + offset, dataSize);
+		serializeExtraData(out);
-
+		return totalRecSize;
-		int pos = offset + 4;
-		_range.serialize(pos, data);
-		pos += CellRangeAddress8Bit.ENCODED_SIZE;
-		serializeExtraData(pos, data);
-		return dataSize + 4;
 	}
 
 	/**

src/java/org/apache/poi/hssf/record/TableRecord.java

@@ -23,7 +23,7 @@ import org.apache.poi.hssf.util.CellReference;
 import org.apache.poi.util.BitField;
 import org.apache.poi.util.BitFieldFactory;
 import org.apache.poi.util.HexDump;
-import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.LittleEndianOutput;
 /**
  * DATATABLE (0x0236)<p/>
  *
@@ -146,13 +146,13 @@ public final class TableRecord extends SharedValueRecordBase {
 		2 // 2 byte fields
 		+ 8; // 4 short fields
 	}
-	protected void serializeExtraData(int offset, byte[] data) {
+	protected void serializeExtraData(LittleEndianOutput out) {
-		LittleEndian.putByte(data, 0 + offset, field_5_flags);
+		out.writeByte(field_5_flags);
-		LittleEndian.putByte(data, 1 + offset, field_6_res);
+		out.writeByte(field_6_res);
-		LittleEndian.putUShort(data, 2 + offset, field_7_rowInputRow);
+		out.writeShort(field_7_rowInputRow);
-		LittleEndian.putUShort(data, 4 + offset, field_8_colInputRow);
+		out.writeShort(field_8_colInputRow);
-		LittleEndian.putUShort(data, 6 + offset, field_9_rowInputCol);
+		out.writeShort(field_9_rowInputCol);
-		LittleEndian.putUShort(data, 8 + offset, field_10_colInputCol);
+		out.writeShort(field_10_colInputCol);
 	}
 
 	public String toString() {

src/java/org/apache/poi/hssf/record/aggregates/FormulaRecordAggregate.java

@@ -21,7 +21,10 @@ import org.apache.poi.hssf.record.CellValueRecordInterface;
 import org.apache.poi.hssf.record.FormulaRecord;
 import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.RecordFormatException;
+import org.apache.poi.hssf.record.SharedFormulaRecord;
 import org.apache.poi.hssf.record.StringRecord;
+import org.apache.poi.hssf.record.formula.ExpPtg;
+import org.apache.poi.hssf.record.formula.Ptg;
 
 /**
  * The formula record aggregate is used to join together the formula record and it's
@@ -31,113 +34,177 @@ import org.apache.poi.hssf.record.StringRecord;
  */
 public final class FormulaRecordAggregate extends RecordAggregate implements CellValueRecordInterface {
 
-    private final FormulaRecord _formulaRecord;
+	private final FormulaRecord _formulaRecord;
-    private SharedValueManager _sharedValueManager;
+	private SharedValueManager _sharedValueManager;
-    /** caches the calculated result of the formula */
+	/** caches the calculated result of the formula */
-    private StringRecord _stringRecord;
+	private StringRecord _stringRecord;
+	private SharedFormulaRecord _sharedFormulaRecord;
 
-    /**
+	/**
-     * @param stringRec may be <code>null</code> if this formula does not have a cached text
+	 * @param stringRec may be <code>null</code> if this formula does not have a cached text
-     * value.
+	 * value.
-     * @param svm the {@link SharedValueManager} for the current sheet
+	 * @param svm the {@link SharedValueManager} for the current sheet
-     */
+	 */
-    public FormulaRecordAggregate(FormulaRecord formulaRec, StringRecord stringRec, SharedValueManager svm) {
+	public FormulaRecordAggregate(FormulaRecord formulaRec, StringRecord stringRec, SharedValueManager svm) {
-        if (svm == null) {
+		if (svm == null) {
-            throw new IllegalArgumentException("sfm must not be null");
+			throw new IllegalArgumentException("sfm must not be null");
-        }
+		}
-        boolean hasStringRec = stringRec != null;
+		boolean hasStringRec = stringRec != null;
-        boolean hasCachedStringFlag = formulaRec.hasCachedResultString();
+		boolean hasCachedStringFlag = formulaRec.hasCachedResultString();
-        if (hasStringRec != hasCachedStringFlag) {
+		if (hasStringRec != hasCachedStringFlag) {
-            throw new RecordFormatException("String record was "
+			throw new RecordFormatException("String record was "
-                    + (hasStringRec ? "": "not ") + " supplied but formula record flag is "
+					+ (hasStringRec ? "": "not ") + " supplied but formula record flag is "
-                    + (hasCachedStringFlag ? "" : "not ") + " set");
+					+ (hasCachedStringFlag ? "" : "not ") + " set");
-        }
+		}
 
-        if (formulaRec.isSharedFormula()) {
+		_formulaRecord = formulaRec;
-            svm.convertSharedFormulaRecord(formulaRec);
+		_sharedValueManager = svm;
-        }
+		_stringRecord = stringRec;
-        _formulaRecord = formulaRec;
+		if (formulaRec.isSharedFormula()) {
-        _sharedValueManager = svm;
+			_sharedFormulaRecord = svm.linkSharedFormulaRecord(this);
-        _stringRecord = stringRec;
+			if (_sharedFormulaRecord == null) {
-    }
+				handleMissingSharedFormulaRecord(formulaRec);
+			}
+		}
+	}
+	/**
+	 * Sometimes the shared formula flag "seems" to be erroneously set (because the corresponding
+	 * {@link SharedFormulaRecord} does not exist). Normally this would leave no way of determining
+	 * the {@link Ptg} tokens for the formula.  However as it turns out in these 
+	 * cases, Excel encodes the unshared {@link Ptg} tokens in the right place (inside the {@link
+	 * FormulaRecord}).  So the the only thing that needs to be done is to ignore the erroneous
+	 * shared formula flag.<br/>
+	 * 
+	 * This method may also be used for setting breakpoints to help diagnose issues regarding the
+	 * abnormally-set 'shared formula' flags. 
+	 * (see TestValueRecordsAggregate.testSpuriousSharedFormulaFlag()).<p/>
+	 */
+	private static void handleMissingSharedFormulaRecord(FormulaRecord formula) {
+		// make sure 'unshared' formula is actually available
+		Ptg firstToken = formula.getParsedExpression()[0]; 
+		if (firstToken instanceof ExpPtg) {
+			throw new RecordFormatException(
+					"SharedFormulaRecord not found for FormulaRecord with (isSharedFormula=true)");
+		}
+		// could log an info message here since this is a fairly unusual occurrence.
+		formula.setSharedFormula(false); // no point leaving the flag erroneously set
+	}
 
-    public FormulaRecord getFormulaRecord() {
+	public FormulaRecord getFormulaRecord() {
-        return _formulaRecord;
+		return _formulaRecord;
-    }
+	}
 
-    /**
+	/**
-     * debug only
+	 * debug only
-     * TODO - encapsulate
+	 * TODO - encapsulate
-     */
+	 */
-    public StringRecord getStringRecord() {
+	public StringRecord getStringRecord() {
-        return _stringRecord;
+		return _stringRecord;
-    }
+	}
 
-    public short getXFIndex() {
+	public short getXFIndex() {
-        return _formulaRecord.getXFIndex();
+		return _formulaRecord.getXFIndex();
-    }
+	}
 
-    public void setXFIndex(short xf) {
+	public void setXFIndex(short xf) {
-        _formulaRecord.setXFIndex(xf);
+		_formulaRecord.setXFIndex(xf);
-    }
+	}
 
-    public void setColumn(short col) {
+	public void setColumn(short col) {
-        _formulaRecord.setColumn(col);
+		_formulaRecord.setColumn(col);
-    }
+	}
 
-    public void setRow(int row) {
+	public void setRow(int row) {
-        _formulaRecord.setRow(row);
+		_formulaRecord.setRow(row);
-    }
+	}
 
-    public short getColumn() {
+	public short getColumn() {
-        return _formulaRecord.getColumn();
+		return _formulaRecord.getColumn();
-    }
+	}
 
-    public int getRow() {
+	public int getRow() {
-        return _formulaRecord.getRow();
+		return _formulaRecord.getRow();
-    }
+	}
 
-    public String toString() {
+	public String toString() {
-        return _formulaRecord.toString();
+		return _formulaRecord.toString();
-    }
+	}
 
-    public void visitContainedRecords(RecordVisitor rv) {
+	public void visitContainedRecords(RecordVisitor rv) {
-         rv.visitRecord(_formulaRecord);
+		 rv.visitRecord(_formulaRecord);
-         Record sharedFormulaRecord = _sharedValueManager.getRecordForFirstCell(_formulaRecord);
+		 // TODO - only bother with this if array or table formula
-         if (sharedFormulaRecord != null) {
+		 Record sharedFormulaRecord = _sharedValueManager.getRecordForFirstCell(_formulaRecord);
-             rv.visitRecord(sharedFormulaRecord);
+		 if (sharedFormulaRecord != null) {
-         }
+			 rv.visitRecord(sharedFormulaRecord);
-         if (_stringRecord != null) {
+		 }
-             rv.visitRecord(_stringRecord);
+		 if (_stringRecord != null) {
-         }
+			 rv.visitRecord(_stringRecord);
-    }
+		 }
+	}
 
-    public String getStringValue() {
+	public String getStringValue() {
-        if(_stringRecord==null) {
+		if(_stringRecord==null) {
-            return null;
+			return null;
-        }
+		}
-        return _stringRecord.getString();
+		return _stringRecord.getString();
-    }
+	}
 
-    public void setCachedStringResult(String value) {
+	public void setCachedStringResult(String value) {
 
-        // Save the string into a String Record, creating one if required
+		// Save the string into a String Record, creating one if required
-        if(_stringRecord == null) {
+		if(_stringRecord == null) {
-            _stringRecord = new StringRecord();
+			_stringRecord = new StringRecord();
-        }
+		}
-        _stringRecord.setString(value);
+		_stringRecord.setString(value);
-        if (value.length() < 1) {
+		if (value.length() < 1) {
-            _formulaRecord.setCachedResultTypeEmptyString();
+			_formulaRecord.setCachedResultTypeEmptyString();
-        } else {
+		} else {
-            _formulaRecord.setCachedResultTypeString();
+			_formulaRecord.setCachedResultTypeString();
-        }
+		}
-    }
+	}
-    public void setCachedBooleanResult(boolean value) {
+	public void setCachedBooleanResult(boolean value) {
-        _stringRecord = null;
+		_stringRecord = null;
-        _formulaRecord.setCachedResultBoolean(value);
+		_formulaRecord.setCachedResultBoolean(value);
-    }
+	}
-    public void setCachedErrorResult(int errorCode) {
+	public void setCachedErrorResult(int errorCode) {
-        _stringRecord = null;
+		_stringRecord = null;
-        _formulaRecord.setCachedResultErrorCode(errorCode);
+		_formulaRecord.setCachedResultErrorCode(errorCode);
-    }
+	}
+
+	public Ptg[] getFormulaTokens() {
+		if (_sharedFormulaRecord == null) {
+			return _formulaRecord.getParsedExpression();
+		}
+		return _sharedFormulaRecord.getFormulaTokens(_formulaRecord);
+	}
+
+	/**
+	 * Also checks for a related shared formula and unlinks it if found
+	 */
+	public void setParsedExpression(Ptg[] ptgs) {
+		notifyFormulaChanging();
+		_formulaRecord.setParsedExpression(ptgs);
+	}
+
+	public void unlinkSharedFormula() {
+		SharedFormulaRecord sfr = _sharedFormulaRecord;
+		if (sfr == null) {
+			throw new IllegalStateException("Formula not linked to shared formula");
+		}
+		Ptg[] ptgs = sfr.getFormulaTokens(_formulaRecord);
+		_formulaRecord.setParsedExpression(ptgs);
+		//Now its not shared!
+		_formulaRecord.setSharedFormula(false);
+		_sharedFormulaRecord = null;
+	}
+	/**
+	 * Should be called by any code which is either deleting this formula cell, or changing
+	 * its type.  This method gives the aggregate a chance to unlink any shared formula
+	 * that may be involved with this cell formula.
+	 */
+	public void notifyFormulaChanging() {
+		if (_sharedFormulaRecord != null) {
+			_sharedValueManager.unlink(_sharedFormulaRecord);
+		}
+	}
 }

src/java/org/apache/poi/hssf/record/aggregates/RecordAggregate.java

@@ -19,7 +19,6 @@ package org.apache.poi.hssf.record.aggregates;
 
 import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.RecordBase;
-import org.apache.poi.hssf.record.RecordInputStream;
 
 /**
  * <tt>RecordAggregate</tt>s are groups of of BIFF <tt>Record</tt>s that are typically stored 
@@ -29,16 +28,6 @@ import org.apache.poi.hssf.record.RecordInputStream;
  * @author Josh Micich
  */
 public abstract class RecordAggregate extends RecordBase {
-	// TODO - delete these methods when all subclasses have been converted
-	protected final void validateSid(short id) {
-		throw new RuntimeException("Should not be called");
-	}
-	protected final void fillFields(RecordInputStream in) {
-		throw new RuntimeException("Should not be called");
-	}
-	public final short getSid() {
-		throw new RuntimeException("Should not be called");
-	}
 
 	/**
 	 * Visit each of the atomic BIFF records contained in this {@link RecordAggregate} in the order

src/java/org/apache/poi/hssf/record/aggregates/RowRecordsAggregate.java

@@ -321,39 +321,38 @@ public final class RowRecordsAggregate extends RecordAggregate {
         return currentRow-1;
     }
 
-    public int writeHidden( RowRecord rowRecord, int row, boolean hidden )
+    /**
-    {
+     * Hide all rows at or below the current outline level
+     * @return index of the <em>next<em> row after the last row that gets hidden
+     */
+    private int writeHidden(RowRecord pRowRecord, int row) {
+        int rowIx = row;
+        RowRecord rowRecord = pRowRecord;
         int level = rowRecord.getOutlineLevel();
-        while (rowRecord != null && this.getRow(row).getOutlineLevel() >= level)
+        while (rowRecord != null && getRow(rowIx).getOutlineLevel() >= level) {
-        {
+            rowRecord.setZeroHeight(true);
-            rowRecord.setZeroHeight( hidden );
+            rowIx++;
-            row++;
+            rowRecord = getRow(rowIx);
-            rowRecord = this.getRow( row );
         }
-        return row - 1;
+        return rowIx;
     }
 
-    public void collapseRow( int rowNumber )
+    public void collapseRow(int rowNumber) {
-    {
 
         // Find the start of the group.
-        int startRow = findStartOfRowOutlineGroup( rowNumber );
+        int startRow = findStartOfRowOutlineGroup(rowNumber);
-        RowRecord rowRecord = getRow( startRow );
+        RowRecord rowRecord = getRow(startRow);
 
         // Hide all the columns until the end of the group
-        int lastRow = writeHidden( rowRecord, startRow, true );
+        int nextRowIx = writeHidden(rowRecord, startRow);
 
+        RowRecord row = getRow(nextRowIx);
+        if (row == null) {
+            row = createRow(nextRowIx);
+            insertRow(row);
+        }
         // Write collapse field
-        if (getRow(lastRow + 1) != null)
+        row.setColapsed(true);
-        {
-            getRow(lastRow + 1).setColapsed( true );
-        }
-        else
-        {
-            RowRecord row = createRow( lastRow + 1);
-            row.setColapsed( true );
-            insertRow( row );
-        }
     }
 
     /**
@@ -500,6 +499,9 @@ public final class RowRecordsAggregate extends RecordAggregate {
         _valuesAgg.insertCell(cvRec);
     }
     public void removeCell(CellValueRecordInterface cvRec) {
+        if (cvRec instanceof FormulaRecordAggregate) {
+            ((FormulaRecordAggregate)cvRec).notifyFormulaChanging();
+        }
         _valuesAgg.removeCell(cvRec);
     }
     public FormulaRecordAggregate createFormula(int row, int col) {

src/java/org/apache/poi/hssf/record/aggregates/SharedValueManager.java

@@ -17,6 +17,9 @@
 
 package org.apache.poi.hssf.record.aggregates;
 
+import java.util.HashMap;
+import java.util.Map;
+
 import org.apache.poi.hssf.record.ArrayRecord;
 import org.apache.poi.hssf.record.FormulaRecord;
 import org.apache.poi.hssf.record.SharedFormulaRecord;
@@ -35,18 +38,72 @@ import org.apache.poi.hssf.record.TableRecord;
  * @author Josh Micich
  */
 public final class SharedValueManager {
+	
+	// This class should probably be generalised to handle array and table groups too
+	private static final class SharedValueGroup {
+		private final SharedValueRecordBase _svr;
+		private final FormulaRecordAggregate[] _frAggs;
+		private int _numberOfFormulas;
+
+		public SharedValueGroup(SharedValueRecordBase svr) {
+			_svr = svr;
+			int width = svr.getLastColumn() - svr.getFirstColumn() + 1;
+			int height = svr.getLastRow() - svr.getFirstRow() + 1;
+			_frAggs = new FormulaRecordAggregate[width * height];
+			_numberOfFormulas = 0;
+		}
+
+		public void add(FormulaRecordAggregate agg) {
+			_frAggs[_numberOfFormulas++] = agg;
+		}
+
+		public void unlinkSharedFormulas() {
+			for (int i = 0; i < _numberOfFormulas; i++) {
+				_frAggs[i].unlinkSharedFormula();
+			}
+		}
+
+		public boolean isInRange(int rowIx, int columnIx) {
+			return _svr.isInRange(rowIx, columnIx);
+		}
+
+		public SharedValueRecordBase getSVR() {
+			return _svr;
+		}
+
+		/**
+		 * Note - Sometimes the first formula in a group is not present (because the range
+		 * is sparsely populated), so this method can return <code>true</code> for a cell
+		 * that is not the top-left corner of the range.
+		 * @return <code>true</code> if this is the first formula cell in the group
+		 */
+		public boolean isFirstCell(int row, int column) {
+			// hack for the moment, just check against the first formula that 
+			// came in through the add() method.
+			FormulaRecordAggregate fra = _frAggs[0];
+			return fra.getRow() == row && fra.getColumn() == column;
+		}
+		
+	}
 
 	public static final SharedValueManager EMPTY = new SharedValueManager(
 			new SharedFormulaRecord[0], new ArrayRecord[0], new TableRecord[0]);
-	private final SharedFormulaRecord[] _sfrs;
 	private final ArrayRecord[] _arrayRecords;
 	private final TableRecord[] _tableRecords;
+	private final Map _groupsBySharedFormulaRecord;
+	/** cached for optimization purposes */
+	private SharedValueGroup[] _groups;
 
 	private SharedValueManager(SharedFormulaRecord[] sharedFormulaRecords,
 			ArrayRecord[] arrayRecords, TableRecord[] tableRecords) {
-		_sfrs = sharedFormulaRecords;
 		_arrayRecords = arrayRecords;
 		_tableRecords = tableRecords;
+		Map m = new HashMap(sharedFormulaRecords.length * 3 / 2);
+		for (int i = 0; i < sharedFormulaRecords.length; i++) {
+			SharedFormulaRecord sfr = sharedFormulaRecords[i];
+			m.put(sfr, new SharedValueGroup(sfr));
+		}
+		_groupsBySharedFormulaRecord = m;
 	}
 
 	/**
@@ -64,42 +121,42 @@ public final class SharedValueManager {
 		return new SharedValueManager(sharedFormulaRecords, arrayRecords, tableRecords);
 	}
 
-	public void convertSharedFormulaRecord(FormulaRecord formula) {
+
+	/**
+	 * @return <code>null</code> if the specified formula does not have any corresponding
+	 * {@link SharedFormulaRecord}
+	 */
+	public SharedFormulaRecord linkSharedFormulaRecord(FormulaRecordAggregate agg) {
+		FormulaRecord formula = agg.getFormulaRecord();
 		int row = formula.getRow();
 		int column = formula.getColumn();
 		// Traverse the list of shared formulas in
 		// reverse order, and try to find the correct one
 		// for us
-		for (int i = 0; i < _sfrs.length; i++) {
+		
-			SharedFormulaRecord shrd = _sfrs[i];
+		SharedValueGroup[] groups = getGroups();
-			if (shrd.isInRange(row, column)) {
+		for (int i = 0; i < groups.length; i++) {
-				shrd.convertSharedFormulaRecord(formula);
+			SharedValueGroup svr = groups[i];
-				return;
+			if (svr.isInRange(row, column)) {
+				svr.add(agg);
+				return (SharedFormulaRecord) svr.getSVR();
 			}
 		}
-		// not found
+		return null;
-		handleMissingSharedFormulaRecord(formula);
 	}
 
-	/**
+	private SharedValueGroup[] getGroups() {
-	 * Sometimes the shared formula flag "seems" to be erroneously set, in which case there is no 
+		if (_groups == null) {
-	 * call to <tt>SharedFormulaRecord.convertSharedFormulaRecord</tt> and hence the 
+			SharedValueGroup[] groups = new SharedValueGroup[_groupsBySharedFormulaRecord.size()];
-	 * <tt>parsedExpression</tt> field of this <tt>FormulaRecord</tt> will not get updated.<br/>
+			_groupsBySharedFormulaRecord.values().toArray(groups);
-	 * As it turns out, this is not a problem, because in these circumstances, the existing value
+			_groups = groups;
-	 * for <tt>parsedExpression</tt> is perfectly OK.<p/>
+			
-	 * 
+		}
-	 * This method may also be used for setting breakpoints to help diagnose issues regarding the
+		return _groups;
-	 * abnormally-set 'shared formula' flags. 
-	 * (see TestValueRecordsAggregate.testSpuriousSharedFormulaFlag()).<p/>
-	 * 
-	 * The method currently does nothing but do not delete it without finding a nice home for this 
-	 * comment.
-	 */
-	private static void handleMissingSharedFormulaRecord(FormulaRecord formula) {
-		// could log an info message here since this is a fairly unusual occurrence.
-		formula.setSharedFormula(false); // no point leaving the flag erroneously set
 	}
 
+
+
 	/**
 	 * Note - does not return SharedFormulaRecords currently, because the corresponding formula
 	 * records have been converted to 'unshared'. POI does not attempt to re-share formulas. On
@@ -125,6 +182,26 @@ public final class SharedValueManager {
 				return ar;
 			}
 		}
+		SharedValueGroup[] groups = getGroups();
+		for (int i = 0; i < groups.length; i++) {
+			SharedValueGroup svg = groups[i];
+			if (svg.isFirstCell(row, column)) {
+				return svg.getSVR();
+			}
+		}
 		return null;
 	}
+
+	/**
+	 * Converts all {@link FormulaRecord}s handled by <tt>sharedFormulaRecord</tt> 
+	 * to plain unshared formulas
+	 */
+	public void unlink(SharedFormulaRecord sharedFormulaRecord) {
+		SharedValueGroup svg = (SharedValueGroup) _groupsBySharedFormulaRecord.remove(sharedFormulaRecord);
+		_groups = null; // be sure to reset cached value
+		if (svg == null) {
+			throw new IllegalStateException("Failed to find formulas for shared formula");
+		}
+		svg.unlinkSharedFormulas();
+	}
 }

src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java

@@ -145,9 +145,6 @@ public final class ValueRecordsAggregate {
     public void construct(CellValueRecordInterface rec, RecordStream rs, SharedValueManager sfh) {
         if (rec instanceof FormulaRecord) {
             FormulaRecord formulaRec = (FormulaRecord)rec;
-            if (formulaRec.isSharedFormula()) {
-                sfh.convertSharedFormulaRecord(formulaRec);
-            }
             // read optional cached text value
             StringRecord cachedText;
             Class nextClass = rs.peekNextClass();

src/java/org/apache/poi/hssf/record/cf/BorderFormatting.java

@@ -1,4 +1,3 @@
-
 /* ====================================================================
    Licensed to the Apache Software Foundation (ASF) under one or more
    contributor license agreements.  See the NOTICE file distributed with
@@ -15,129 +14,54 @@
    See the License for the specific language governing permissions and
    limitations under the License.
 ==================================================================== */
-        
 
-/*
- * FontFormatting.java
- *
- * Created on January 22, 2008, 10:05 PM
- */
 package org.apache.poi.hssf.record.cf;
 
-import org.apache.poi.hssf.record.RecordInputStream;
 import org.apache.poi.util.BitField;
 import org.apache.poi.util.BitFieldFactory;
 import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.LittleEndianInput;
+import org.apache.poi.util.LittleEndianOutput;
 
 /**
  * Border Formatting Block of the Conditional Formatting Rule Record.
- * 
+ *
  * @author Dmitriy Kumshayev
  */
+public final class BorderFormatting {
 
-public class BorderFormatting 
+    /** No border */
-{
-	
-    /**
-     * No border
-     */
-
     public final static short    BORDER_NONE                = 0x0;
-
+    /** Thin border */
-    /**
-     * Thin border
-     */
-
     public final static short    BORDER_THIN                = 0x1;
-
+    /** Medium border */
-    /**
-     * Medium border
-     */
-
     public final static short    BORDER_MEDIUM              = 0x2;
-
+    /** dash border */
-    /**
-     * dash border
-     */
-
     public final static short    BORDER_DASHED              = 0x3;
-
+    /** dot border */
-    /**
+    public final static short    BORDER_HAIR                = 0x4;
-     * dot border
+    /** Thick border */
-     */
-
-    public final static short    BORDER_HAIR              = 0x4;
-
-    /**
-     * Thick border
-     */
-
     public final static short    BORDER_THICK               = 0x5;
-
+    /** double-line border */
-    /**
-     * double-line border
-     */
-
     public final static short    BORDER_DOUBLE              = 0x6;
-
+    /** hair-line border */
-    /**
+    public final static short    BORDER_DOTTED              = 0x7;
-     * hair-line border
+    /** Medium dashed border */
-     */
-
-    public final static short    BORDER_DOTTED                = 0x7;
-
-    /**
-     * Medium dashed border
-     */
-
     public final static short    BORDER_MEDIUM_DASHED       = 0x8;
-
+    /** dash-dot border */
-    /**
-     * dash-dot border
-     */
-
     public final static short    BORDER_DASH_DOT            = 0x9;
-
+    /** medium dash-dot border */
-    /**
-     * medium dash-dot border
-     */
-
     public final static short    BORDER_MEDIUM_DASH_DOT     = 0xA;
-
+    /** dash-dot-dot border */
-    /**
-     * dash-dot-dot border
-     */
-
     public final static short    BORDER_DASH_DOT_DOT        = 0xB;
-
+    /** medium dash-dot-dot border */
-    /**
-     * medium dash-dot-dot border
-     */
-
     public final static short    BORDER_MEDIUM_DASH_DOT_DOT = 0xC;
-
+    /** slanted dash-dot border */
-    /**
-     * slanted dash-dot border
-     */
-
     public final static short    BORDER_SLANTED_DASH_DOT    = 0xD;
-	
+
-    public BorderFormatting()
-    {
-        field_13_border_styles1	= (short)0;
-        field_14_border_styles2	= (short)0;
-    }
-    
-    /** Creates new FontFormatting */
-    public BorderFormatting(RecordInputStream in)
-	{
-        field_13_border_styles1	= in.readInt();
-        field_14_border_styles2	= in.readInt();
-	}
-    
     // BORDER FORMATTING BLOCK
     // For Border Line Style codes see HSSFCellStyle.BORDER_XXXXXX
-    private int 			 field_13_border_styles1;
+    private int              field_13_border_styles1;
     private static final BitField  bordLeftLineStyle  = BitFieldFactory.getInstance(0x0000000F);
     private static final BitField  bordRightLineStyle = BitFieldFactory.getInstance(0x000000F0);
     private static final BitField  bordTopLineStyle   = BitFieldFactory.getInstance(0x00000F00);
@@ -147,12 +71,25 @@ public class BorderFormatting
     private static final BitField  bordTlBrLineOnOff  = BitFieldFactory.getInstance(0x40000000);
     private static final BitField  bordBlTrtLineOnOff = BitFieldFactory.getInstance(0x80000000);
 
-    private int 			 field_14_border_styles2;
+    private int              field_14_border_styles2;
     private static final BitField  bordTopLineColor   = BitFieldFactory.getInstance(0x0000007F);
     private static final BitField  bordBottomLineColor= BitFieldFactory.getInstance(0x00003f80);
     private static final BitField  bordDiagLineColor  = BitFieldFactory.getInstance(0x001FC000);
     private static final BitField  bordDiagLineStyle  = BitFieldFactory.getInstance(0x01E00000);
 
+
+    public BorderFormatting() {
+        field_13_border_styles1    = 0;
+        field_14_border_styles2    = 0;
+    }
+
+    /** Creates new FontFormatting */
+    public BorderFormatting(LittleEndianInput in) {
+        field_13_border_styles1    = in.readInt();
+        field_14_border_styles2    = in.readInt();
+    }
+
+
     /**
      * set the type of border to use for the left border of the cell
      * @param border type
@@ -171,10 +108,8 @@ public class BorderFormatting
      * @see #BORDER_MEDIUM_DASH_DOT_DOT
      * @see #BORDER_SLANTED_DASH_DOT
      */
-
+    public void setBorderLeft(int border) {
-    public void setBorderLeft(short border)
+        field_13_border_styles1 = bordLeftLineStyle.setValue(field_13_border_styles1, border);
-    {
-    	field_13_border_styles1 = bordLeftLineStyle.setValue(field_13_border_styles1, border);
     }
 
     /**
@@ -195,10 +130,8 @@ public class BorderFormatting
      * @see #BORDER_MEDIUM_DASH_DOT_DOT
      * @see #BORDER_SLANTED_DASH_DOT
      */
-
+    public int getBorderLeft() {
-    public short getBorderLeft()
+        return bordLeftLineStyle.getValue(field_13_border_styles1);
-    {
-    	return (short)bordLeftLineStyle.getValue(field_13_border_styles1);
     }
 
     /**
@@ -219,10 +152,8 @@ public class BorderFormatting
      * @see #BORDER_MEDIUM_DASH_DOT_DOT
      * @see #BORDER_SLANTED_DASH_DOT
      */
-
+    public void setBorderRight(int border) {
-    public void setBorderRight(short border)
+        field_13_border_styles1 = bordRightLineStyle.setValue(field_13_border_styles1, border);
-    {
-    	field_13_border_styles1 = bordRightLineStyle.setValue(field_13_border_styles1, border);
     }
 
     /**
@@ -243,10 +174,8 @@ public class BorderFormatting
      * @see #BORDER_MEDIUM_DASH_DOT_DOT
      * @see #BORDER_SLANTED_DASH_DOT
      */
-
+    public int getBorderRight() {
-    public short getBorderRight()
+        return bordRightLineStyle.getValue(field_13_border_styles1);
-    {
-    	return (short)bordRightLineStyle.getValue(field_13_border_styles1);
     }
 
     /**
@@ -267,10 +196,8 @@ public class BorderFormatting
      * @see #BORDER_MEDIUM_DASH_DOT_DOT
      * @see #BORDER_SLANTED_DASH_DOT
      */
-
+    public void setBorderTop(int border) {
-    public void setBorderTop(short border)
+        field_13_border_styles1 = bordTopLineStyle.setValue(field_13_border_styles1, border);
-    {
-    	field_13_border_styles1 = bordTopLineStyle.setValue(field_13_border_styles1, border);
     }
 
     /**
@@ -291,10 +218,8 @@ public class BorderFormatting
      * @see #BORDER_MEDIUM_DASH_DOT_DOT
      * @see #BORDER_SLANTED_DASH_DOT
      */
-
+    public int getBorderTop() {
-    public short getBorderTop()
+        return bordTopLineStyle.getValue(field_13_border_styles1);
-    {
-    	return (short)bordTopLineStyle.getValue(field_13_border_styles1);
     }
 
     /**
@@ -315,10 +240,8 @@ public class BorderFormatting
      * @see #BORDER_MEDIUM_DASH_DOT_DOT
      * @see #BORDER_SLANTED_DASH_DOT
      */
-
+    public void setBorderBottom(int border) {
-    public void setBorderBottom(short border)
+        field_13_border_styles1 = bordBottomLineStyle.setValue(field_13_border_styles1, border);
-    {
-    	field_13_border_styles1 = bordBottomLineStyle.setValue(field_13_border_styles1, border);
     }
 
     /**
@@ -339,11 +262,10 @@ public class BorderFormatting
      * @see #BORDER_MEDIUM_DASH_DOT_DOT
      * @see #BORDER_SLANTED_DASH_DOT
      */
-    public short getBorderBottom()
+    public int getBorderBottom() {
-    {
+        return bordBottomLineStyle.getValue(field_13_border_styles1);
-    	return (short)bordBottomLineStyle.getValue(field_13_border_styles1);
     }
-    
+
     /**
      * set the type of border to use for the diagonal border of the cell
      * @param border type
@@ -362,10 +284,8 @@ public class BorderFormatting
      * @see #BORDER_MEDIUM_DASH_DOT_DOT
      * @see #BORDER_SLANTED_DASH_DOT
      */
-
+    public void setBorderDiagonal(int border) {
-    public void setBorderDiagonal(short border)
+        field_14_border_styles2 = bordDiagLineStyle.setValue(field_14_border_styles2, border);
-    {
-    	field_14_border_styles2 = bordDiagLineStyle.setValue(field_14_border_styles2, border);
     }
 
     /**
@@ -386,18 +306,16 @@ public class BorderFormatting
      * @see #BORDER_MEDIUM_DASH_DOT_DOT
      * @see #BORDER_SLANTED_DASH_DOT
      */
-    public short getBorderDiagonal()
+    public int getBorderDiagonal() {
-    {
+        return bordDiagLineStyle.getValue(field_14_border_styles2);
-    	return (short)bordDiagLineStyle.getValue(field_14_border_styles2);
     }
 
     /**
      * set the color to use for the left border
      * @param color The index of the color definition
      */
-    public void setLeftBorderColor(short color)
+    public void setLeftBorderColor(int color) {
-    {
+        field_13_border_styles1 = bordLeftLineColor.setValue(field_13_border_styles1, color);
-    	field_13_border_styles1 = bordLeftLineColor.setValue(field_13_border_styles1, color);
     }
 
     /**
@@ -405,18 +323,16 @@ public class BorderFormatting
      * @see org.apache.poi.hssf.usermodel.HSSFPalette#getColor(short)
      * @param color The index of the color definition
      */
-    public short getLeftBorderColor()
+    public int getLeftBorderColor() {
-    {
+        return bordLeftLineColor.getValue(field_13_border_styles1);
-    	return (short)bordLeftLineColor.getValue(field_13_border_styles1);
     }
 
     /**
      * set the color to use for the right border
      * @param color The index of the color definition
      */
-    public void setRightBorderColor(short color)
+    public void setRightBorderColor(int color) {
-    {
+        field_13_border_styles1 = bordRightLineColor.setValue(field_13_border_styles1, color);
-    	field_13_border_styles1 = bordRightLineColor.setValue(field_13_border_styles1, color);
     }
 
     /**
@@ -424,18 +340,16 @@ public class BorderFormatting
      * @see org.apache.poi.hssf.usermodel.HSSFPalette#getColor(short)
      * @param color The index of the color definition
      */
-    public short getRightBorderColor()
+    public int getRightBorderColor() {
-    {
+        return bordRightLineColor.getValue(field_13_border_styles1);
-    	return (short)bordRightLineColor.getValue(field_13_border_styles1);
     }
 
     /**
      * set the color to use for the top border
      * @param color The index of the color definition
      */
-    public void setTopBorderColor(short color)
+    public void setTopBorderColor(int color) {
-    {
+        field_14_border_styles2 = bordTopLineColor.setValue(field_14_border_styles2, color);
-    	field_14_border_styles2 = bordTopLineColor.setValue(field_14_border_styles2, color);
     }
 
     /**
@@ -443,18 +357,17 @@ public class BorderFormatting
      * @see org.apache.poi.hssf.usermodel.HSSFPalette#getColor(short)
      * @param color The index of the color definition
      */
-    public short getTopBorderColor()
+    public int getTopBorderColor() {
-    {
+        return bordTopLineColor.getValue(field_14_border_styles2);
-    	return (short)bordTopLineColor.getValue(field_14_border_styles2);
     }
 
     /**
      * set the color to use for the bottom border
      * @param color The index of the color definition
      */
-    public void setBottomBorderColor(short color)
+    public void setBottomBorderColor(int color)
     {
-    	field_14_border_styles2 = bordBottomLineColor.setValue(field_14_border_styles2, color);
+        field_14_border_styles2 = bordBottomLineColor.setValue(field_14_border_styles2, color);
     }
 
     /**
@@ -462,18 +375,16 @@ public class BorderFormatting
      * @see org.apache.poi.hssf.usermodel.HSSFPalette#getColor(short)
      * @param color The index of the color definition
      */
-    public short getBottomBorderColor()
+    public int getBottomBorderColor() {
-    {
+        return bordBottomLineColor.getValue(field_14_border_styles2);
-    	return (short)bordBottomLineColor.getValue(field_14_border_styles2);
     }
-    
+
     /**
      * set the color to use for the diagonal borders
      * @param color The index of the color definition
      */
-    public void setDiagonalBorderColor(short color)
+    public void setDiagonalBorderColor(int color) {
-    {
+        field_14_border_styles2 = bordDiagLineColor.setValue(field_14_border_styles2, color);
-    	field_14_border_styles2 = bordDiagLineColor.setValue(field_14_border_styles2, color);
     }
 
     /**
@@ -481,50 +392,44 @@ public class BorderFormatting
      * @see org.apache.poi.hssf.usermodel.HSSFPalette#getColor(short)
      * @param color The index of the color definition
      */
-    public short getDiagonalBorderColor()
+    public int getDiagonalBorderColor() {
-    {
+        return bordDiagLineColor.getValue(field_14_border_styles2);
-    	return (short)bordDiagLineColor.getValue(field_14_border_styles2);
     }
 
     /**
      * Of/off bottom left to top right line
-     *   
+     *
-     * @param on - if true - on, otherwise off
+     * @param on - if <code>true</code> - on, otherwise off
      */
-    public void setForwardDiagonalOn(boolean on)
+    public void setForwardDiagonalOn(boolean on) {
-    {
+        field_13_border_styles1 = bordBlTrtLineOnOff.setBoolean(field_13_border_styles1, on);
-    	field_13_border_styles1 = bordBlTrtLineOnOff.setBoolean(field_13_border_styles1, on);
     }
 
     /**
      * Of/off top left to bottom right line
-     *   
+     *
-     * @param on - if true - on, otherwise off
+     * @param on - if <code>true</code> - on, otherwise off
      */
-    public void setBackwardDiagonalOn(boolean on)
+    public void setBackwardDiagonalOn(boolean on) {
-    {
+        field_13_border_styles1 = bordTlBrLineOnOff.setBoolean(field_13_border_styles1, on);
-    	field_13_border_styles1 = bordTlBrLineOnOff.setBoolean(field_13_border_styles1, on);
-    }
-    
-    /**
-     * @return true if forward diagonal is on
-     */
-    public boolean isForwardDiagonalOn()
-    {
-    	return bordBlTrtLineOnOff.isSet(field_13_border_styles1);
     }
 
     /**
-     * @return true if backward diagonal is on
+     * @return <code>true</code> if forward diagonal is on
      */
-    public boolean isBackwardDiagonalOn()
+    public boolean isForwardDiagonalOn() {
-    {
+        return bordBlTrtLineOnOff.isSet(field_13_border_styles1);
-    	return bordTlBrLineOnOff.isSet(field_13_border_styles1);
     }
-    
+
-    
+    /**
-    public String toString()
+     * @return <code>true</code> if backward diagonal is on
-    {
+     */
+    public boolean isBackwardDiagonalOn() {
+        return bordTlBrLineOnOff.isSet(field_13_border_styles1);
+    }
+
+
+    public String toString() {
         StringBuffer buffer = new StringBuffer();
         buffer.append("    [Border Formatting]\n");
         buffer.append("          .lftln     = ").append(Integer.toHexString(getBorderLeft())).append("\n");
@@ -540,21 +445,21 @@ public class BorderFormatting
         buffer.append("    [/Border Formatting]\n");
         return buffer.toString();
     }
-    
+
-    public Object clone() 
+    public Object clone() {
-    {
       BorderFormatting rec = new BorderFormatting();
-      rec.field_13_border_styles1 = field_13_border_styles1; 
+      rec.field_13_border_styles1 = field_13_border_styles1;
-      rec.field_14_border_styles2 = field_14_border_styles2; 
+      rec.field_14_border_styles2 = field_14_border_styles2;
       return rec;
     }
-    
+
-    public int serialize(int offset, byte [] data)
+    public int serialize(int offset, byte [] data) {
-    {
+        LittleEndian.putInt(data, offset+0, field_13_border_styles1);
-    	LittleEndian.putInt(data, offset, field_13_border_styles1);
+        LittleEndian.putInt(data, offset+4, field_14_border_styles2);
-    	offset += 4;
+        return 8;
-    	LittleEndian.putInt(data, offset, field_14_border_styles2);
+    }
-    	offset += 4;
+    public void serialize(LittleEndianOutput out) {
-    	return 8;
+        out.writeInt(field_13_border_styles1);
+        out.writeInt(field_14_border_styles2);
     }
 }

src/java/org/apache/poi/hssf/record/cf/PatternFormatting.java

@@ -1,4 +1,3 @@
-
 /* ====================================================================
    Licensed to the Apache Software Foundation (ASF) under one or more
    contributor license agreements.  See the NOTICE file distributed with
@@ -15,28 +14,20 @@
    See the License for the specific language governing permissions and
    limitations under the License.
 ==================================================================== */
-        
 
-/*
- * FontFormatting.java
- *
- * Created on January 22, 2008, 10:05 PM
- */
 package org.apache.poi.hssf.record.cf;
 
-import org.apache.poi.hssf.record.RecordInputStream;
 import org.apache.poi.util.BitField;
 import org.apache.poi.util.BitFieldFactory;
-import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.LittleEndianInput;
+import org.apache.poi.util.LittleEndianOutput;
 
 /**
  * Pattern Formatting Block of the Conditional Formatting Rule Record.
  * 
  * @author Dmitriy Kumshayev
  */
-
+public final class PatternFormatting implements Cloneable {
-public class PatternFormatting implements Cloneable
-{
     /**  No background */
     public final static short     NO_FILL             = 0  ;
     /**  Solidly filled */
@@ -75,29 +66,29 @@ public class PatternFormatting implements Cloneable
     public final static short     LESS_DOTS           = 17 ;
     /**  Least Dots */
     public final static short     LEAST_DOTS          = 18 ;
-	
-    public PatternFormatting()
-    {
-        field_15_pattern_style	= (short)0;
-        field_16_pattern_color_indexes	= (short)0;
-    }
     
-    /** Creates new FontFormatting */
-    public PatternFormatting(RecordInputStream in)
-	{
-    	field_15_pattern_style	= in.readShort();
-        field_16_pattern_color_indexes	= in.readShort();
-	}
     
     // PATTERN FORMATING BLOCK
     // For Pattern Styles see constants at HSSFCellStyle (from NO_FILL to LEAST_DOTS)
-    private short 			 field_15_pattern_style;
+    private int              field_15_pattern_style;
     private static final BitField  fillPatternStyle = BitFieldFactory.getInstance(0xFC00);
     
-    private short 			 field_16_pattern_color_indexes;
+    private int              field_16_pattern_color_indexes;
-    private static final BitField  patternColorIndex = BitFieldFactory.getInstance(0x007F);		 
+    private static final BitField  patternColorIndex = BitFieldFactory.getInstance(0x007F);         
-    private static final BitField  patternBackgroundColorIndex = BitFieldFactory.getInstance(0x3F80);		 
+    private static final BitField  patternBackgroundColorIndex = BitFieldFactory.getInstance(0x3F80);         
 
+    
+    public PatternFormatting() {
+        field_15_pattern_style    = 0;
+        field_16_pattern_color_indexes    = 0;
+    }
+    
+    /** Creates new FontFormatting */
+    public PatternFormatting(LittleEndianInput in) {
+        field_15_pattern_style    = in.readUShort();
+        field_16_pattern_color_indexes    = in.readUShort();
+    }
+    
     /**
      * setting fill pattern
      *
@@ -121,63 +112,48 @@ public class PatternFormatting implements Cloneable
      *
      * @param fp  fill pattern 
      */
-    public void setFillPattern(short fp)
+    public void setFillPattern(int fp) {
-    {
+        field_15_pattern_style = fillPatternStyle.setValue(field_15_pattern_style, fp);
-    	field_15_pattern_style = fillPatternStyle.setShortValue(field_15_pattern_style, fp);
     }
 
     /**
-     * get the fill pattern 
      * @return fill pattern
      */
-
+    public int getFillPattern() {
-    public short getFillPattern()
+        return fillPatternStyle.getValue(field_15_pattern_style);
-    {
-        return fillPatternStyle.getShortValue(field_15_pattern_style);
     }
     
     /**
      * set the background fill color.
-     *
-     * @param bg  color
      */
-
+    public void setFillBackgroundColor(int bg) {        
-    public void setFillBackgroundColor(short bg)
+        field_16_pattern_color_indexes = patternBackgroundColorIndex.setValue(field_16_pattern_color_indexes,bg);
-    {    	
-    	field_16_pattern_color_indexes = patternBackgroundColorIndex.setShortValue(field_16_pattern_color_indexes,bg);
     }
 
     /**
-     * get the background fill color
      * @see org.apache.poi.hssf.usermodel.HSSFPalette#getColor(short)
-     * @return fill color
+     * @return get the background fill color
      */
-    public short getFillBackgroundColor()
+    public int getFillBackgroundColor() {
-    {
+        return patternBackgroundColorIndex.getValue(field_16_pattern_color_indexes);
-    	return patternBackgroundColorIndex.getShortValue(field_16_pattern_color_indexes);
     }
 
     /**
      * set the foreground fill color
-     * @param bg  color
      */
-    public void setFillForegroundColor(short fg)
+    public void setFillForegroundColor(int fg) {
-    {
+        field_16_pattern_color_indexes = patternColorIndex.setValue(field_16_pattern_color_indexes,fg);
-    	field_16_pattern_color_indexes = patternColorIndex.setShortValue(field_16_pattern_color_indexes,fg);
     }
 
     /**
-     * get the foreground fill color
      * @see org.apache.poi.hssf.usermodel.HSSFPalette#getColor(short)
-     * @return fill color
+     * @return get the foreground fill color
      */
-    public short getFillForegroundColor()
+    public int getFillForegroundColor() {
-    {
+        return patternColorIndex.getValue(field_16_pattern_color_indexes);
-    	return patternColorIndex.getShortValue(field_16_pattern_color_indexes);
     }
     
-    public String toString()
+    public String toString() {
-    {
         StringBuffer buffer = new StringBuffer();
         buffer.append("    [Pattern Formatting]\n");
         buffer.append("          .fillpattern= ").append(Integer.toHexString(getFillPattern())).append("\n");
@@ -187,20 +163,15 @@ public class PatternFormatting implements Cloneable
         return buffer.toString();
     }
     
-    public Object clone() 
+    public Object clone()  {
-    {
       PatternFormatting rec = new PatternFormatting();
       rec.field_15_pattern_style = field_15_pattern_style;
       rec.field_16_pattern_color_indexes = field_16_pattern_color_indexes; 
       return rec;
     }
-    
+
-    public int serialize(int offset, byte [] data)
+    public void serialize(LittleEndianOutput out) {
-    {
+        out.writeShort(field_15_pattern_style);
-    	LittleEndian.putShort(data, offset, field_15_pattern_style);
+        out.writeShort(field_16_pattern_color_indexes);
-    	offset += 2;
-    	LittleEndian.putShort(data, offset, field_16_pattern_color_indexes);
-    	offset += 2;
-    	return 4;
     }
 }

src/java/org/apache/poi/hssf/usermodel/HSSFBorderFormatting.java

@@ -75,52 +75,52 @@ public final class HSSFBorderFormatting
 
 	public short getBorderBottom()
 	{
-		return borderFormatting.getBorderBottom();
+		return (short)borderFormatting.getBorderBottom();
 	}
 
 	public short getBorderDiagonal()
 	{
-		return borderFormatting.getBorderDiagonal();
+		return (short)borderFormatting.getBorderDiagonal();
 	}
 
 	public short getBorderLeft()
 	{
-		return borderFormatting.getBorderLeft();
+		return (short)borderFormatting.getBorderLeft();
 	}
 
 	public short getBorderRight()
 	{
-		return borderFormatting.getBorderRight();
+		return (short)borderFormatting.getBorderRight();
 	}
 
 	public short getBorderTop()
 	{
-		return borderFormatting.getBorderTop();
+		return (short)borderFormatting.getBorderTop();
 	}
 
 	public short getBottomBorderColor()
 	{
-		return borderFormatting.getBottomBorderColor();
+		return (short)borderFormatting.getBottomBorderColor();
 	}
 
 	public short getDiagonalBorderColor()
 	{
-		return borderFormatting.getDiagonalBorderColor();
+		return (short)borderFormatting.getDiagonalBorderColor();
 	}
 
 	public short getLeftBorderColor()
 	{
-		return borderFormatting.getLeftBorderColor();
+		return (short)borderFormatting.getLeftBorderColor();
 	}
 
 	public short getRightBorderColor()
 	{
-		return borderFormatting.getRightBorderColor();
+		return (short)borderFormatting.getRightBorderColor();
 	}
 
 	public short getTopBorderColor()
 	{
-		return borderFormatting.getTopBorderColor();
+		return (short)borderFormatting.getTopBorderColor();
 	}
 
 	public boolean isBackwardDiagonalOn()

src/java/org/apache/poi/hssf/usermodel/HSSFCell.java

@@ -269,9 +269,8 @@ public class HSSFCell implements Cell {
      * @see #CELL_TYPE_BOOLEAN
      * @see #CELL_TYPE_ERROR
      */
-
+    public void setCellType(int cellType) {
-    public void setCellType(int cellType)
+        notifyFormulaChanging();
-    {
         int row=record.getRow();
         short col=record.getColumn();
         short styleIndex=record.getXFIndex();
@@ -533,7 +532,7 @@ public class HSSFCell implements Cell {
      * @param value  value to set the cell to.  For formulas we'll set the formula
      * string, for String cells we'll set its value.  For other types we will
      * change the cell to a string cell and set its value.
-     * If value is null then we will change the cell to a Blank cell.
+     * If value is <code>null</code> then we will change the cell to a Blank cell.
      */
 
     public void setCellValue(RichTextString value)
@@ -544,6 +543,7 @@ public class HSSFCell implements Cell {
         short styleIndex=record.getXFIndex();
         if (hvalue == null)
         {
+            notifyFormulaChanging();
             setCellType(CELL_TYPE_BLANK, false, row, col, styleIndex);
             return;
         }
@@ -581,25 +581,35 @@ public class HSSFCell implements Cell {
         short styleIndex=record.getXFIndex();
 
         if (formula==null) {
+            notifyFormulaChanging();
             setCellType(CELL_TYPE_BLANK, false, row, col, styleIndex);
             return;
         }
         setCellType(CELL_TYPE_FORMULA, false, row, col, styleIndex);
-        FormulaRecordAggregate rec = (FormulaRecordAggregate) record;
+        FormulaRecordAggregate agg = (FormulaRecordAggregate) record;
-        FormulaRecord frec = rec.getFormulaRecord();
+        FormulaRecord frec = agg.getFormulaRecord();
         frec.setOptions((short) 2);
         frec.setValue(0);
 
         //only set to default if there is no extended format index already set
-        if (rec.getXFIndex() == (short)0) {
+        if (agg.getXFIndex() == (short)0) {
-            rec.setXFIndex((short) 0x0f);
+            agg.setXFIndex((short) 0x0f);
         }
         Ptg[] ptgs = HSSFFormulaParser.parse(formula, book);
-        frec.setParsedExpression(ptgs);
+        agg.setParsedExpression(ptgs);
+    }
+    /**
+     * Should be called any time that a formula could potentially be deleted.
+     * Does nothing if this cell currently does not hold a formula
+     */
+    private void notifyFormulaChanging() {
+        if (record instanceof FormulaRecordAggregate) {
+            ((FormulaRecordAggregate)record).notifyFormulaChanging();
+        }
     }
 
     public String getCellFormula() {
-        return HSSFFormulaParser.toFormulaString(book, ((FormulaRecordAggregate)record).getFormulaRecord().getParsedExpression());
+        return HSSFFormulaParser.toFormulaString(book, ((FormulaRecordAggregate)record).getFormulaTokens());
     }
 
     /**

src/java/org/apache/poi/hssf/usermodel/HSSFEvaluationWorkbook.java

@@ -19,7 +19,6 @@ package org.apache.poi.hssf.usermodel;
 
 import org.apache.poi.hssf.model.HSSFFormulaParser;
 import org.apache.poi.hssf.model.Workbook;
-import org.apache.poi.hssf.record.FormulaRecord;
 import org.apache.poi.hssf.record.NameRecord;
 import org.apache.poi.hssf.record.aggregates.FormulaRecordAggregate;
 import org.apache.poi.hssf.record.formula.NamePtg;
@@ -121,8 +120,8 @@ public final class HSSFEvaluationWorkbook implements FormulaRenderingWorkbook, E
 			// to make sure that all formulas POI can evaluate can also be parsed.
 			return HSSFFormulaParser.parse(cell.getCellFormula(), _uBook);
 		}
-		FormulaRecord fr = ((FormulaRecordAggregate) cell.getCellValueRecord()).getFormulaRecord();
+		FormulaRecordAggregate fra = (FormulaRecordAggregate) cell.getCellValueRecord();
-		return fr.getParsedExpression();
+		return fra.getFormulaTokens();
 	}
 
 	private static final class Name implements EvaluationName {

src/java/org/apache/poi/hssf/usermodel/HSSFPatternFormatting.java

@@ -87,7 +87,7 @@ public class HSSFPatternFormatting
 	 */
 	public short getFillBackgroundColor()
 	{
-		return patternFormatting.getFillBackgroundColor();
+		return (short)patternFormatting.getFillBackgroundColor();
 	}
 
 	/**
@@ -96,7 +96,7 @@ public class HSSFPatternFormatting
 	 */
 	public short getFillForegroundColor()
 	{
-		return patternFormatting.getFillForegroundColor();
+		return (short)patternFormatting.getFillForegroundColor();
 	}
 
 	/**
@@ -105,7 +105,7 @@ public class HSSFPatternFormatting
 	 */
 	public short getFillPattern()
 	{
-		return patternFormatting.getFillPattern();
+		return (short)patternFormatting.getFillPattern();
 	}
 
 	/**

src/java/org/apache/poi/hssf/util/CellRangeAddress8Bit.java

@@ -16,9 +16,10 @@
 
 package org.apache.poi.hssf.util;
 
-import org.apache.poi.hssf.record.RecordInputStream;
 import org.apache.poi.ss.util.CellRangeAddressBase;
-import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.LittleEndianByteArrayOutputStream;
+import org.apache.poi.util.LittleEndianInput;
+import org.apache.poi.util.LittleEndianOutput;
 
 /**
  * See OOO documentation: excelfileformat.pdf sec 2.5.14 - 'Cell Range Address'<p/>
@@ -35,25 +36,31 @@ public final class CellRangeAddress8Bit extends CellRangeAddressBase {
 		super(firstRow, lastRow, firstCol, lastCol);
 	}
 
-	public CellRangeAddress8Bit(RecordInputStream in) {
+	public CellRangeAddress8Bit(LittleEndianInput in) {
 		super(readUShortAndCheck(in), in.readUShort(), in.readUByte(), in.readUByte());
 	}
 
-	private static int readUShortAndCheck(RecordInputStream in) {
+	private static int readUShortAndCheck(LittleEndianInput in) {
-		if (in.remaining() < ENCODED_SIZE) {
+		if (in.available() < ENCODED_SIZE) {
 			// Ran out of data
 			throw new RuntimeException("Ran out of data reading CellRangeAddress");
 		}
 		return in.readUShort();
 	}
 
+	/**
+	 * @deprecated use {@link #serialize(LittleEndianOutput)}
+	 */
 	public int serialize(int offset, byte[] data) {
-		LittleEndian.putUShort(data, offset + 0, getFirstRow());
+		serialize(new LittleEndianByteArrayOutputStream(data, offset, ENCODED_SIZE));
-		LittleEndian.putUShort(data, offset + 2, getLastRow());
-		LittleEndian.putByte(data, offset + 4, getFirstColumn());
-		LittleEndian.putByte(data, offset + 5, getLastColumn());
 		return ENCODED_SIZE;
 	}
+	public void serialize(LittleEndianOutput out) {
+		out.writeShort(getFirstRow());
+		out.writeShort(getLastRow());
+		out.writeByte(getFirstColumn());
+		out.writeByte(getLastColumn());
+	}
 	
 	public CellRangeAddress8Bit copy() {
 		return new CellRangeAddress8Bit(getFirstRow(), getLastRow(), getFirstColumn(), getLastColumn());

src/java/org/apache/poi/hssf/util/CellRangeAddressList.java

@@ -16,11 +16,7 @@
 
 package org.apache.poi.hssf.util;
 
-import java.util.ArrayList;
-import java.util.List;
-
 import org.apache.poi.hssf.record.RecordInputStream;
-import org.apache.poi.util.LittleEndian;
 
 /**
  * Implementation of the cell range address lists,like is described

src/java/org/apache/poi/ss/formula/Formula.java

@@ -0,0 +1,133 @@
+package org.apache.poi.ss.formula;
+
+import org.apache.poi.hssf.record.formula.Ptg;
+import org.apache.poi.util.LittleEndianByteArrayInputStream;
+import org.apache.poi.util.LittleEndianInput;
+import org.apache.poi.util.LittleEndianOutput;
+
+public class Formula {
+
+	private static final byte[] EMPTY_BYTE_ARRAY = { };
+	private final byte[] _byteEncoding;
+	private final int _encodedTokenLen;
+	
+	private Formula(byte[] byteEncoding, int encodedTokenLen) {
+		_byteEncoding = byteEncoding;
+		_encodedTokenLen = encodedTokenLen;
+		if (false) { // set to true to eagerly check Ptg decoding 
+    		LittleEndianByteArrayInputStream in = new LittleEndianByteArrayInputStream(byteEncoding);
+    		Ptg.readTokens(encodedTokenLen, in);
+    		int nUnusedBytes = _byteEncoding.length - in.getReadIndex();
+    		if (nUnusedBytes > 0) {
+    			// TODO - this seems to occur when IntersectionPtg is present
+        		// This example file "IntersectionPtg.xls"
+        		// used by test: TestIntersectionPtg.testReading()
+        		// has 10 bytes unused at the end of the formula
+    			// 10 extra bytes are just 0x01 and 0x00
+    			System.out.println(nUnusedBytes + " unused bytes at end of formula");
+    		}
+		}
+	}
+	/**
+	 * Convenience method for {@link #read(int, LittleEndianInput, int)}
+	 */
+	public static Formula read(int encodedTokenLen, LittleEndianInput in) {
+		return read(encodedTokenLen, in, encodedTokenLen);
+	}
+	/**
+	 * When there are no array constants present, <tt>encodedTokenLen</tt>==<tt>totalEncodedLen</tt>
+	 * @param encodedTokenLen number of bytes in the stream taken by the plain formula tokens
+	 * @param totalEncodedLen the total number of bytes in the formula (includes trailing encoding
+	 * for array constants, but does not include 2 bytes for initial <tt>ushort encodedTokenLen</tt> field.
+	 * @return A new formula object as read from the stream.  Possibly empty, never <code>null</code>.
+	 */
+	public static Formula read(int encodedTokenLen, LittleEndianInput in, int totalEncodedLen) {
+		byte[] byteEncoding = new byte[totalEncodedLen];
+		in.readFully(byteEncoding);
+		return new Formula(byteEncoding, encodedTokenLen);
+	}
+	
+	public Ptg[] getTokens() {
+		LittleEndianInput in = new LittleEndianByteArrayInputStream(_byteEncoding);
+		return Ptg.readTokens(_encodedTokenLen, in);
+	}
+	/**
+	 * Writes  The formula encoding is includes:
+	 * <ul>
+	 * <li>ushort tokenDataLen</li>
+	 * <li>tokenData</li>
+	 * <li>arrayConstantData (if present)</li>
+	 * </ul>
+	 */
+	public void serialize(LittleEndianOutput out) {
+		out.writeShort(_encodedTokenLen);
+		out.write(_byteEncoding);
+	}
+
+	public void serializeTokens(LittleEndianOutput out) {
+		out.write(_byteEncoding, 0, _encodedTokenLen);
+	}
+	public void serializeArrayConstantData(LittleEndianOutput out) {
+		int len = _byteEncoding.length-_encodedTokenLen;
+		out.write(_byteEncoding, _encodedTokenLen, len);
+	}
+	
+	
+	/**
+	 * @return total formula encoding length.  The formula encoding includes:
+	 * <ul>
+	 * <li>ushort tokenDataLen</li>
+	 * <li>tokenData</li>
+	 * <li>arrayConstantData (optional)</li>
+	 * </ul>
+	 * Note - this value is different to <tt>tokenDataLength</tt>
+	 */
+	public int getEncodedSize() {
+		return 2 + _byteEncoding.length;
+	}
+	/**
+	 * This method is often used when the formula length does not appear immediately before
+	 * the encoded token data.
+	 * 
+	 * @return the encoded length of the plain formula tokens.  This does <em>not</em> include
+	 * the leading ushort field, nor any trailing array constant data.
+	 */
+	public int getEncodedTokenSize() {
+		return _encodedTokenLen;
+	}
+	
+	/**
+	 * Creates a {@link Formula} object from a supplied {@link Ptg} array. 
+	 * Handles <code>null</code>s OK.
+	 * @param ptgs may be <code>null</code>
+	 * @return Never <code>null</code> (Possibly empty if the supplied <tt>ptgs</tt> is <code>null</code>)
+	 */
+	public static Formula create(Ptg[] ptgs) {
+		if (ptgs == null) {
+			return new Formula(EMPTY_BYTE_ARRAY, 0);
+		}
+		int totalSize = Ptg.getEncodedSize(ptgs);
+		byte[] encodedData = new byte[totalSize];
+		Ptg.serializePtgs(ptgs, encodedData, 0);
+		int encodedTokenLen = Ptg.getEncodedSizeWithoutArrayData(ptgs);
+		return new Formula(encodedData, encodedTokenLen);
+	}
+	/**
+	 * Gets the {@link Ptg} array from the supplied {@link Formula}. 
+	 * Handles <code>null</code>s OK.
+	 * 
+	 * @param formula may be <code>null</code>
+	 * @return possibly <code>null</code> (if the supplied <tt>formula</tt> is <code>null</code>)
+	 */
+	public static Ptg[] getTokens(Formula formula) {
+		if (formula == null) {
+			return null;
+		}
+		return formula.getTokens();
+	}
+	
+	public Formula copy() {
+		// OK to return this for the moment because currently immutable
+		return this;
+	}
+}

src/java/org/apache/poi/ss/util/CellRangeAddress.java

@@ -18,7 +18,8 @@ package org.apache.poi.ss.util;
 
 import org.apache.poi.hssf.record.RecordInputStream;
 import org.apache.poi.hssf.record.SelectionRecord;
-import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.LittleEndianByteArrayOutputStream;
+import org.apache.poi.util.LittleEndianOutput;
 
 /**
  * See OOO documentation: excelfileformat.pdf sec 2.5.14 - 'Cell Range Address'<p/>
@@ -36,13 +37,20 @@ public class CellRangeAddress extends CellRangeAddressBase {
 		super(firstRow, lastRow, firstCol, lastCol);
 	}
 
+	/**
+	 * @deprecated use {@link #serialize(LittleEndianOutput)}
+	 */
 	public int serialize(int offset, byte[] data) {
-		LittleEndian.putUShort(data, offset + 0, getFirstRow());
+		serialize(new LittleEndianByteArrayOutputStream(data, offset, ENCODED_SIZE));
-		LittleEndian.putUShort(data, offset + 2, getLastRow());
-		LittleEndian.putUShort(data, offset + 4, getFirstColumn());
-		LittleEndian.putUShort(data, offset + 6, getLastColumn());
 		return ENCODED_SIZE;
 	}
+	public void serialize(LittleEndianOutput out) {
+		out.writeShort(getFirstRow());
+		out.writeShort(getLastRow());
+		out.writeShort(getFirstColumn());
+		out.writeShort(getLastColumn());
+	}
+	
 	public CellRangeAddress(RecordInputStream in) {
 		super(readUShortAndCheck(in), in.readUShort(), in.readUShort(), in.readUShort());
 	}

src/java/org/apache/poi/ss/util/CellRangeAddressList.java

@@ -20,7 +20,8 @@ import java.util.ArrayList;
 import java.util.List;
 
 import org.apache.poi.hssf.record.RecordInputStream;
-import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.LittleEndianByteArrayOutputStream;
+import org.apache.poi.util.LittleEndianOutput;
 
 /**
  * Implementation of the cell range address lists,like is described
@@ -122,16 +123,19 @@ public class CellRangeAddressList {
 	}
 
 	public int serialize(int offset, byte[] data) {
-		int pos = 2;
+		int totalSize = getSize();
-
+		serialize(new LittleEndianByteArrayOutputStream(data, offset, totalSize));
+		return totalSize;
+	}
+	public void serialize(LittleEndianOutput out) {
 		int nItems = _list.size();
-		LittleEndian.putUShort(data, offset, nItems);
+		out.writeShort(nItems);
 		for (int k = 0; k < nItems; k++) {
 			CellRangeAddress region = (CellRangeAddress) _list.get(k);
-			pos += region.serialize(offset + pos, data);
+			region.serialize(out);
 		}
-		return getSize();
 	}
+	
 
 	public CellRangeAddressList copy() {
 		CellRangeAddressList result = new CellRangeAddressList();

src/java/org/apache/poi/util/LittleEndianByteArrayOutputStream.java

@@ -81,6 +81,11 @@ public final class LittleEndianByteArrayOutputStream implements LittleEndianOutp
 		System.arraycopy(b, 0, _buf, _writeIndex, len);
 		_writeIndex += len;
 	}
+	public void write(byte[] b, int offset, int len) {
+		checkPosition(len);
+		System.arraycopy(b, offset, _buf, _writeIndex, len);
+		_writeIndex += len;
+	}
 	public int getWriteIndex() {
 		return _writeIndex;
 	}

src/java/org/apache/poi/util/LittleEndianOutput.java

@@ -26,5 +26,6 @@ public interface LittleEndianOutput {
 	void writeInt(int v);
 	void writeLong(long v);
 	void writeDouble(double v);
-	void write(byte[] data);
+	void write(byte[] b);
+	void write(byte[] b, int offset, int len);
 }

src/java/org/apache/poi/util/LittleEndianOutputStream.java

@@ -80,4 +80,12 @@ public final class LittleEndianOutputStream extends FilterOutputStream implement
 			throw new RuntimeException(e);
 		}
 	}
+	public void write(byte[] b, int off, int len) {
+		// suppress IOException for interface method
+		try {
+			super.write(b, off, len);
+		} catch (IOException e) {
+			throw new RuntimeException(e);
+		}
+	}
 }

src/testcases/org/apache/poi/hssf/record/TestFormulaRecord.java

@@ -17,8 +17,6 @@
 
 package org.apache.poi.hssf.record;
 
-import java.io.ByteArrayInputStream;
-
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.record.formula.AttrPtg;
@@ -152,4 +150,22 @@ public final class TestFormulaRecord extends TestCase {
 		FuncVarPtg choose = (FuncVarPtg)ptgs[8];
 		assertEquals("CHOOSE", choose.getName());
 	}
+	
+	public void testReserialize() {
+		FormulaRecord formulaRecord = new FormulaRecord();
+		formulaRecord.setRow(1);
+		formulaRecord.setColumn((short) 1);
+		formulaRecord.setParsedExpression(new Ptg[] { new RefPtg("B$5"), });
+		formulaRecord.setValue(3.3);
+		byte[] ser = formulaRecord.serialize();
+		assertEquals(31, ser.length);
+
+		RecordInputStream in = TestcaseRecordInputStream.create(ser);
+		FormulaRecord fr2 = new FormulaRecord(in);
+		assertEquals(3.3, fr2.getValue(), 0.0);
+		Ptg[] ptgs = fr2.getParsedExpression();
+		assertEquals(1, ptgs.length);
+		RefPtg rp = (RefPtg) ptgs[0];
+		assertEquals("B$5", rp.toFormulaString());
+	}
 }

src/testcases/org/apache/poi/hssf/record/TestNameRecord.java

@@ -17,46 +17,42 @@
 
 package org.apache.poi.hssf.record;
 
+import org.apache.poi.util.HexRead;
+
 import junit.framework.TestCase;
 
 /**
  * Tests the NameRecord serializes/deserializes correctly
- *
+ * 
  * @author Danny Mui (dmui at apache dot org)
  */
 public final class TestNameRecord extends TestCase {
 
-    /**
+	/**
-     * Makes sure that additional name information is parsed properly such as menu/description
+	 * Makes sure that additional name information is parsed properly such as menu/description
-     */
+	 */
-    public void testFillExtras()
+	public void testFillExtras() {
-    {
 
-        byte[] examples = {
+		byte[] examples = HexRead.readFromString(""
-            (byte) 0x88, (byte) 0x03, (byte) 0x67, (byte) 0x06,
+				+ "88 03 67 06 07 00 00 00 00 00 00 23 00 00 00 4D "
-            (byte) 0x07, (byte) 0x00, (byte) 0x00, (byte) 0x00,
+				+ "61 63 72 6F 31 3A 01 00 00 00 11 00 00 4D 61 63 "
-            (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x23,
+				+ "72 6F 20 72 65 63 6F 72 64 65 64 20 32 37 2D 53 "
-            (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0x4D,
+				+ "65 70 2D 39 33 20 62 79 20 41 4C 4C 57 4F 52");
-            (byte) 0x61, (byte) 0x63, (byte) 0x72, (byte) 0x6F,
-            (byte) 0x31, (byte) 0x3A, (byte) 0x01, (byte) 0x00,
-            (byte) 0x00, (byte) 0x00, (byte) 0x11, (byte) 0x00,
-            (byte) 0x00, (byte) 0x4D, (byte) 0x61, (byte) 0x63,
-            (byte) 0x72, (byte) 0x6F, (byte) 0x20, (byte) 0x72,
-            (byte) 0x65, (byte) 0x63, (byte) 0x6F, (byte) 0x72,
-            (byte) 0x64, (byte) 0x65, (byte) 0x64, (byte) 0x20,
-            (byte) 0x32, (byte) 0x37, (byte) 0x2D, (byte) 0x53,
-            (byte) 0x65, (byte) 0x70, (byte) 0x2D, (byte) 0x39,
-            (byte) 0x33, (byte) 0x20, (byte) 0x62, (byte) 0x79,
-            (byte) 0x20, (byte) 0x41, (byte) 0x4C, (byte) 0x4C,
-            (byte) 0x57, (byte) 0x4F, (byte) 0x52
-        };
 
+		NameRecord name = new NameRecord(TestcaseRecordInputStream.create(NameRecord.sid, examples));
+		String description = name.getDescriptionText();
+		assertNotNull(description);
+		assertTrue(description.endsWith("Macro recorded 27-Sep-93 by ALLWOR"));
+	}
 
-        NameRecord name = new NameRecord(TestcaseRecordInputStream.create(NameRecord.sid, examples));
+	public void testReserialize() {
-        String description = name.getDescriptionText();
+		byte[] data = HexRead
-        assertNotNull( description );
+				.readFromString(""
-        assertTrue( "text contains ALLWOR", description.indexOf( "ALLWOR" ) > 0 );
+						+ "20 00 00 01 0B 00 00 00 01 00 00 00 00 00 00 06 3B 00 00 00 00 02 00 00 00 09 00]");
-    }
+		RecordInputStream in = TestcaseRecordInputStream.create(NameRecord.sid, data);
+		NameRecord nr = new NameRecord(in);
+		assertEquals(0x0020, nr.getOptionFlag());
+		byte[] data2 = nr.serialize();
+		TestcaseRecordInputStream.confirmRecordEncoding(NameRecord.sid, data, data2);
+	}
 }
-
-

src/testcases/org/apache/poi/hssf/record/TestObjRecord.java

@@ -20,6 +20,8 @@ package org.apache.poi.hssf.record;
 import java.util.Arrays;
 import java.util.List;
 
+import org.apache.poi.util.HexRead;
+
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
@@ -38,17 +40,17 @@ public final class TestObjRecord extends TestCase {
      *     [ftCmo]
      *     [ftEnd]
      */
-    private static final byte[] recdata = {
+    private static final byte[] recdata = HexRead.readFromString(""
-        0x15, 0x00, 0x12, 0x00, 0x06, 0x00, 0x01, 0x00, 0x11, 0x60,
+            + "15 00 12 00 06 00 01 00 11 60 "
-        (byte)0xF4, 0x02, 0x41, 0x01, 0x14, 0x10, 0x1F, 0x02, 0x00, 0x00,
+            + "F4 02 41 01 14 10 1F 02 00 00 "
-        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
+            +"00 00 00 00 00 00"
-        // TODO - this data seems to require two extra bytes padding. not sure where original file is.
+            // TODO - this data seems to require two extra bytes padding. not sure where original file is.
-        // it's not bug 38607 attachment 17639
+            // it's not bug 38607 attachment 17639
-    };
+    );
 
-    private static final byte[] recdataNeedingPadding = {
+    private static final byte[] recdataNeedingPadding = HexRead.readFromString(""
-        21, 0, 18, 0, 0, 0, 1, 0, 17, 96, 0, 0, 0, 0, 56, 111, -52, 3, 0, 0, 0, 0, 6, 0, 2, 0, 0, 0, 0, 0, 0, 0
+            + "15 00 12 00 00 00 01 00 11 60 00 00 00 00 38 6F CC 03 00 00 00 00 06 00 02 00 00 00 00 00 00 00"
-    };
+    );
 
     public void testLoad() {
         ObjRecord record = new ObjRecord(TestcaseRecordInputStream.create(ObjRecord.sid, recdata));
@@ -113,4 +115,23 @@ public final class TestObjRecord extends TestCase {
         assertEquals(GroupMarkerSubRecord.class, subrecords.get(1).getClass());
         assertEquals(EndSubRecord.class, subrecords.get(2).getClass());
     }
+    
+    /**
+     * Check that ObjRecord tolerates and preserves padding to a 4-byte boundary
+     * (normally padding is to a 2-byte boundary).
+     */
+    public void test4BytePadding() {
+        // actual data from file saved by Excel 2007
+        byte[] data = HexRead.readFromString(""
+                + "15 00 12 00  1E 00 01 00  11 60 B4 6D  3C 01 C4 06 "
+                + "49 06 00 00  00 00 00 00  00 00 00 00");
+        // this data seems to have 2 extra bytes of padding more than usual
+        // the total may have been padded to the nearest quad-byte length
+        RecordInputStream in = TestcaseRecordInputStream.create(ObjRecord.sid, data);
+        // check read OK
+        ObjRecord record = new ObjRecord(in);
+        // check that it re-serializes to the same data
+        byte[] ser = record.serialize();
+        TestcaseRecordInputStream.confirmRecordEncoding(ObjRecord.sid, data, ser);
+    }
 }

src/testcases/org/apache/poi/hssf/record/TestSharedFormulaRecord.java

@@ -21,8 +21,15 @@ import junit.framework.AssertionFailedError;
 import junit.framework.ComparisonFailure;
 import junit.framework.TestCase;
 
+import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.record.formula.Ptg;
 import org.apache.poi.hssf.record.formula.RefPtg;
+import org.apache.poi.hssf.usermodel.HSSFCell;
+import org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator;
+import org.apache.poi.hssf.usermodel.HSSFSheet;
+import org.apache.poi.hssf.usermodel.HSSFWorkbook;
+import org.apache.poi.hssf.usermodel.RecordInspector;
+import org.apache.poi.ss.usermodel.CellValue;
 import org.apache.poi.util.LittleEndianInput;
 
 /**
@@ -30,6 +37,10 @@ import org.apache.poi.util.LittleEndianInput;
  */
 public final class TestSharedFormulaRecord extends TestCase {
 
+	/**
+	 * A sample spreadsheet known to have one sheet with 4 shared formula ranges
+	 */
+	private static final String SHARED_FORMULA_TEST_XLS = "SharedFormulaTest.xls";
 	/**
 	 * Binary data for an encoded formula.  Taken from attachment 22062 (bugzilla 45123/45421).
 	 * The shared formula is in Sheet1!C6:C21, with text "SUMPRODUCT(--(End_Acct=$C6),--(End_Bal))"
@@ -86,4 +97,113 @@ public final class TestSharedFormulaRecord extends TestCase {
 			}
 		}
 	}
+	
+	/**
+	 * Make sure that POI preserves {@link SharedFormulaRecord}s
+	 */
+	public void testPreserveOnReserialize() {
+		HSSFWorkbook wb;
+		HSSFSheet sheet;
+		HSSFCell cellB32769;
+		HSSFCell cellC32769;
+		
+		// Reading directly from XLS file
+		wb = HSSFTestDataSamples.openSampleWorkbook(SHARED_FORMULA_TEST_XLS);
+		sheet = wb.getSheetAt(0);
+		cellB32769 = sheet.getRow(32768).getCell(1);
+		cellC32769 = sheet.getRow(32768).getCell(2);
+		// check reading of formulas which are shared (two cells from a 1R x 8C range) 
+		assertEquals("B32770*2", cellB32769.getCellFormula()); 
+		assertEquals("C32770*2", cellC32769.getCellFormula());
+		confirmCellEvaluation(wb, cellB32769, 4);
+		confirmCellEvaluation(wb, cellC32769, 6);
+		// Confirm this example really does have SharedFormulas.
+		// there are 3 others besides the one at A32769:H32769
+		assertEquals(4, countSharedFormulas(sheet)); 
+		
+		
+		// Re-serialize and check again
+		wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
+		sheet = wb.getSheetAt(0);
+		cellB32769 = sheet.getRow(32768).getCell(1);
+		cellC32769 = sheet.getRow(32768).getCell(2);
+		assertEquals("B32770*2", cellB32769.getCellFormula());
+		confirmCellEvaluation(wb, cellB32769, 4);
+		assertEquals(4, countSharedFormulas(sheet));
+	}
+	
+	public void testUnshareFormulaDueToChangeFormula() {
+		HSSFWorkbook wb;
+		HSSFSheet sheet;
+		HSSFCell cellB32769;
+		HSSFCell cellC32769;
+		
+		wb = HSSFTestDataSamples.openSampleWorkbook(SHARED_FORMULA_TEST_XLS);
+		sheet = wb.getSheetAt(0);
+		cellB32769 = sheet.getRow(32768).getCell(1);
+		cellC32769 = sheet.getRow(32768).getCell(2);
+		
+		// Updating cell formula, causing it to become unshared
+		cellB32769.setCellFormula("1+1");
+		confirmCellEvaluation(wb, cellB32769, 2);
+		// currently (Oct 2008) POI handles this by exploding the whole shared formula group
+		assertEquals(3, countSharedFormulas(sheet)); // one less now
+		// check that nearby cell of the same group still has the same formula
+		assertEquals("C32770*2", cellC32769.getCellFormula());
+		confirmCellEvaluation(wb, cellC32769, 6);
+	}
+	public void testUnshareFormulaDueToDelete() {
+		HSSFWorkbook wb;
+		HSSFSheet sheet;
+		HSSFCell cell;
+		final int ROW_IX = 2;
+
+		// changing shared formula cell to blank
+		wb = HSSFTestDataSamples.openSampleWorkbook(SHARED_FORMULA_TEST_XLS);
+		sheet = wb.getSheetAt(0);
+	
+		assertEquals("A$1*2", sheet.getRow(ROW_IX).getCell(1).getCellFormula());
+		cell = sheet.getRow(ROW_IX).getCell(1);
+		cell.setCellType(HSSFCell.CELL_TYPE_BLANK);
+		assertEquals(3, countSharedFormulas(sheet));
+		
+		wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
+		sheet = wb.getSheetAt(0);
+		assertEquals("A$1*2", sheet.getRow(ROW_IX+1).getCell(1).getCellFormula());
+		
+		// deleting shared formula cell
+		wb = HSSFTestDataSamples.openSampleWorkbook(SHARED_FORMULA_TEST_XLS);
+		sheet = wb.getSheetAt(0);
+	
+		assertEquals("A$1*2", sheet.getRow(ROW_IX).getCell(1).getCellFormula());
+		cell = sheet.getRow(ROW_IX).getCell(1);
+		sheet.getRow(ROW_IX).removeCell(cell);
+		assertEquals(3, countSharedFormulas(sheet));
+		
+		wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
+		sheet = wb.getSheetAt(0);
+		assertEquals("A$1*2", sheet.getRow(ROW_IX+1).getCell(1).getCellFormula());
+	}
+
+	private static void confirmCellEvaluation(HSSFWorkbook wb, HSSFCell cell, double expectedValue) {
+		HSSFFormulaEvaluator fe = new HSSFFormulaEvaluator(wb);
+		CellValue cv = fe.evaluate(cell);
+		assertEquals(HSSFCell.CELL_TYPE_NUMERIC, cv.getCellType());
+		assertEquals(expectedValue, cv.getNumberValue(), 0.0);
+	}
+
+	/**
+	 * @return the number of {@link SharedFormulaRecord}s encoded for the specified sheet
+	 */
+	private static int countSharedFormulas(HSSFSheet sheet) {
+		Record[] records = RecordInspector.getRecords(sheet, 0);
+		int count = 0;
+		for (int i = 0; i < records.length; i++) {
+			Record rec = records[i];
+			if(rec instanceof SharedFormulaRecord) {
+				count++;
+			}
+		}
+		return count;
+	}
 }

src/testcases/org/apache/poi/hssf/usermodel/TestWorkbook.java

@@ -494,14 +494,16 @@ public final class TestWorkbook extends TestCase {
     }
 
 
-    public void testManyRows() {
+    /**
+     * Test for row indexes beyond {@link Short#MAX_VALUE}.
+     * This bug was first fixed in svn r352609.
+     */
+    public void testRowIndexesBeyond32768() {
         HSSFWorkbook workbook = new HSSFWorkbook();
         HSSFSheet sheet = workbook.createSheet();
         HSSFRow row;
         HSSFCell cell;
-        int i, j;
+        for (int i = 32700; i < 32771; i++) {
-        for ( i = 0, j = 32771; j > 0; i++, j-- )
-        {
             row = sheet.createRow(i);
             cell = row.createCell(0);
             cell.setCellValue(i);