903012 - Replace PR_STATIC_ASSERT with static_assert in WaveReader

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Ralph Giles (:rillian)]

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

Comment 1

[Ralph Giles (:rillian)]

Attachment: Part 1: replace PR_STATIC_ASSERT with static_assert in WaveReader

Comment 2

[Ralph Giles (:rillian)]

Attachment: Part 2: fix assertion nits

Comment 3

[Ralph Giles (:rillian)]

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

Comment 4

[(no longer active)]

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!

Comment 5

[Matthew Gregan [:kinetik]]

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)|.

Comment 6

[Ralph Giles (:rillian)]

Attachment: Part 2: fix assertion nits v2

Thanks. Updated patch to incorporate review suggestions. Carrying forward r=kinetik.

Comment 7

[Ralph Giles (:rillian)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ea96533eaff9
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce87bb4db33f

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ea96533eaff9
https://hg.mozilla.org/mozilla-central/rev/ce87bb4db33f