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

Abhishek Koneru akoneru at redhat.com
Tue Jun 19 15:53:23 UTC 2012


Please find attached the patch with fixes for the review comments given
for PATCH 13.

Regards,
Abhishek Koneru

On Mon, 2012-06-18 at 16:10 -0500, Endi Sukma Dewata wrote:
> 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".
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: pki-akoneru-0013-1-Fixes-for-Review-Comments-for-Patch-13.patch
Type: text/x-patch
Size: 8669 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20120619/4f491406/attachment.bin>


More information about the Pki-devel mailing list