On 6/15/2012 3:51 PM, Abhishek Koneru wrote: > Please review the patch which has the fixes for Coverity issues of type > NULL_RETURNS. Some comments: 1. In UGSubsystem.java:421 if uid is null it will throw an exception with this message: "No User with UID "+uid. Since the uid is null the message is not helpful for troubleshooting. It would be better to say something like "No Attribute UID in LDAP Entry "+entry.getDN(). 2. Same issue in UGSubsystem.java:480. 3. Formatting issue in UGSubsystem.java:1218. 4. In UGSubsystem.java:1647 the bitwise XOR operator is used on boolean values. This is fine but I'd rather not do it because it can be done with logical operator too. The extra parenthesis is not needed because of operator precedence. So the code may look like this: // if both are null return true if (rdn1 == null && rdn2 == null) { return true; } // if one of them is null return false if (rdn1 == null || rdn2 == null) { return false; } // at this point none of them is null 5. In RequestTest.java the checkReturnValue() might not be correct. If the retVal is null all instanceof checking will be false, so type will be empty string. Some of the tests already check the retVal using assertNotNull() which should throw an exception if it's null. Let's make sure the other tests do the same thing and see if Coverity still complains. 6. In RecoveryService.java:454,594 if the code couldn't find the certificate from the request it will throw CMS_KRA_INVALID_KEYRECORD. It might be better to throw CMS_KRA_PKCS12_FAILED_1 with additional parameter "Missing certificate".