----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103674/#review9760 -----------------------------------------------------------
Ship it! Looks good, Please implement the FIXME you added to the code // FIXME: appendChildNodes() reparents nodes, so needs to clone After that you can commit. - Thorsten Zachmann On Jan. 11, 2012, 2:21 p.m., Dag Andersen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/103674/ > ----------------------------------------------------------- > > (Updated Jan. 11, 2012, 2:21 p.m.) > > > Review request for Calligra. > > > Description > ------- > > Symptom: > Plan (sometimes) crashes and valgrind reports "Invalid read size". > > When KoXml* is converted to QDom* a temporary QDomDocument is used to create > nodes. > This results in deleted nodes when the document goes out of scope. > From QDomDocument api docs: > "Since elements, text nodes, comments, processing instructions, etc., cannot > exist outside the context of a document, the document class also contains the > factory functions needed to create these objects. The node objects created > have an ownerDocument() function which associates them with the document > within whose context they were created." > > In short the patch passes ownerDoc as a reference and appends the converted > nodes to the ownerDoc. > Also asQDomNode()/asQDomElement() does not return anything, the node must be > found in ownerDoc. > > Modifications to the users is not included. > > Note that the patch is not quite finished because I don't understand the > TestXmlReaderWithoutSpaces unit test. > > > Diffs > ----- > > libs/odf/KoXmlReader.h 231651c > libs/odf/KoXmlReader.cpp 13a6d47 > libs/odf/tests/TestXmlReader.cpp 6631b64 > libs/odf/tests/TestXmlReaderWithoutSpaces.cpp 5179846 > > Diff: http://git.reviewboard.kde.org/r/103674/diff/diff > > > Testing > ------- > > Unit test passes (but it did before too). > No crash in Plan. > > > Thanks, > > Dag Andersen > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
