[Pki-devel] [PATCH] 0012 - Fixes for Coverity Issues of type StringBuffer_Concatenation, Wrong_Map_Iterator, No_Equals_Method , Reverse_INull for DogTag 10

[Endi Sukma Dewata]

On 6/13/2012 4:31 PM, Abhishek Koneru wrote:
> Please find attached PATCH 12 - with fixes for issues in Coverity of
> types StringBuffer_Concatenation, Wrong_Map_Iterator, No_Equals_Method ,
> Reverse_INull for review.

Some issues:

1. There's a formatting issue in CMSCRLExtensions.java:586.

2. In CRLIssuingPoint.java:2712-2737 the String concatenations are 
replaced with StringBuffer. This is fine and not a big deal but it can 
be done more efficiently. See http://kaioa.com/node/59 and 
http://javarevisited.blogspot.com/2011/07/string-vs-stringbuffer-vs-stringbuilder.html.

In this code the only thread using the StringBuffer is the current 
thread itself, so there's no concurrency issue. So the StringBuffer can 
be replaced with StringBuilder.

Every time you do a "+" operation on Strings, the JVM will create a 
StringBuilder, append the Strings, then generate a new String that 
contains the concatenated text. So instead of this:

   splitTimes.append(
       "," + Long.toString(deltaTime) + "," + Long.toString(crlTime)
       + "," + Long.toString(totalTime) + ")");

you can reuse the same StringBuilder object like this:

   splitTimes.append(",").append(deltaTime).append(",").append(crlTime)
       .append(",").append(totalTime).append(")");

Notice the Long.toString() is not necessary because there are separate 
append() methods for each primitive type. Also with autoboxing the 
primitive types will be converted automatically to objects, so the code 
in lines 2722, 2729-2738 can be simplified like this:

   splitTimes.append(mSplits[i]);

   new Object[] {
       getId(),
       getCRLNumber(),
       getLastUpdate(),
       getNextUpdate(),
       mCRLSize,
       totalTime,
       crlTime,
       "" + deltaTime + splitTimes
   }

3. In CRLIssuingPoint.java:3098-3105,3151 whenever you remove the { } 
surrounding a code block you should fix the indentation.

4. In NameValuePairs.java:47 you can use StringBuilder too. In line 51 
it's better to use separate append() for each String component.

5. In SimpleProperties.java you should use Eclipse to generate 
hashCode() too, it goes together with equals().

6. In Auditor.java the StringBuilder can be created once at line 99, 
then the subsequent code will append each String component separately.

7. In ACL.java:153 you can use StringBuilder. In lines 159 and 163 it's 
not necessary to call toString() explicitly.

8. In ExtDataHashtable.java:59 I think you can actually remove the 
putAll() because it's identical to the one in the super class.

9. In RequestTest.java you should add hashCode() as well.

10. Same thing in CMCResponse.java:125-130, use StringBuilder, separate 
appends, no explicit toString().

11. Same thing in PrettyPrintCert.java:89,200,221, use StringBuilder, 
separate appends, no explicit toString().

12. Add hashCode() to the following files:
  - CMSProperties.java
  - PKCS10Attributes.java
  - DSAPrivateKey.java
  - DSAPublicKey.java
  - RSAPublicKey.java
  - AlgIdDSA.java
  - CRLExtensions.java
  - CertificateExtensions.java
  - Extensions.java

-- 
Endi S. Dewata