zorba-coders team mailing list archive

[Chris Hillery <chillery+launchpad@xxxxxxxxx>]

Review: Needs Fixing

Sorry, but this implementation isn't right. You're still sucking the entire contents of the istream into memory via that stringstream. That's not acceptable. (You also have a potential memory leak since you don't use an auto_ptr<> for the stringstream you allocate on the stack, but that will be irrelevant since there shouldn't be a stringstream at all.)

I believe this implementation will also throw an error if there aren't at least 3 bytes in the input stream.

Please re-read my comment from 2013-04-24. The best solution would be to implement a buffer::attach() iostreams class with a 3-byte buffer. This would allow you to safely read and, if necessary, put back 3 bytes to check for a BOM. You could then attach THAT to either unparsed::attach() or transcode::attach() as you do here. I kind of feel like there must be an open-source class that does that already; it's completely generic.

Failing that, the closest thing to a right answer would be to revert to the code you had before using istream::unget(), with an additional check that the stream is seekable before attempting it. It's half a solution, but it's better than no solution.
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug1123835/+merge/158978
Your team Zorba Coders is subscribed to branch lp:zorba.