Closed Bug 1362735 Opened 3 years ago Closed 3 years ago

Clean up nsIPKCS11 implementation

Categories

(Core :: Security: PSM, enhancement, P1, minor)

enhancement

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: Cykesiopka, Assigned: Cykesiopka)

Details

(Whiteboard: [psm-assigned])

Attachments

(1 file)

The implementation of nsIPKCS11, nsCrypto.(cpp|h), could use some cleaning up:
  1. The file names are confusing and difficult to discover. The implementation of nsIPKCS11 should be named nsPKCS11 or just PKCS11.
  2. The code isn't namespaced.
  3. nsCrypto.h is unnecessarily exported.
Comment on attachment 8867489 [details]
Bug 1362735 - Clean up nsIPKCS11 implementation.

https://reviewboard.mozilla.org/r/139034/#review142592

Cool - lgtm. Just the one comment about nsDOMCID.h.

::: security/manager/ssl/nsNSSModule.cpp:18
(Diff revision 1)
>  #include "mozilla/ModuleUtils.h"
>  #include "nsCURILoader.h"
>  #include "nsCertOverrideService.h"
> -#include "nsCrypto.h"
>  #include "nsCryptoHash.h"
> -#include "nsDOMCID.h" // For the NS_CRYPTO_CONTRACTID define
> +#include "nsDOMCID.h" // For the NS_PKCS11_CONTRACTID define

Looks like nsDOMCID.h has

```
#define NS_CRYPTO_CONTRACTID "@mozilla.org/security/crypto;1"
#define NS_PKCS11_CONTRACTID "@mozilla.org/security/pkcs11;1"
```

but `NS_CRYPTO_CONTRACTID` doesn't appear to be used anywhere and `NS_PKCS11_CONTRACTID` is only used in this file, so I imagine we can (re)move them from that file (and then remove this #include).
Attachment #8867489 - Flags: review?(dkeeler) → review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/910f57fb56bf
Clean up nsIPKCS11 implementation. r=keeler
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/910f57fb56bf
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.