From koffice-devel Tue Mar 13 19:12:35 2012 From: "Thomas Zander" Date: Tue, 13 Mar 2012 19:12:35 +0000 To: koffice-devel Subject: Re: Review Request: Change dynamic_cast in KoPAOdfSaveHelper constructor Message-Id: <20120313191235.9045.62242 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=koffice-devel&m=133166602208435 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============1486802789060580279==" --===============1486802789060580279== Content-Type: multipart/alternative; boundary="===============1191055317064985232==" --===============1191055317064985232== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104263/#review11369 ----------------------------------------------------------- Ship it! The change looks correct. = The change made by me some time ago (2nd christmas day 2011) where there us= ed to be a KoPAPageBase seems to me a likely cause of this bug, apologies f= or not finding the unit test regression, and thanks for the fix! - Thomas Zander On March 13, 2012, 6:59 p.m., Robert Mathias Marmorstein wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/104263/ > ----------------------------------------------------------- > = > (Updated March 13, 2012, 6:59 p.m.) > = > = > Review request for KOffice. > = > = > Description > ------- > = > The TestPACopyPastePage unit test was failing with a segmentation fault (= signal 11). Debugging it in gdb showed that the cause was a null pointer f= or the "page" variable in a loop through the master pages. Tracing this ba= ck, 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 inst= ead (and reverse the logic of the if statement). So that's what this patch= does. = > = > = > Diffs > ----- > = > libs/kopageapp/KoPAOdfPageSaveHelper.cpp 1a8f965 = > = > Diff: http://git.reviewboard.kde.org/r/104263/diff/ > = > = > Testing > ------- > = > Code still compiles and runs and master pages work. The TestPACopyPasteP= age 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 pr= oblems). > = > = > Thanks, > = > Robert Mathias Marmorstein > = > --===============1191055317064985232== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable

This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/104263/

Ship it!

The change=
 looks correct. =


The change made by me some time ago (2nd christmas day 2011) where there us=
ed to be a KoPAPageBase seems to me a likely cause of this bug, apologies f=
or not finding the unit test regression, and thanks for the fix!

- Thomas

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.

Descripti= on

The TestPACopyPastePage unit test was failing with a segment=
ation fault (signal 11).  Debugging it in gdb showed that the cause was a n=
ull pointer for the "page" variable in a loop through the master =
pages.  Tracing this back, I discovered some odd code that seemed to be dyn=
amic_casting a Page* to a Page*.  I'm not sure that behaviour is even d=
efined 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 stateme=
nt).  So that's what this patch does.

Testing <= /h1>

Code still compiles and runs and master pages work.  The Tes=
tPACopyPastePage 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).

Diffs=

libs/kopageapp/KoPAOdfPageSaveHelper.cpp (= 1a8f965)

--===============1191055317064985232==-- --===============1486802789060580279== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ koffice-devel mailing list koffice-devel@kde.org https://mail.kde.org/mailman/listinfo/koffice-devel --===============1486802789060580279==--