The proposal to merge lp:~zorba-coders/zorba/bug1123835 into lp:zorba has been
updated.
Status: Needs review = Rejected
For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/bug1123835/+merge/158978
--
https://code.launchpad.net/~zorba-coders/zorba/bug1123835/+merge/158978
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
3 of the failing test-cases are correct (fn-unparsed-text-lines-039,
fn-unparsed-text-039, fn-unparsed-text-available-038).
Here is why:
The 3 test-cases use fn/unparsed-text/non-xml-character.txt that contains BOM
followed by NULL character.
Here is the catch: NULL is not a valid XML 1.0 nor
Review: Needs Fixing
Unfortunately, I don't think the proposed change is safe. istream::unget() is
not guaranteed to work, and in particular it probably won't work if the stream
is coming via HTTP.
However, there is a function StreamResource::isStreamSeekable(). Perhaps you
could check that
Ok, maybe ignore point #1 - that function is there after all. I'm not sure how
I missed it; must have been a typo in my search. Anyway, I'll review the code
more thoroughly a bit later tonight. Points #2 and #3 still need to be fixed.
Points #2 and #3 should be fixed.
The proposed changes, so
Juan Zacarias has proposed merging lp:~zorba-coders/zorba/bug1123835 into
lp:zorba.
Commit message:
Fixes for FOTS errors in fn:unparsed-text* functions
Requested reviews:
Chris Hillery (ceejatec)
Related bugs:
Bug #1123835 in Zorba: fn-unparsed-text* failures (at least 20 failures)
This branch doesn't solve all the errors yet this is the list of errors missing
solution with a brief description of the current problem.
The missing errors are caused by 3 problems
* utf-8 encoding: missing the stream suggested to Paul that can handle errors
when invalid utf-8 are found.
*
Review: Needs Fixing
1. It looks like you didn't check in some changes? sequences_impl.cpp refers to
a method URI::get_encoded_fragment() that doesn't exist.
2. sequences_impl.cpp now shows up on my system as ISO-8859 English text
rather than ASCII English text when I use the file command. My
Ok, maybe ignore point #1 - that function is there after all. I'm not sure how
I missed it; must have been a typo in my search. Anyway, I'll review the code
more thoroughly a bit later tonight. Points #2 and #3 still need to be fixed.
--
9 matches
Mail list logo
