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

Status: RESOLVED FIXED

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

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1415397 - use Span<> to replace low level pointer arithmetic in ReadFromCache().

Review commit: https://reviewboard.mozilla.org/r/197460/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/197460/

Comment 2

[Code Review Bot [:reviewbot]]

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

[Benjamin Chen [:bechen]]

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

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

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!

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

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

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

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/197460/diff/1-2/

Comment 7

[JW Wang [:jwwang] [:jw_wang]]

Thanks for the reviews!

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3e95c596ad5b
use Span<> to replace low level pointer arithmetic in ReadFromCache(). r=bechen,gerald

Comment 9

[Code Review Bot [:reviewbot]]

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

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/3e95c596ad5b

Comment 11

[Pulsebot]

Backout by [e-mail]:
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

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out because it depended on backed out bug 1412737:

https://hg.mozilla.org/mozilla-central/rev/75c04adb414bc00d39a0af80691a82af1ceebbd0

Comment 13

[JW Wang [:jwwang] [:jw_wang]]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/197460/diff/2-3/

Comment 14

[JW Wang [:jwwang] [:jw_wang]]

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.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/05002ad2b8f9
use Span<> to replace low level pointer arithmetic in ReadFromCache(). r=bechen,gerald

Comment 16

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/05002ad2b8f9