<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://reviewboard.kde.org/r/5149/">http://reviewboard.kde.org/r/5149/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for marble.</div>
<div>By Dennis Nienhüser.</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Currently it's pretty easy for plugins to be loaded into Marble, because only their inheritance of a *Plugin class is checked. Interface version bumps are not considered. This leads to crashes as soon as an ABI breaking change is committed in plugin related classes. There are a couple of examples (backtraces) in Bugzilla that show this behavior.
Three things are needed to fix this:
- The PluginManager must check that both *Plugin and *PluginInterface is inherited. The latter is the one where we have the version string check from Q_DECLARE_INTERFACE(..., ".../1.02") to prevent loading of incompatible plugins
- No library class must have a Q_DECLARE_INTERFACE statement. Otherwise loading an old plugin with a new libmarblewidget still succeeds because the new libmarblewidget declares that the correct interface is implemented
- Instead the plugin must Q_DECLARE_INTERFACE its type and version
Please review carefully that I found all superfluous Q_DECLARE_INTERFACE calls in src/lib and missing Q_DECLARE_INTERFACE calls in src/plugins. This also needs to be backported.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">All compiled plugins still load fine (you may have to touch some of them after applying the patch to trigger them being recompiled if Marble refuses to load them. Watch the output of --debug-info).
Used an old (=plugin with other version string) plugin to check that this one is not loaded anymore.
</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=239831">239831</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdeedu/marble/src/lib/AbstractDataPlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/PluginManager.cpp <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/RenderPlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/lib/RunnerPlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/network/qnam/QNamNetworkPlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/positionprovider/geoclue/GeoCluePositionProviderPlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/positionprovider/gpsd/GpsdPositionProviderPlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/positionprovider/maemo/MaemoPositionProviderPlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/render/fileview/FileViewFloatItem.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/render/navigation/NavigationFloatItem.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/gosmore/GosmorePlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/HostipPlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/LatLonPlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/LocalDatabasePlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/NameFinderPlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/NominatimPlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/openrouteservice/OpenRouteServicePlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/routino/RoutinoPlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/traveling-salesman/TravelingSalesmanPlugin.h <span style="color: grey">(1167911)</span></li>
<li>/trunk/KDE/kdeedu/marble/src/plugins/runner/yours/YoursPlugin.h <span style="color: grey">(1167911)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/5149/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>