Closed Bug 1308868 Opened 3 years ago Closed Last year

[Static Analysis][Clang-Plugin] Ignore class members initialisation check - wave 2

Categories

(Firefox Build System :: Source Code Analysis, defect)

defect
Not set

Tracking

(firefox52 wontfix)

RESOLVED FIXED
Tracking Status
firefox52 --- wontfix

People

(Reporter: andi, Assigned: andi)

References

(Blocks 1 open bug)

Details

Attachments

(7 files, 1 obsolete file)

58 bytes, text/x-review-board-request
jrmuizel
: review+
Details
686 bytes, patch
Details | Diff | Splinter Review
900 bytes, patch
baku
: review+
Details | Diff | Splinter Review
1.89 KB, patch
edgar
: review+
Details | Diff | Splinter Review
663 bytes, patch
baku
: review+
Details | Diff | Splinter Review
744 bytes, patch
adw
: review+
Details | Diff | Splinter Review
1021 bytes, patch
rillian
: review+
Details | Diff | Splinter Review
This is wave 2 - for Bug 1282408

As for some member variables we want to ignore it's initialisation check performed by clang-plugin - https://bugzilla.mozilla.org/show_bug.cgi?id=525063
We've added a flag for the declaration of each variable that we want to be ignored.
On this bug there will be posted patches of this kind.
Keywords: sec-audit
Comment on attachment 8799765 [details]
Bug 1308868 - Add ignore-initialization-check annotation to mPendingTextTrackChange from TextTrackList.

https://reviewboard.mozilla.org/r/84888/#review83506

::: dom/media/TextTrackList.h:70
(Diff revision 1)
>  
>    IMPL_EVENT_HANDLER(change)
>    IMPL_EVENT_HANDLER(addtrack)
>    IMPL_EVENT_HANDLER(removetrack)
>  
> -  bool mPendingTextTrackChange;
> +  MOZ_INIT_OUTSIDE_CTOR bool mPendingTextTrackChange;

Isn't the warning telling us that on some constructor paths mPendingTrackChange isn't initialized? How about we just go:

bool mPendingTextTrackChange = false;

here, so it's alway initialized?
Attachment #8799765 - Flags: review?(cpearce) → review-
Attachment #8799765 - Attachment is obsolete: true
MozReview-Commit-ID: FNTRHFpTx2E
Attachment #8801087 - Flags: review?(jorendorff)
MozReview-Commit-ID: 2dLsmPWeqSk
Attachment #8801095 - Flags: review?(amarchesini)
Attachment #8801095 - Flags: review?(amarchesini) → review+
Pushed by bpostelnicu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b08765d5eb6
Add ignore-initialization-check annotation to mWorkerPrivate from ClientNavigateRunnable. r=baku
Attachment #8801095 - Flags: checkin+
Attachment #8801099 - Flags: review?(echen) → review+
Pushed by bpostelnicu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba0be6c3f68a
Add ignore-initialization-check annotation to variables from TelephonyCall and TelephonyCallGroup. r=echen
Attachment #8801099 - Flags: checkin+
MozReview-Commit-ID: 1dIKcbbeStW
Attachment #8801702 - Flags: review?(amarchesini)
Attachment #8801702 - Flags: review?(amarchesini) → review+
Pushed by bpostelnicu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f65cc8c9aace
Add ignore-initialization-check annotation to mErrorCode from U2FStatus. r=baku
Attachment #8801702 - Flags: checkin+
Duplicate of this bug: 1310687
MozReview-Commit-ID: ZDCY7i19x5
Attachment #8802091 - Flags: review?(adw)
Comment on attachment 8802097 [details] [diff] [review]
Add ignore-initialization-check annotation to mPosition and mLine from TextTrackCue

Review of attachment 8802097 [details] [diff] [review]:
-----------------------------------------------------------------

forward review to Ralph.
Attachment #8802097 - Flags: review?(jwwang) → review?(giles)
Attachment #8802091 - Flags: review?(adw) → review+
Comment on attachment 8802097 [details] [diff] [review]
Add ignore-initialization-check annotation to mPosition and mLine from TextTrackCue

Review of attachment 8802097 [details] [diff] [review]:
-----------------------------------------------------------------

This is fine; the accesses are protected by the `IsAutoKeyword` tags. It would be better to convert these to a Variant type so the compiler checks for us, though.
Attachment #8802097 - Flags: review?(giles) → review+
Pushed by bpostelnicu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/40a7a506f5bb
Add ignore-initialization-check annotation to time from RecentURIKey. r=adw
Attachment #8802091 - Flags: checkin+
Pushed by bpostelnicu@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/37e81a1c152c
Add ignore-initialization-check annotation to mPosition and mLine from TextTrackCue. r=giles
Attachment #8802097 - Flags: checkin+
Comment on attachment 8799342 [details]
Bug 1308868 - Add ignore-initialization-check annotation to members from TriangleTyped.

https://reviewboard.mozilla.org/r/84544/#review87488
Attachment #8799342 - Flags: review?(jmuizelaar) → review+
Too late for firefox 52, mass-wontfix.
Attachment #8801087 - Flags: review?(jorendorff)
Product: Core → Firefox Build System
The leave-open keyword is there and there is no activity for 6 months.
:andi, maybe it's time to close this bug?
Flags: needinfo?(bpostelnicu)
Most of the work for this bug has already landed in m-c, an since now we will have coverity during review phase this is no longer necessary, so closing it.
Status: NEW → RESOLVED
Closed: Last year
Flags: needinfo?(bpostelnicu)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.