[Marble-devel] Review Request: Stricter checks when loading plugins

[Dennis Nienhüser]

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

Review request for marble.


Summary
-------

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.


This addresses bug 239831.
    https://bugs.kde.org/show_bug.cgi?id=239831


Diffs
-----

  /trunk/KDE/kdeedu/marble/src/lib/AbstractDataPlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/lib/PluginManager.cpp 1167911 
  /trunk/KDE/kdeedu/marble/src/lib/RenderPlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/lib/RunnerPlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/network/qnam/QNamNetworkPlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/positionprovider/geoclue/GeoCluePositionProviderPlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/positionprovider/gpsd/GpsdPositionProviderPlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/positionprovider/maemo/MaemoPositionProviderPlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/render/fileview/FileViewFloatItem.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/render/navigation/NavigationFloatItem.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/gosmore/GosmorePlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/hostip/HostipPlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/latlon/LatLonPlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/localdatabase/LocalDatabasePlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/namefinder/NameFinderPlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/nominatim/NominatimPlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/openrouteservice/OpenRouteServicePlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/routino/RoutinoPlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/traveling-salesman/TravelingSalesmanPlugin.h 1167911 
  /trunk/KDE/kdeedu/marble/src/plugins/runner/yours/YoursPlugin.h 1167911 

Diff: http://reviewboard.kde.org/r/5149/diff


Testing
-------

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.


Thanks,

Dennis

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/marble-devel/attachments/20100825/c98d465c/attachment.htm