[Marble-devel] Review Request: Clean gpsd position provider shutdown

[jensmh at gmx.de]

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

Ship it!


I could not test it, but it looks otherwise fine to me.


/trunk/KDE/kdeedu/marble/src/plugins/positionprovider/gpsd/GpsdConnection.cpp
<http://reviewboard.kde.org/r/3794/#comment4766>

    should not the "#endif" be here? otherwise for gpsd before 2.90 the gpdsInfo signal will not be emitted anymore.


- jmho


On 2010-04-23 19:38:29, Dennis Nienhüser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3794/
> -----------------------------------------------------------
> 
> (Updated 2010-04-23 19:38:29)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> When gpsd is running, but no data is available, the gpsd position provider is currently blocked indefinitely for libgsp >= 2.90 by the poll() call. In that case the destruction of the thread hangs in wait() after terminate(), which leads to marble freezing when turning gpsd off. It may also cause the crash reported in https://bugs.kde.org/show_bug.cgi?id=234311.
> 
> The patch changes the behavior to only call the blocking method when data is available (and it won't block in that case). Additionally it removes the possibly dangerous (according to qt docs) terminate() call.
> 
> 
> This addresses bug 234311.
>     https://bugs.kde.org/show_bug.cgi?id=234311
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeedu/marble/src/plugins/positionprovider/gpsd/GpsdConnection.cpp 1117994 
>   /trunk/KDE/kdeedu/marble/src/plugins/positionprovider/gpsd/GpsdPositionProviderPlugin.cpp 1117994 
> 
> Diff: http://reviewboard.kde.org/r/3794/diff
> 
> 
> Testing
> -------
> 
> Using libgps-2.92-4 and some gps device supported by gpsd.
> 
> 
> Thanks,
> 
> Dennis
> 
>