Closed Bug 1415397 Opened 7 years ago Closed 7 years ago

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

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla58

Tracking Flags:

Tracking

Status

firefox58

---

fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Bug 1415397 - use Span<> to replace low level pointer arithmetic in ReadFromCache(). 7 years ago JW Wang [:jwwang] [:jw_wang] 59 bytes, text/x-review-board-request	bechen : review+ mozbugz : review+	Details

JW Wang [:jwwang] [:jw_wang]

Assignee

Description

•

7 years ago

      No description provided.

JW Wang [:jwwang] [:jw_wang]

Assignee

Updated

•

7 years ago

Assignee: nobody → jwwang

Blocks: 1355409

Priority: -- → P3

Comment hidden (mozreview-request)

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

JW Wang [:jwwang] [:jw_wang]

Assignee

Updated

•

7 years ago

Attachment #8926209 - Flags: review?(bechen)

Code Review Bot [:reviewbot]

Comment 2

•

7 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`

Benjamin Chen [:bechen]

Comment 3

•

7 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+

JW Wang [:jwwang] [:jw_wang]

Assignee

Comment 4

•

7 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!

JW Wang [:jwwang] [:jw_wang]

Assignee

Updated

•

7 years ago

Attachment #8926209 - Flags: review?(gsquelart)

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

Comment 5

•

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

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/

JW Wang [:jwwang] [:jw_wang]

Assignee

Comment 7

•

7 years ago

Thanks for the reviews!

Pulsebot

Comment 8

•

7 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

Code Review Bot [:reviewbot]

Comment 9

•

7 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`

Narcis Beleuzu [:NarcisB]

Comment 10

•

7 years ago

bugherder

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

Status: NEW → RESOLVED

Closed: 7 years ago

status-firefox58: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla58

Pulsebot

Comment 11

•

7 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

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

Comment 12

•

7 years ago

Backed out because it depended on backed out bug 1412737:

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

Status: RESOLVED → REOPENED

status-firefox58: fixed → ---

Flags: needinfo?(jwwang)

Resolution: FIXED → ---

Target Milestone: mozilla58 → ---

Comment hidden (mozreview-request)

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/

JW Wang [:jwwang] [:jw_wang]

Assignee

Comment 14

•

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

Pulsebot

Comment 15

•

7 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

Dorel Luca [:dluca]

Comment 16

•

7 years ago

bugherder

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

Status: REOPENED → RESOLVED

Closed: 7 years ago → 7 years ago

status-firefox58: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla58

You need to log in before you can comment on or make changes to this bug.