NS_InputStreamIsBuffered should initialize count or TestInputStream should set the count to zero

NEW
Unassigned

Status

()

9 years ago
9 years ago

People

(Reporter: johnjbarton, Unassigned)

Tracking

1.9.2 Branch
x86
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

9 years ago
I have built FF 3.6 with DEBUG and I am trying to get Chromebug+Firebug to run on it without asserts.

I assert here:
---
     nsresult rv = tee->mWriter(in, tee->mClosure, fromSegment, offset, count, writeCount);
    if (NS_FAILED(rv) || (*writeCount == 0)) {
        NS_ASSERTION((NS_FAILED(rv) ? (*writeCount == 0) : PR_TRUE),
                "writer returned an error with non-zero writeCount");
        return rv;
    }
---
in nsInputStreamTee.cpp. The reason is that mWriter is
---
static NS_METHOD
TestInputStream(nsIInputStream *inStr,
                void *closure,
                const char *buffer,
                PRUint32 offset,
                PRUint32 count,
                PRUint32 *countWritten)
{
    PRBool *result = static_cast<PRBool *>(closure);
    *result = PR_TRUE;
    return NS_ERROR_ABORT;  // don't call me anymore
}
---
The rv will be NS_ERROR_ABORT, but the writeCount will be what ever garbage value happened to be in that location? 

There seems to be two ways to fix this.

1) set writeCount to zero in TestInputStream
http://mxr.mozilla.org/mozilla1.9.2/source/xpcom/io/nsStreamUtils.cpp#695
or
2) initialize 'n' to zero in
http://mxr.mozilla.org/mozilla1.9.2/source/xpcom/io/nsStreamUtils.cpp#702

I think both should be done.
Or the assert should be fixed to not assert bogus things about out params of functions that return error?
Well, the point of the assert is to catch callees who want to consume some data of the stream but then return an error. That doesn't seem entirely unreasonable, and also, this is not an xpcom function - just a C++ callback that happens to be defined in IDL.

I think 2) should be done, 1) doesn't seem necessary to me.
(Reporter)

Comment 3

9 years ago
Created attachment 426907 [details] [diff] [review]
(trivial) patch for 1.9.2 

I've been running my local copy of FF 3.6 with this patch. WFM ;-)
Attachment #426907 - Attachment is patch: true
Attachment #426907 - Attachment mime type: application/octet-stream → text/plain
I'm sorry, I didn't read comment 0 carefully enough when I said 2) should be done. What I meant was that nsInputStreamTee::WriteSegmentFun should initialize *writeCount before calling mWriter (not nsStreamUtils)
(Reporter)

Comment 5

9 years ago
Created attachment 427377 [details] [diff] [review]
(trivial) patch for 1.9.2

per comment 4, a refreshed patch
Attachment #426907 - Attachment is obsolete: true
(Reporter)

Comment 6

9 years ago
I guess I need to do something to cause this to move forward, but I don't know what.
You need to log in before you can comment on or make changes to this bug.