Use Span<> to replace low level pointer arithmetic in MediaCacheStream::ReadFromCache()

RESOLVED FIXED in Firefox 58

Status

()

enhancement
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jwwang, Assigned: jwwang)

Tracking

(Blocks 1 bug)

unspecified
mozilla58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

2 years ago
No description provided.
Assignee

Updated

2 years ago
Assignee: nobody → jwwang
Blocks: 1355409
Priority: -- → P3
Assignee

Updated

2 years ago
Attachment #8926209 - Flags: review?(bechen)

Comment 2

2 years ago
mozreview-review
Comment on attachment 8926209 [details]
Bug 1415397 - use Span<> to replace low level pointer arithmetic in ReadFromCache().

https://reviewboard.mozilla.org/r/197460/#review202686


C/C++ static analysis found 0 defects in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`

Comment 3

2 years ago
mozreview-review
Comment on attachment 8926209 [details]
Bug 1415397 - use Span<> to replace low level pointer arithmetic in ReadFromCache().

https://reviewboard.mozilla.org/r/197460/#review202706

::: dom/media/MediaCache.cpp:2514
(Diff revision 1)
> +  }
> +
> +  if (aBuffer.Length() > size_t(BLOCK_SIZE)) {
> +    // Clamp the buffer to avoid overflow below since we will read at most
> +    // BLOCK_SIZE bytes.
> +    aBuffer = aBuffer.From(BLOCK_SIZE);

Per discuss, this should be aBuffer.First().
Attachment #8926209 - Flags: review?(bechen) → review+
Assignee

Comment 4

2 years ago
mozreview-review-reply
Comment on attachment 8926209 [details]
Bug 1415397 - use Span<> to replace low level pointer arithmetic in ReadFromCache().

https://reviewboard.mozilla.org/r/197460/#review202706

> Per discuss, this should be aBuffer.First().

Good catch!
Assignee

Updated

2 years ago
Attachment #8926209 - Flags: review?(gsquelart)

Comment 5

2 years ago
mozreview-review
Comment on attachment 8926209 [details]
Bug 1415397 - use Span<> to replace low level pointer arithmetic in ReadFromCache().

https://reviewboard.mozilla.org/r/197460/#review202782
Attachment #8926209 - Flags: review?(gsquelart) → review+
Comment hidden (mozreview-request)
Assignee

Comment 7

2 years ago
Thanks for the reviews!

Comment 8

2 years ago
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3e95c596ad5b
use Span<> to replace low level pointer arithmetic in ReadFromCache(). r=bechen,gerald

Comment 9

2 years ago
mozreview-review
Comment on attachment 8926209 [details]
Bug 1415397 - use Span<> to replace low level pointer arithmetic in ReadFromCache().

https://reviewboard.mozilla.org/r/197460/#review202794


C/C++ static analysis found 0 defects in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3e95c596ad5b
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58

Comment 11

2 years ago
Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/mozilla-central/rev/75c04adb414b
Backed out changeset 3e95c596ad5b because backed out (bug 1412737 depended on it. a=backout on a CLOSED TREE
Backed out because it depended on backed out bug 1412737:

https://hg.mozilla.org/mozilla-central/rev/75c04adb414bc00d39a0af80691a82af1ceebbd0
Status: RESOLVED → REOPENED
Flags: needinfo?(jwwang)
Resolution: FIXED → ---
Target Milestone: mozilla58 → ---
Comment hidden (mozreview-request)
Assignee

Comment 14

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d36f2a845f599cdf9e8b24dd7ecfff486dea8360

g4 looks green. I think this bug is innocent and can be re-landed. I will check bug 1412737 later.
Flags: needinfo?(jwwang)

Comment 15

2 years ago
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/05002ad2b8f9
use Span<> to replace low level pointer arithmetic in ReadFromCache(). r=bechen,gerald

Comment 16

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/05002ad2b8f9
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.