Closed Bug 1366584 Opened 4 years ago Closed 3 years ago

Add initial [must_use] properties to PSM IDL files

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: Cykesiopka, Assigned: Cykesiopka)

References

(Blocks 1 open bug)

Details

(Whiteboard: [psm-assigned])

Attachments

(1 file)

The [must_use] property on XPIDL methods and attributes is useful for making sure errors are properly handled.

As a first step, we should add the property to PSM methods and attributes that are already correctly checked everywhere.
Comment on attachment 8870899 [details]
Bug 1366584 - Add initial [must_use] properties to PSM IDL files.

https://reviewboard.mozilla.org/r/142472/#review146094

Cool - looks good to me.

::: security/manager/ssl/nsICertificateDialogs.idl:23
(Diff revision 1)
>     *  Provides user with ability to choose trust settings for the cert.
>     *  Asks the user to grant permission to import the certificate.
>     *
>     *  @param ctx A user interface context.
>     *  @param cert The certificate that is about to get installed.
>     *  @param trust a bit mask of trust flags, 

If you feel like it, this might be a good opportunity to remove all of the trailing whitespace in this file, but not a big deal either way.

::: security/manager/ssl/nsICryptoHMAC.idl:49
(Diff revision 1)
>       * WARNING: This approach is not FIPS compliant.
>       *
>       * @throws NS_ERROR_INVALID_ARG if an unsupported algorithm
>       *        type is passed.
>       *
>       * NOTE: This method must be called before any other method 

Same with this file and some others.

::: security/manager/ssl/nsISiteSecurityService.idl:34
(Diff revision 1)
>  [scriptable, uuid(31313372-842c-4110-bdf1-6aea17c845ad), builtinclass]
>  interface nsISiteSecurityState : nsISupports
>  {
> +  [must_use]
>    readonly attribute ACString hostname;
>    [infallible] readonly attribute long long expireTime;

Hmmm - for some attributes it might make sense to use infallible instead, but maybe we can address that later. Just a thought.
Attachment #8870899 - Flags: review?(dkeeler) → review+
Comment on attachment 8870899 [details]
Bug 1366584 - Add initial [must_use] properties to PSM IDL files.

https://reviewboard.mozilla.org/r/142472/#review146094

Thanks for the review!

> If you feel like it, this might be a good opportunity to remove all of the trailing whitespace in this file, but not a big deal either way.

Done for this and the other files.

> Hmmm - for some attributes it might make sense to use infallible instead, but maybe we can address that later. Just a thought.

Yeah, I'll file a follow-up for this.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3cd4770074d3f4d341642c527c2a82aa315037c0
(OSX 10.10 xpcshell tests are still queued, but it doesn't really matter - the changes here really only affect whether things build or not.)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/a184b592cfa3
Add initial [must_use] properties to PSM IDL files. r=keeler
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a184b592cfa3
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
See Also: → 1368104
You need to log in before you can comment on or make changes to this bug.