[Pki-devel] [PATCH] 13 -- Fixes for Coverity Issues of type NULL_RETURNS for review

[Endi Sukma Dewata]

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".

-- 
Endi S. Dewata