Re: Review Request: Change dynamic_cast in KoPAOdfSaveHelper constructor
Robert Mathias Marmorstein <robert <at> narnia.homeunix.com>
2012-03-13 19:00:20 GMT
My one concern about this is that I don't understand why the unit test was passing before (if it ever was). "git blame" shows that this logic has been this way since the code was first added several years ago.
- Robert Mathias
On March 13th, 2012, 6:59 p.m., Robert Mathias Marmorstein wrote:
Review request for KOffice.
By Robert Mathias Marmorstein.
Updated March 13, 2012, 6:59 p.m.
The TestPACopyPastePage unit test was failing with a segmentation fault (signal 11). Debugging it in gdb showed that the cause was a null pointer for the "page" variable in a loop through the master pages. Tracing this back, I discovered some odd code that seemed to be dynamic_casting a Page* to a Page*. I'm not sure that behaviour is even defined in the C++ standard, but I'm pretty sure what we really want to do is cast to a MasterPage instead (and reverse the logic of the if statement). So that's what this patch does.
Code still compiles and runs and master pages work. The TestPACopyPastePage unit test now passes instead of failing and all other unit tests still pass (except the two that were failing already -- both related to layout problems).
- libs/kopageapp/KoPAOdfPageSaveHelper.cpp (1a8f965)
koffice-devel mailing list
koffice-devel <at> kde.org