Check result of Read32 in nsSupportsArray::Read

RESOLVED FIXED in Firefox 43

Status

()

Core
XPCOM
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

(Blocks: 1 bug)

Trunk
mozilla43
Points:
---

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Comment 1

3 years ago
Created attachment 8646653 [details] [diff] [review]
Check result of Read32 in nsSupportsArray::Read.

This is a simple patch, but it would be good if you double checked that it is okay to bail out in the middle of the method.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fe40a45f6e7
Attachment #8646653 - Flags: review?(nfroyd)
Comment on attachment 8646653 [details] [diff] [review]
Check result of Read32 in nsSupportsArray::Read.

Review of attachment 8646653 [details] [diff] [review]:
-----------------------------------------------------------------

Eric wanted to review more code, so Eric can decide whether bailing out in the middle here is OK. :)
Attachment #8646653 - Flags: review?(nfroyd) → review?(erahm)
Comment on attachment 8646653 [details] [diff] [review]
Check result of Read32 in nsSupportsArray::Read.

Review of attachment 8646653 [details] [diff] [review]:
-----------------------------------------------------------------

r=me. We return early elsewhere in the function [1], so this doesn't change the overall behavior of bailing on failure.

[1] https://dxr.mozilla.org/mozilla-central/rev/d4f3a8a75577e4af2914a4e899ca2e724f9715c4/xpcom/ds/nsSupportsArray.cpp#213-216,223-228

::: xpcom/ds/nsSupportsArray.cpp
@@ +188,5 @@
>    nsresult rv;
>  
>    uint32_t newArraySize;
>    rv = aStream->Read32(&newArraySize);
> +  if (NS_FAILED(rv)) return rv;

nit: Lets use the predominant style here, multiline and braced.
Attachment #8646653 - Flags: review?(erahm) → review+
(Assignee)

Comment 5

3 years ago
(In reply to Eric Rahm [:erahm] from comment #3)
> nit: Lets use the predominant style here, multiline and braced.

Fixed.
https://hg.mozilla.org/mozilla-central/rev/0051268bfb18
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox43: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.