Replace uses of ScopedPK11SlotInfo with UniquePK11SlotInfo in PSM

RESOLVED FIXED in Firefox 49

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Cykesiopka, Assigned: Cykesiopka)

Tracking

unspecified
mozilla49
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

(Whiteboard: [psm-assigned])

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
ScopedPK11SlotInfo is based on Scoped.h, which is deprecated in favour of the
standardised UniquePtr.
Whiteboard: [psm-assigned]
(Assignee)

Comment 1

3 years ago
Created attachment 8749891 [details]
MozReview Request: Bug 1270005 - Replace uses of ScopedPK11SlotInfo with UniquePK11SlotInfo in PSM. r=keeler

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+
(Assignee)

Comment 3

3 years ago
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.
(Assignee)

Comment 4

3 years ago
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

Comment 7

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/222ef20fe633
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.