Replace PR_STATIC_ASSERT with static_assert in WaveReader

RESOLVED FIXED in mozilla26

Status

()

Core
Audio/Video
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: rillian, Assigned: rillian)

Tracking

Trunk
mozilla26
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Now that we can use C++11 static_assert, replace PR_STATIC_ASSERT macros in content/media/wave/
(Assignee)

Comment 1

4 years ago
Created attachment 787651 [details] [diff] [review]
Part 1: replace PR_STATIC_ASSERT with static_assert in WaveReader
Assignee: nobody → giles
Attachment #787651 - Flags: review?(kinetik)
Attachment #787651 - Flags: feedback?(ehsan)
(Assignee)

Comment 2

4 years ago
Created attachment 787652 [details] [diff] [review]
Part 2: fix assertion nits
Attachment #787652 - Flags: review?(kinetik)
(Assignee)

Comment 3

4 years ago
https://tbpl.mozilla.org/?tree=Try&rev=7c8355c5865b

Comment 4

4 years ago
Comment on attachment 787651 [details] [diff] [review]
Part 1: replace PR_STATIC_ASSERT with static_assert in WaveReader

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

Nice!
Attachment #787651 - Flags: feedback?(ehsan) → feedback+
Attachment #787651 - Flags: review?(kinetik) → review+
Comment on attachment 787652 [details] [diff] [review]
Part 2: fix assertion nits

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

::: content/media/wave/WaveReader.cpp
@@ +337,4 @@
>      return false;
>    }
>  
> +  static_assert(sizeof(uint32_t) * 2 + 4 <= RIFF_INITIAL_SIZE,

I think it'd be slightly clearer if we make it |sizeof(uint32_t) * 3| and change |p += 4| to |p += sizeof(uint32_t)|.
Attachment #787652 - Flags: review?(kinetik) → review+
(Assignee)

Comment 6

4 years ago
Created attachment 788746 [details] [diff] [review]
Part 2: fix assertion nits v2

Thanks. Updated patch to incorporate review suggestions. Carrying forward r=kinetik.
Attachment #787652 - Attachment is obsolete: true
Attachment #788746 - Flags: review+
(Assignee)

Comment 7

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea96533eaff9
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce87bb4db33f
https://hg.mozilla.org/mozilla-central/rev/ea96533eaff9
https://hg.mozilla.org/mozilla-central/rev/ce87bb4db33f
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.