Avoid dereferencing find() without checking for == end()

Avoid dereferencing find() without checking for == end()

If (due to an unexpected failure) one of the find() calls in
audio_video_metadata_extractor_unittest.cc fails (returns end()) then
dereferencing it triggers undefined behavior. This makes the results of
the test undefined. In most cases the test failure will still be caught
but there are no guarantees.

This change adds a helper function to simplify using find() in a way
that is safe even in the face of unexpected failures.

This was discovered while investigating crbug.com/581768.

R=dalecurtis@chromium.org

Committed: https://crrev.com/a7a346e3e460cd7273a1346f01e8c260b518a5c6
Cr-Commit-Position: refs/heads/master@{#372219}

[brucedawson, 2016-01-28 21:53:52]

Can you take a look at this fix? If the pattern looks good then I'll apply it to
all of the find() calls and submit for official review by you.

The illegal dereferences were hit when the compiler bug caused unexpected (but
not undefined) data to be put in the tag dictionary.

I could also pass in 'extractor' and the array index instead of
extractor->stream_infos()[0].tags, but I'm not convinced that is an improvement
in the context of tests.

[DaleCurtis, 2016-01-28 21:54:47]

lgtm % nit

https://codereview.chromium.org/1643633005/diff/1/media/base/audio_video_meta...
File media/base/audio_video_metadata_extractor_unittest.cc (right):

https://codereview.chromium.org/1643633005/diff/1/media/base/audio_video_meta...
media/base/audio_video_metadata_extractor_unittest.cc:68: EXPECT_NE(tag_data,
tags.end());
I'd just drop this since it's just noise in the test log if it fails here (vs on
the actual test line below) and use a ternary for style.

[brucedawson, 2016-01-28 22:17:26]

Finished. The tests pass/fail appropriately, without dereferencing end()

https://codereview.chromium.org/1643633005/diff/1/media/base/audio_video_meta...
File media/base/audio_video_metadata_extractor_unittest.cc (right):

https://codereview.chromium.org/1643633005/diff/1/media/base/audio_video_meta...
media/base/audio_video_metadata_extractor_unittest.cc:68: EXPECT_NE(tag_data,
tags.end());
On 2016/01/28 21:54:47, DaleCurtis wrote:
> I'd just drop this since it's just noise in the test log if it fails here (vs
on
> the actual test line below) and use a ternary for style.

Sounds good.

The original advice was to use ASSERT, but that does a return; and therefore
cannot be used from a helper function such as this.

[brucedawson, 2016-01-29 00:07:48]

The CQ bit was checked by brucedawson@chromium.org

[brucedawson, 2016-01-29 00:07:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/1643633005/#ps20001
(title: "Completed fix, and CR nit fixes")

[commit-bot: I haz the power, 2016-01-29 00:09:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1643633005/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1643633005/20001

[commit-bot: I haz the power, 2016-01-29 00:29:40]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-29 00:30:26]

Description was changed from

==========
Avoid dereferencing find() without checking for == end()

If (due to an unexpected failure) one of the find() calls in
audio_video_metadata_extractor_unittest.cc fails (returns end()) then
dereferencing it triggers undefined behavior. This makes the results of
the test undefined. In most cases the test failure will still be caught
but there are no guarantees.

This change adds a helper function to simplify using find() in a way
that is safe even in the face of unexpected failures.

This was discovered while investigating crbug.com/581768.

R=dalecurtis@chromium.org
==========

to

==========
Avoid dereferencing find() without checking for == end()

If (due to an unexpected failure) one of the find() calls in
audio_video_metadata_extractor_unittest.cc fails (returns end()) then
dereferencing it triggers undefined behavior. This makes the results of
the test undefined. In most cases the test failure will still be caught
but there are no guarantees.

This change adds a helper function to simplify using find() in a way
that is safe even in the face of unexpected failures.

This was discovered while investigating crbug.com/581768.

R=dalecurtis@chromium.org

Committed: https://crrev.com/a7a346e3e460cd7273a1346f01e8c260b518a5c6
Cr-Commit-Position: refs/heads/master@{#372219}
==========

[commit-bot: I haz the power, 2016-01-29 00:30:26]

Patchset 2 (id:??) landed as
https://crrev.com/a7a346e3e460cd7273a1346f01e8c260b518a5c6
Cr-Commit-Position: refs/heads/master@{#372219}