Closed Bug 319636 Opened 15 years ago Closed 15 years ago

Provide convenience function for consuming an entire nsIInputStream

Categories

(Core :: XPCOM, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: darin.moz, Assigned: darin.moz)

References

Details

(Keywords: fixed1.8.1)

Attachments

(2 files)

Provide convenience function for consuming an entire nsIInputStream
Attached patch v1 patchSplinter Review
Simple patch.  This is something the places project wants to use.
Attachment #205355 - Flags: superreview?(bzbarsky)
Attachment #205355 - Flags: review?(cbiesinger)
Blocks: 317839
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.9alpha
Comment on attachment 205355 [details] [diff] [review]
v1 patch

>Index: io/nsStreamUtils.h
>Index: io/nsStreamUtils.cpp
>+NS_ConsumeStream(nsIInputStream *stream, nsACString &result)
>+        char *buf = result.BeginWriting();

Shouldn't you add |length| here?  Otherwise you're always reading into the beginning of the string...

sr=bzbarsky with that fixed.
Attachment #205355 - Flags: superreview?(bzbarsky) → superreview+
Yes, indeed.  My testcase was apparently not comprehensive enough :-(
The stream in the testcase just handed back all the data in a single chunk is all... ;)
Comment on attachment 205355 [details] [diff] [review]
v1 patch

heh, the other day I was thinking that a function like this would be useful :)

should the comments say that the stream needs to be blocking for reasonable behaviour? (at least I think that's the case...)

shouldn't you truncate the string at the beginning of this function?
Attachment #205355 - Flags: review?(cbiesinger) → review+
I decided to let this function work with non-blocking streams as well.  The comments say that the function will append to the buffer and return a failure if it encountered an error at some point, but any data in the buffer will remain in the buffer.  This allows the function to be used with either type of stream.

I can make the function Truncate up front I suppose.  That makes the function a little less useful, but it perhaps makes its behavior a bit more predictable.  Hmm...
Attached patch v1.1 patchSplinter Review
Here's the version of the patch that I checked in.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
This patch is giving me the following error on WinXP/cygwin/Mingw

e:/mozilla/source/mozilla/xpcom/build/dlldeps.cpp: In function `void XXXNeverCal
led()':
e:/mozilla/source/mozilla/xpcom/build/dlldeps.cpp:122: error: cannot convert `ns
CString' to `PRUint32' for argument `2' to `nsresult NS_ConsumeStream(nsIInputSt
ream*, PRUint32, nsACString_internal&)'
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
> CString' to `PRUint32' for argument `2' to `nsresult
> NS_ConsumeStream(nsIInputSt

crap, the line in dlldeps.cpp needs to have a "0" in the second parameter to NS_ConsumeStream.  It looks like the CVS servers are down for the count, so I won't be able to fix this until they come back online.
(In reply to comment #9)
> crap, the line in dlldeps.cpp needs to have a "0" in the second parameter to
> NS_ConsumeStream.  It looks like the CVS servers are down for the count, so I
> won't be able to fix this until they come back online.

I tried to build Sunbird from trunk today (dlldeps.cpp,v1.137 with "0") and get the following error: 

c:/dev/moz_build/mozilla/xpcom/build/dlldeps.cpp: 
   In function `void XXXNeverCalled()':

c:/dev/moz_build/mozilla/xpcom/build/dlldeps.cpp:122: 
   error: invalid initialization of non-const reference of type 
   'nsACString_internal&' from a temporary of type 'nsCString'

c:/dev/moz_build/mozilla/xpcom/build/../io/nsStreamUtils.h:136: 
   error: in passing argument 3 of `nsresult NS_ConsumeStream(nsIInputStream*, 
   PRUint32, nsACString_internal&)'
I applied a fix for this.  Please update dlldeps.cpp
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
Blocks: 324274
marking dependency for branch landing
Depends on: 315438
Keywords: fixed1.8.1
You need to log in before you can comment on or make changes to this bug.