1193564 - Check result of Read32 in nsSupportsArray::Read

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: 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

Comment 2

[Nathan Froyd [:froydnj]]

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

Comment 3

[Eric Rahm [: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.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0051268bfb18

Comment 5

[Andrew McCreight [:mccr8]]

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

Fixed.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0051268bfb18