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.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to     : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp

Reply via email to