1270005 - Replace uses of ScopedPK11SlotInfo with UniquePK11SlotInfo in PSM

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[:Cykesiopka]

ScopedPK11SlotInfo is based on Scoped.h, which is deprecated in favour of the
standardised UniquePtr.

Comment 1

[:Cykesiopka]

Attachment: 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/

Comment 2

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

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

Comment 3

[:Cykesiopka]

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 4

[:Cykesiopka]

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/

Comment 5

[:Cykesiopka]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee54b7cbad13f1a20b630bb3ec577b85af676739

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/222ef20fe633

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/222ef20fe633