----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118943/#review60996 -----------------------------------------------------------
src/lib/marble/geodata/data/GeoDataAnimatedUpdate.cpp <https://git.reviewboard.kde.org/r/118943/#comment42481> Must check for null pointers here. Alternatively, go back to using a non-pointer version of m_update. src/lib/marble/geodata/data/GeoDataAnimatedUpdate.cpp <https://git.reviewboard.kde.org/r/118943/#comment42482> Let's delete the old update here (unless you switch back to using a non-pointer version). Also m_update should have its parent set in either case: delete d->m_update; d->m_update = update; if ( d->m_update ) { d->m_update->setParent( this ); } src/lib/marble/geodata/data/GeoDataUpdate.cpp <https://git.reviewboard.kde.org/r/118943/#comment42483> This is not enough. You must also cover the case where this has m_change set and the other a null pointer, and vice versa. Or use a non-pointer version. src/lib/marble/geodata/data/GeoDataUpdate.cpp <https://git.reviewboard.kde.org/r/118943/#comment42484> Must set parent and, in case of a pointer version, delete the old m_change (see above). src/lib/marble/geodata/writers/kml/KmlAnimatedUpdateTagWriter.cpp <https://git.reviewboard.kde.org/r/118943/#comment42485> For safety better Q_ASSERT on the nodeType() before using a static_cast src/lib/marble/geodata/writers/kml/KmlUpdateTagWriter.cpp <https://git.reviewboard.kde.org/r/118943/#comment42486> I'd also check that change()->size() > 0 to avoid ending up with an empty <Update/> tag tests/data/Tour.kml <https://git.reviewboard.kde.org/r/118943/#comment42487> Isn't that the default value which shouldn't be written? - Dennis Nienhüser On June 26, 2014, 6:36 a.m., Sanjiban Bairagya wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/118943/ > ----------------------------------------------------------- > > (Updated June 26, 2014, 6:36 a.m.) > > > Review request for Marble, Dennis Nienhüser and Torsten Rahn. > > > Repository: marble > > > Description > ------- > > This patch allows to accept Placemarks as children of GeoDataChange > > > Diffs > ----- > > src/lib/marble/geodata/CMakeLists.txt bac40ac > src/lib/marble/geodata/data/GeoDataAnimatedUpdate.h 5794656 > src/lib/marble/geodata/data/GeoDataAnimatedUpdate.cpp 511073c > src/lib/marble/geodata/data/GeoDataChange.h PRE-CREATION > src/lib/marble/geodata/data/GeoDataChange.cpp PRE-CREATION > src/lib/marble/geodata/data/GeoDataUpdate.h ddadee2 > src/lib/marble/geodata/data/GeoDataUpdate.cpp ce42922 > src/lib/marble/geodata/handlers/kml/KmlAltitudeModeTagHandler.cpp 9827969 > src/lib/marble/geodata/handlers/kml/KmlChangeTagHandler.h PRE-CREATION > src/lib/marble/geodata/handlers/kml/KmlChangeTagHandler.cpp PRE-CREATION > src/lib/marble/geodata/handlers/kml/KmlGxAltitudeModeTagHandler.cpp 3a6bf5a > src/lib/marble/geodata/handlers/kml/KmlModelTagHandler.cpp fe2f126 > src/lib/marble/geodata/handlers/kml/KmlPlacemarkTagHandler.cpp 110bc40 > src/lib/marble/geodata/handlers/kml/KmlUpdateTagHandler.cpp 534749c > src/lib/marble/geodata/parser/GeoDataTypes.h dafcea3 > src/lib/marble/geodata/parser/GeoDataTypes.cpp e87b876 > src/lib/marble/geodata/writers/kml/KmlAnimatedUpdateTagWriter.h > PRE-CREATION > src/lib/marble/geodata/writers/kml/KmlAnimatedUpdateTagWriter.cpp > PRE-CREATION > src/lib/marble/geodata/writers/kml/KmlLookAtTagWriter.cpp 4e45daa > src/lib/marble/geodata/writers/kml/KmlPlaylistTagWriter.cpp 86511bb > src/lib/marble/geodata/writers/kml/KmlPointTagWriter.cpp 70eadbc > src/lib/marble/geodata/writers/kml/KmlUpdateTagWriter.cpp fc1ddf6 > tests/TestTour.cpp db2d9c9 > tests/data/Tour.kml d9fc9ed > > Diff: https://git.reviewboard.kde.org/r/118943/diff/ > > > Testing > ------- > > Tested and passed with tests/Tour.kml > > > Thanks, > > Sanjiban Bairagya > >
_______________________________________________ Marble-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/marble-devel
