Closed Bug 1270005 Opened 4 years ago Closed 4 years ago

Replace uses of ScopedPK11SlotInfo with UniquePK11SlotInfo in PSM

Categories

(Core :: Security: PSM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: Cykesiopka, Assigned: Cykesiopka)

References

Details

(Whiteboard: [psm-assigned])

Attachments

(1 file)

ScopedPK11SlotInfo is based on Scoped.h, which is deprecated in favour of the
standardised UniquePtr.
Whiteboard: [psm-assigned]
ScopedPK11SlotInfo is based on Scoped.h, which is deprecated in favour of the
standardised UniquePtr.

Also changes PK11SlotInfo parameters of various functions to make ownership more
explicit, and replaces some manual management of PK11SlotInfo pointers.

Review commit: https://reviewboard.mozilla.org/r/51235/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/51235/
Attachment #8749891 - Flags: review?(dkeeler)
Comment on attachment 8749891 [details]
MozReview Request: Bug 1270005 - Replace uses of ScopedPK11SlotInfo with UniquePK11SlotInfo in PSM. r=keeler

https://reviewboard.mozilla.org/r/51235/#review48231

Great - r=me.

::: security/manager/ssl/nsNSSCertificateDB.cpp:75
(Diff revision 1)
>      return MapSECStatus(SECFailure);
>    }
> -  if (PK11_NeedUserInit(slot)) {
> +  if (PK11_NeedUserInit(slot.get())) {
>      // Ignore the return value. Presumably PK11_InitPin will fail if the user
>      // has a non-default password.
> -    (void) PK11_InitPin(slot, nullptr, nullptr);
> +    (void) PK11_InitPin(slot.get(), nullptr, nullptr);

Technically this is unrelated, but it might be good to use Unused here to indicate we're deliberately ignoring the return value.

::: security/manager/ssl/nsNSSCertificateDB.cpp:1050
(Diff revision 1)
> -    ScopedPK11SlotInfo keySlot(PK11_GetInternalKeySlot());
> -    NS_ASSERTION(keySlot,"Failed to get the internal key slot");
> -    localRef = new nsPK11Token(keySlot);
> +    UniquePK11SlotInfo keySlot(PK11_GetInternalKeySlot());
> +    if (!keySlot) {
> +      return NS_ERROR_FAILURE;
> +    }
> +    localRef = new nsPK11Token(keySlot.get());
>    }

nit: maybe update this to } else { while we're here
Attachment #8749891 - Flags: review?(dkeeler) → review+
https://reviewboard.mozilla.org/r/51235/#review48231

Thanks for the review!

> Technically this is unrelated, but it might be good to use Unused here to indicate we're deliberately ignoring the return value.

Sounds good, will do.
Comment on attachment 8749891 [details]
MozReview Request: Bug 1270005 - Replace uses of ScopedPK11SlotInfo with UniquePK11SlotInfo in PSM. r=keeler

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51235/diff/1-2/
Attachment #8749891 - Attachment description: MozReview Request: Bug 1270005 - Replace uses of ScopedPK11SlotInfo with UniquePK11SlotInfo in PSM. → MozReview Request: Bug 1270005 - Replace uses of ScopedPK11SlotInfo with UniquePK11SlotInfo in PSM. r=keeler
https://hg.mozilla.org/mozilla-central/rev/222ef20fe633
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.