Review Request 111043: Fix possibly memory leak in sidewinder code by not called destructor on incomplete type of QTextDocument

Commit Hook null at kde.org
Sun Jun 30 22:53:59 BST 2013


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111043/
-----------------------------------------------------------

(Updated June 30, 2013, 9:53 p.m.)


Status
------

This change has been marked as submitted.


Review request for Calligra, Marijn Kruisselbrink and Sebastian Sauer.


Description
-------

TxORecord is deconstructed in places where QTextDocument is not a completely defined type (because that is only forward declared in the header with the declaration of TxORecord, excel.h. Delete on objects of incomplete type is just undefined, so not an error usually, thus the compiler will not stop on it, see e.g. http://en.wikibooks.org/wiki/More_C++_Idioms/Checked_delete for the background.
Which means all QTextDocument members of TxORecord are currently leaked.

The most simple fix would have been to just remove the forward declaration of <QTextDocument>  with a #include <QTextDocument> in excel.h, so that QTextDocument is completely defined.

But somehow I was tempted to instead move the members to a private class, like all/most of the other record classes have, that way also moving the deletion of the QTextDocument member to records.cpp, where the type is completely defined.

Whatever you prefer, your choice, just tell.

Okay to backport to 2.7?


Diffs
-----

  filters/sheets/excel/import/ODrawClient.cpp c558d73 
  filters/sheets/excel/sidewinder/excel.h 5b0076c 
  filters/sheets/excel/sidewinder/excel.cpp 50f54c0 
  filters/sheets/excel/sidewinder/worksheetsubstreamhandler.cpp bd66048 

Diff: http://git.reviewboard.kde.org/r/111043/diff/


Testing
-------


Thanks,

Friedrich W. H. Kossebau

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20130630/25aee9bf/attachment.htm>


More information about the calligra-devel mailing list