[PATCH] 17 Added Null Value Handling for all implementations of IDBAttrMapper: mapObjectToLDAPAttributeSet as part of Forward Null Coverity Fixes for DogTag10
10:23 a.m.
Please find attached the patch which handles null object passed to map
to an LDAP Attribute for review.
Regards,
Abhishek Koneru
11:15 a.m.
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.
--
Endi S. Dewata
2:35 p.m.
New subject: [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
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.
2:36 p.m.
New subject: [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
Sorry for the spam. Forgot to add the patch.
Thanks,
Abhishek Koneru
On Thu, 2012-06-28 at 15:35 -0400, Abhishek Koneru wrote:
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.
>
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
2:22 p.m.
New subject: [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
On 6/28/2012 2:36 PM, Abhishek Koneru wrote:
Sorry for the spam. Forgot to add the patch.
Thanks,
Abhishek Koneru
On Thu, 2012-06-28 at 15:35 -0400, Abhishek Koneru wrote:
> Please review the patch 17-2 with fixes for review comments given for
> Patch 17.
ACK. Merged & pushed to master.
--
Endi S. Dewata
4558
days inactive
4561
days old
4 comments
2 participants
participants (2)
-
Abhishek Koneru -
Endi Sukma Dewata