From 06f1a6a42eb825e6cfb18567306d8e89299b31fe Mon Sep 17 00:00:00 2001 From: Andreas Beeker Date: Sun, 16 Feb 2014 19:35:16 +0000 Subject: [PATCH] FindBugs fix - fixed "equals() method does not check for null argument" - see http://findbugs.sourceforge.net/bugDescriptions.html#NP_EQUALS_SHOULD_HANDLE_NULL_ARGUMENT git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1568812 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/poi/POIXMLDocumentPart.java | 2 +- .../org/apache/poi/TestPOIXMLDocument.java | 2 +- .../src/org/apache/poi/hwpf/model/Ffn.java | 57 +++++-------------- .../org/apache/poi/hwpf/model/FontTable.java | 33 ++++------- .../poi/hwpf/model/ParagraphHeight.java | 1 + .../apache/poi/hwpf/model/PropertyNode.java | 2 + .../poi/hwpf/model/SectionDescriptor.java | 1 + .../org/apache/poi/hwpf/model/StyleSheet.java | 32 +++++------ .../org/apache/poi/hwpf/model/TextPiece.java | 17 +++--- .../apache/poi/hwpf/model/TextPieceTable.java | 1 + .../src/org/apache/poi/hwpf/model/UPX.java | 1 + .../org/apache/poi/hwpf/sprm/SprmBuffer.java | 1 + .../apache/poi/hwpf/usermodel/BorderCode.java | 1 + .../poi/hwpf/usermodel/DateAndTime.java | 1 + .../hwpf/usermodel/LineSpacingDescriptor.java | 1 + 15 files changed, 61 insertions(+), 92 deletions(-) diff --git a/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java b/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java index de889d9df1..7f4ee8ce33 100644 --- a/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java +++ b/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java @@ -284,7 +284,7 @@ public class POIXMLDocumentPart { @Override public String toString(){ - return packagePart == null ? null : packagePart.toString(); + return packagePart == null ? "" : packagePart.toString(); } /** diff --git a/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java b/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java index c6d2bd7058..3aca24f36c 100644 --- a/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java +++ b/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java @@ -170,7 +170,7 @@ public final class TestPOIXMLDocument extends TestCase { assertNull(part.getRelationId(null)); assertFalse(part.removeRelation(null, true)); part.removeRelation(null); - assertNull(part.toString()); + assertEquals("",part.toString()); part.onDocumentCreate(); //part.getTargetPart(null); } diff --git a/src/scratchpad/src/org/apache/poi/hwpf/model/Ffn.java b/src/scratchpad/src/org/apache/poi/hwpf/model/Ffn.java index 6c8666f7aa..4f469dad09 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/model/Ffn.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/model/Ffn.java @@ -182,49 +182,20 @@ public final class Ffn } @Override - public boolean equals(Object o) - { - boolean retVal = true; - - if (((Ffn)o).get_cbFfnM1() == _cbFfnM1) - { - if(((Ffn)o)._info == _info) - { - if(((Ffn)o)._wWeight == _wWeight) - { - if(((Ffn)o)._chs == _chs) - { - if(((Ffn)o)._ixchSzAlt == _ixchSzAlt) - { - if(Arrays.equals(((Ffn)o)._panose,_panose)) - { - if(Arrays.equals(((Ffn)o)._fontSig,_fontSig)) - { - if(!(Arrays.equals(((Ffn)o)._xszFfn,_xszFfn))) - retVal = false; - } - else - retVal = false; - } - else - retVal = false; - } - else - retVal = false; - } - else - retVal = false; - } - else - retVal = false; - } - else - retVal = false; - } - else - retVal = false; - - return retVal; + public boolean equals(Object other) { + if (!(other instanceof Ffn)) return false; + Ffn o = (Ffn)other; + + return ( + o._cbFfnM1 == this._cbFfnM1 + && o._info == this._info + && o._wWeight == _wWeight + && o._chs == _chs + && o._ixchSzAlt == _ixchSzAlt + && Arrays.equals(o._panose,_panose) + && Arrays.equals(o._fontSig,_fontSig) + && Arrays.equals(o._xszFfn,_xszFfn) + ); } diff --git a/src/scratchpad/src/org/apache/poi/hwpf/model/FontTable.java b/src/scratchpad/src/org/apache/poi/hwpf/model/FontTable.java index 0abd236a07..bedf1e59fe 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/model/FontTable.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/model/FontTable.java @@ -138,29 +138,20 @@ public final class FontTable } @Override - public boolean equals(Object o) - { - boolean retVal = true; + public boolean equals(Object other) { + if (!(other instanceof FontTable)) return false; + FontTable o = (FontTable)other; - if(((FontTable)o).getStringCount() == _stringCount) - { - if(((FontTable)o).getExtraDataSz() == _extraDataSz) - { - Ffn[] fontNamesNew = ((FontTable)o).getFontNames(); - for(int i = 0;i<_stringCount; i++) - { - if(!(_fontNames[i].equals(fontNamesNew[i]))) - retVal = false; - } + if (o._stringCount != this._stringCount + || o._extraDataSz != this._extraDataSz + || o._fontNames.length != this._fontNames.length + ) return false; + + for (int i=0; i> implements Compar public boolean equals(Object o) { + if (!(o instanceof PropertyNode)) return false; + if (limitsAreEqual(o)) { Object testBuf = ((PropertyNode)o)._buf; diff --git a/src/scratchpad/src/org/apache/poi/hwpf/model/SectionDescriptor.java b/src/scratchpad/src/org/apache/poi/hwpf/model/SectionDescriptor.java index bdddac7f3e..fead9ebcc7 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/model/SectionDescriptor.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/model/SectionDescriptor.java @@ -80,6 +80,7 @@ public final class SectionDescriptor @Override public boolean equals(Object o) { + if (!(o instanceof SectionDescriptor)) return false; SectionDescriptor sed = (SectionDescriptor)o; return sed.fn == fn && sed.fnMpr == fnMpr; } diff --git a/src/scratchpad/src/org/apache/poi/hwpf/model/StyleSheet.java b/src/scratchpad/src/org/apache/poi/hwpf/model/StyleSheet.java index 98df05f956..e780a486af 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/model/StyleSheet.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/model/StyleSheet.java @@ -178,28 +178,22 @@ public final class StyleSheet implements HDFType { @Override public boolean equals(Object o) { + if (!(o instanceof StyleSheet)) return false; StyleSheet ss = (StyleSheet)o; - if (ss._stshif.equals( this._stshif ) && ss._cbStshi == _cbStshi) - { - if (ss._styleDescriptions.length == _styleDescriptions.length) - { - for (int x = 0; x < _styleDescriptions.length; x++) - { - // check for null - if (ss._styleDescriptions[x] != _styleDescriptions[x]) - { - // check for equality - if (!ss._styleDescriptions[x].equals(_styleDescriptions[x])) - { - return false; - } - } - } - return true; - } + if (!ss._stshif.equals( this._stshif ) + || ss._cbStshi != this._cbStshi + || ss._styleDescriptions.length != this._styleDescriptions.length + ) return false; + + for (int i=0; i<_styleDescriptions.length; i++) { + StyleDescription tsd = this._styleDescriptions[i]; + StyleDescription osd = ss._styleDescriptions[i]; + if (tsd == null && osd == null) continue; + if (tsd == null || osd == null || !osd.equals(tsd)) return false; } - return false; + + return true; } @Override diff --git a/src/scratchpad/src/org/apache/poi/hwpf/model/TextPiece.java b/src/scratchpad/src/org/apache/poi/hwpf/model/TextPiece.java index 0c3e8c3725..22a55c53f6 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/model/TextPiece.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/model/TextPiece.java @@ -204,13 +204,16 @@ public class TextPiece extends PropertyNode @Override public boolean equals(Object o) { - if (limitsAreEqual(o)) - { - TextPiece tp = (TextPiece)o; - return getStringBuilder().toString().equals(tp.getStringBuilder().toString()) && - tp._usesUnicode == _usesUnicode && _pd.equals(tp._pd); - } - return false; + if (!(o instanceof TextPiece)) return false; + TextPiece tp = (TextPiece)o; + assert(_buf != null && tp._buf != null && _pd != null && tp._pd != null); + + return ( + limitsAreEqual(o) + && tp._usesUnicode == this._usesUnicode + && tp._buf.toString().equals(this._buf.toString()) + && tp._pd.equals(this._pd) + ); } @Override diff --git a/src/scratchpad/src/org/apache/poi/hwpf/model/TextPieceTable.java b/src/scratchpad/src/org/apache/poi/hwpf/model/TextPieceTable.java index facb4e472c..11a8d604d9 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/model/TextPieceTable.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/model/TextPieceTable.java @@ -160,6 +160,7 @@ public class TextPieceTable implements CharIndexTranslator public boolean equals( Object o ) { + if (!(o instanceof TextPieceTable)) return false; TextPieceTable tpt = (TextPieceTable) o; int size = tpt._textPieces.size(); diff --git a/src/scratchpad/src/org/apache/poi/hwpf/model/UPX.java b/src/scratchpad/src/org/apache/poi/hwpf/model/UPX.java index 95dd00bc30..5de2fe5248 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/model/UPX.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/model/UPX.java @@ -43,6 +43,7 @@ public final class UPX @Override public boolean equals(Object o) { + if (!(o instanceof UPX)) return false; UPX upx = (UPX)o; return Arrays.equals(_upx, upx._upx); } diff --git a/src/scratchpad/src/org/apache/poi/hwpf/sprm/SprmBuffer.java b/src/scratchpad/src/org/apache/poi/hwpf/sprm/SprmBuffer.java index 440c666403..73120db367 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/sprm/SprmBuffer.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/sprm/SprmBuffer.java @@ -154,6 +154,7 @@ public final class SprmBuffer implements Cloneable @Override public boolean equals(Object obj) { + if (!(obj instanceof SprmBuffer)) return false; SprmBuffer sprmBuf = (SprmBuffer)obj; return (Arrays.equals(_buf, sprmBuf._buf)); } diff --git a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/BorderCode.java b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/BorderCode.java index 47e3b352f9..a61db1bd24 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/BorderCode.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/BorderCode.java @@ -71,6 +71,7 @@ public final class BorderCode implements Cloneable { @Override public boolean equals(Object o) { + if (!(o instanceof BorderCode)) return false; BorderCode brc = (BorderCode)o; return _info == brc._info && _info2 == brc._info2; } diff --git a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/DateAndTime.java b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/DateAndTime.java index 9000cc1ec8..a9b2fd9da8 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/DateAndTime.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/DateAndTime.java @@ -75,6 +75,7 @@ public final class DateAndTime @Override public boolean equals(Object o) { + if (!(o instanceof DateAndTime)) return false; DateAndTime dttm = (DateAndTime)o; return _info == dttm._info && _info2 == dttm._info2; } diff --git a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/LineSpacingDescriptor.java b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/LineSpacingDescriptor.java index e327f039ce..6e6b8020ad 100644 --- a/src/scratchpad/src/org/apache/poi/hwpf/usermodel/LineSpacingDescriptor.java +++ b/src/scratchpad/src/org/apache/poi/hwpf/usermodel/LineSpacingDescriptor.java @@ -75,6 +75,7 @@ public final class LineSpacingDescriptor @Override public boolean equals(Object o) { + if (!(o instanceof LineSpacingDescriptor)) return false; LineSpacingDescriptor lspd = (LineSpacingDescriptor)o; return _dyaLine == lspd._dyaLine && _fMultiLinespace == lspd._fMultiLinespace;