5:24 p.m.
Please find attached the patch with fixes for Forward_Null type issues
in Coverity for DogTag 10 fro review.
Regards,
Abhishek Koneru
10:58 p.m.
Some comments:
1. In CMSEngine.java (getPasswordStore()) , you solve one case where a
null pointer dereference can take place (i.e. the exception thrown).
But there is another. What happens if mPasswordStore == null and
pwdClass == null ? Can pwdClass be null? If not, then we should remove
the conditional.
2. In JssSubsystem.java, when throwing the exceptions, add the name of
the function. An example is in getCACerts(). In getCACerts(), there is
also a formatting problem:
certs =
CryptoManager.getInstance().getCACerts();
This happens in a bunch of places in this file -- please fix these.
3. Your solution in X509CRLImpl.java is not very intuitive. I prefer a
simple check ...
if ((sigProvider != null) && (sigProvider.equals("...")) { ..
4. In GenericASN1Extension.java, rather than initializing s to "", I
would prefer to keep the initialization to null. You will then need an
explicit check for s == null after reading from the file. If s is still
null at that point, return ""
5. In DRMTool.java, I'm not convinced your solution preserves the
original intent of the code. There is a difference between
previous_line = null and previous_line = "". I would suggest that you
talk with mharmsen to find the correct solution to this problem.
On Mon, 2012-06-25 at 18:24 -0400, Abhishek Koneru wrote:
Please find attached the patch with fixes for Forward_Null type
issues
in Coverity for DogTag 10 fro review.
Regards,
Abhishek Koneru
_______________________________________________
Pki-devel mailing list
Pki-devel(a)redhat.com
https://www.redhat.com/mailman/listinfo/pki-devel
2:27 p.m.
New subject: [Patch] 16-2 Fixes for review comments on -- [PATCH] 16 Fixes for Coverity Issues of type Forward Null for DogTag 10 fro review.
Please review the attached patch with fixes for the below comments.
Regards,
Abhishek Koneru
On Tue, 2012-06-26 at 23:58 -0400, Ade Lee wrote:
Some comments:
1. In CMSEngine.java (getPasswordStore()) , you solve one case where a
null pointer dereference can take place (i.e. the exception thrown).
But there is another. What happens if mPasswordStore == null and
pwdClass == null ? Can pwdClass be null? If not, then we should remove
the conditional.
2. In JssSubsystem.java, when throwing the exceptions, add the name of
the function. An example is in getCACerts(). In getCACerts(), there is
also a formatting problem:
certs =
CryptoManager.getInstance().getCACerts();
This happens in a bunch of places in this file -- please fix these.
3. Your solution in X509CRLImpl.java is not very intuitive. I prefer a
simple check ...
if ((sigProvider != null) && (sigProvider.equals("...")) { ..
4. In GenericASN1Extension.java, rather than initializing s to "", I
would prefer to keep the initialization to null. You will then need an
explicit check for s == null after reading from the file. If s is still
null at that point, return ""
5. In DRMTool.java, I'm not convinced your solution preserves the
original intent of the code. There is a difference between
previous_line = null and previous_line = "". I would suggest that you
talk with mharmsen to find the correct solution to this problem.
On Mon, 2012-06-25 at 18:24 -0400, Abhishek Koneru wrote:
> Please find attached the patch with fixes for Forward_Null type issues
> in Coverity for DogTag 10 fro review.
>
> Regards,
> Abhishek Koneru
> _______________________________________________
> Pki-devel mailing list
> Pki-devel(a)redhat.com
> https://www.redhat.com/mailman/listinfo/pki-devel
12:30 a.m.
New subject: [Patch] 16-2 Fixes for review comments on -- [PATCH] 16 Fixes for Coverity Issues of type Forward Null for DogTag 10 fro review.
ACK - pushed to master.
On Thu, 2012-06-28 at 15:27 -0400, Abhishek Koneru wrote:
Please review the attached patch with fixes for the below comments.
Regards,
Abhishek Koneru
On Tue, 2012-06-26 at 23:58 -0400, Ade Lee wrote:
> Some comments:
>
> 1. In CMSEngine.java (getPasswordStore()) , you solve one case where a
> null pointer dereference can take place (i.e. the exception thrown).
> But there is another. What happens if mPasswordStore == null and
> pwdClass == null ? Can pwdClass be null? If not, then we should remove
> the conditional.
>
> 2. In JssSubsystem.java, when throwing the exceptions, add the name of
> the function. An example is in getCACerts(). In getCACerts(), there is
> also a formatting problem:
>
> certs =
> CryptoManager.getInstance().getCACerts();
>
> This happens in a bunch of places in this file -- please fix these.
>
> 3. Your solution in X509CRLImpl.java is not very intuitive. I prefer a
> simple check ...
> if ((sigProvider != null) && (sigProvider.equals("...")) { ..
>
> 4. In GenericASN1Extension.java, rather than initializing s to "", I
> would prefer to keep the initialization to null. You will then need an
> explicit check for s == null after reading from the file. If s is still
> null at that point, return ""
>
> 5. In DRMTool.java, I'm not convinced your solution preserves the
> original intent of the code. There is a difference between
> previous_line = null and previous_line = "". I would suggest that you
> talk with mharmsen to find the correct solution to this problem.
>
> On Mon, 2012-06-25 at 18:24 -0400, Abhishek Koneru wrote:
> > Please find attached the patch with fixes for Forward_Null type issues
> > in Coverity for DogTag 10 fro review.
> >
> > Regards,
> > Abhishek Koneru
> > _______________________________________________
> > Pki-devel mailing list
> > Pki-devel(a)redhat.com
> > https://www.redhat.com/mailman/listinfo/pki-devel
>
>
4622
days inactive
4626
days old
3 comments
2 participants
participants (2)
-
Abhishek Koneru -
Ade Lee