[Pki-devel] [PATCH] 17-2 Fixes for review comments on -- [PATCH] 17 Added Null Value Handling for all implementations of IDBAttrMapper: mapObjectToLDAPAttributeSet as part of Forward Null Coverity Fixes for DogTag10

[Abhishek Koneru]

Please review the patch 17-2 with fixes for review comments given for
Patch 17.

Regards,
Abhishek Koneru
On Wed, 2012-06-27 at 11:15 -0500, Endi Sukma Dewata wrote:
> On 6/26/2012 10:23 AM, Abhishek Koneru wrote:
> > Please find attached the patch which handles null object passed to map
> > to an LDAP Attribute for review.
> 
> 1. In DateArrayMapper.java:68 to be consistent let's check the original 
> parameter obj instead of dates and move it to the beginning of the method.
> 
> 2. Formatting issue in KeyRecordMapper.java:62.
> 
> 3. In LongMapper.LongToDB() the patch checks for null parameter then 
> return a null. The original code would have thrown an exception. For 
> mapObjectToLDAPAttributeSet() it doesn't matter because the null obj is 
> checked earlier, but there are other places that call LongToDB() 
> directly and probably expect an exception. I'd suggest we revert this 
> particular change.
> 
> 4. This is not a problem, so you don't have to change it. Just FYI, in 
> RequestRecord.java:501 instead of replacing close() with flush() you 
> could also move the next statement outside of the try-finally block. 
> This way it guarantees that the stream is already closed before you use it.
> 
> 5. In RequestRecord.java:511 the exception was added inside a commented 
> block:
> 
>    //if (Debug.ON) {
>    ...
>    throw new EBaseException(...);
>    //}
> 
> Suppose one day the block is uncommented, the exception will be thrown 
> only during debugging. I'd suggest we move it after the block.
>