Closed Bug 1415397 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: Audio/Video: Playback, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

No description provided.
Assignee: nobody → jwwang
Blocks: 1355409
Priority: -- → P3
Attachment #8926209 - Flags: review?(bechen)
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 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+
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!
Attachment #8926209 - Flags: review?(gsquelart)
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+
Thanks for the reviews!
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 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`
https://hg.mozilla.org/mozilla-central/rev/3e95c596ad5b
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
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 → ---
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)
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
https://hg.mozilla.org/mozilla-central/rev/05002ad2b8f9
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.