[Pki-devel] [PATCH 11-3] -- Re: [PATCH 11-2] -- Re: [PATCH 11] Dogtag 10 - Fixes for Coverity issues relating to NULL_RETURNS Category -- for review

Abhishek Koneru akoneru at redhat.com
Wed Jun 13 16:04:22 UTC 2012


Please find attached PATCH 11-3 for review and commit.
All the fixes for review comments included.

--Abhishek Koneru
On Tue, 2012-06-12 at 23:15 -0500, Endi Sukma Dewata wrote:
> On 6/12/2012 4:53 PM, Abhishek Koneru wrote:
> > Submitting the PATCH 11-2 with fixes for review comments given.
> > Build passed the smoke test.
> 
> >> 1. The link to JSS jar file should have been
> >> /usr/share/java/jss/jss4.jar. This link is created by scripts/dev_setup.
> 
> The .classpath is not fixed yet.
> 
> >> 12. There's a formatting issue in DBSubsystem.java:411. Also, it would
> >> be better to include which entry that has a problem:
> >>
> >>     throw new Exception(
> >>         "Missing attribute " + PROP_NEXT_RANGE + " in entry " + dn);
> 
> The new code shows the constant name (PROP_NEXT_RANGE) instead of the 
> actual attribute name (nextRange):
> 
>         throw new Exception(
>             "Missing Attribute PROP_NEXT_RANGE in Entry" + dn);
> 
> >> 20. In RecoveryService.java:200 it might be better to throw
> >> EKRAException with user message CMS_KRA_INVALID_KEYRECORD. This may make
> >> some other changes unnecessary.
> 
> The new code gets the message from LogMessages instead of UserMessages:
> 
>    throw new EKRAException(
>        CMS.getLogMessage("CMS_KRA_INVALID_KEYRECORD"));
> 
> 
> Some more issues:
> 
> 28. In IAuthToken.java:218 the @return tag should be fixed, the method 
> no longer returns null on error.
> 
> 29. In IService.java:47 the new @throws tag is no longer needed.
> 
> 30. Formatting issue in CMCAuth.java:907.
> 
> 31. In ChallengeRevocationServlet1.java:135 the @throws tag doesn't 
> match the exception.
> 
> 32. In AuthTokenTest.java:144 include an exception message, for example: 
> Unable to get key as String array.
> 
> 33. In RecoveryService.java:132 the new @throws tag is no longer needed.
> 
> 34. In CMSLDAP.java:212 it should throw the original exception object.
> 
> 35. In HTTPClient.java:424 the inner try-catch block is actually not 
> needed anymore. The finally-clause (lines 432-441) can be moved to the 
> outer try-catch block at line 447. The null assignments (lines 438-440) 
> are not necessary because these objects will be garbage collected 
> anyway. Usually objects that require closing are handled like this:
> 
>    // declare variables and initialize with null
>    Socket socket = null;
>    OutputStream rawos = null;
>    BufferedOutputStream os = null;
>    PrintStream ps = null;
> 
>    try {
>        // create objects
>        socket = new Socket(hostname, port);
>        rawos = socket.getOutputStream();
>        os = new BufferedOutputStream(rawos);
>        ps = new PrintStream(os);
>        ...
> 
>    } catch (Exception e) {
>        ...
>        throw e;
> 
>    } finally {
>        // close in reverse order and ignore any errors
>        if (ps != null)
>            try { ps.close(); }
>            catch (Exception e) { e.printStackTrace(); }
>        if (rawos != null)
>            try { rawos.close(); }
>            catch (Exception e) { e.printStackTrace(); }
>        if (os != null)
>            try { os.close(); }
>            catch (Exception e) { e.printStackTrace(); }
>        if (socket != null)
>            try { socket.close(); }
>            catch (Exception e) { e.printStackTrace(); }
>    }
> 
> You probably can just close the outermost stream (the one created last, 
> i.e. ps), it should automatically close the other streams and the socket 
> too.
> 
> 36. Formatting issue in HTTPClient.java:1196.
> 
> 37. In DerInputStream.java:115 the @throws tag doesn't match the exception.
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: pki-akoneru-0011-3-Fixes-for-Review-Comments.patch
Type: text/x-patch
Size: 12489 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20120613/ebb98ab2/attachment.bin>


More information about the Pki-devel mailing list