530901 - nsDOMFileReader::OnStopRequest can return |rv| without ever initializing it

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Daniel Holbert [:dholbert]]

This warning on bsmedberg's warning tracker:
>  content/base/src/nsDOMFileReader.cpp:457: 'rv' may be used uninitialized in this function
[  http://office.smedbergs.us:8080/warning?id=30221  ]
points out that "rv" in nsDOMFileReader::OnStopRequest can be returned without ever having been initialized.

Code snippet:
{
 457   nsresult rv;
 458   switch (mDataFormat) {
 459     case FILE_AS_BINARY:
 460       break; //Already accumulated mResult
 461     case FILE_AS_TEXT:
 462       rv = GetAsText(mCharset, mFileData, mDataLen, mResult);
 463       break;
[SNIP]
 477   return rv;
}

In the FILE_AS_BINARY case, we leave rv unset -- and we later return it.  That's bad.

This affects both mozilla-central and mozilla-1.9.2.
http://hg.mozilla.org/mozilla-central/annotate/d10f2f103f43/content/base/src/nsDOMFileReader.cpp#l457
http://hg.mozilla.org/releases/mozilla-1.9.2/annotate/e7dfac59fa7c/content/base/src/nsDOMFileReader.cpp#l457

Comment 1

[Daniel Holbert [:dholbert]]

I have no idea what this code does, but based on the "//Already accumulated mResult" comment, it sounds like we should just be setting rv = NS_OK there.

Jonas, is that correct?

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to fix

This is technically a better fix. If something goes wrong when converting the read data we now dispatch an error event.

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 414397 [details] [diff] [review]
Patch to fix

This fixes a minor bug and aligns us with the spec, I think we should land this for 1.9.2 while we still can.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Fixed on trunk and 1.9.2

http://hg.mozilla.org/mozilla-central/rev/3ca32b7c0d21
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/af0e2b9566cc