From 87202a08b1738affdce4531aa6872a08474d62d4 Mon Sep 17 00:00:00 2001 From: Erik de Castro Lopo Date: Thu, 26 Nov 2015 09:34:48 +1100 Subject: [PATCH 1/3] Navigator: Reorder data members to fix valgrind warnings During construction of an Navigator object, a pointer to the incomplete object was passed to the RTL constructor which then called a method on the incomplete Navigator object accessing uninitialized data. --- src/modules/navigator/navigator.h | 10 +++++----- src/modules/navigator/navigator_main.cpp | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/modules/navigator/navigator.h b/src/modules/navigator/navigator.h index 6facdd9081..b154d21769 100644 --- a/src/modules/navigator/navigator.h +++ b/src/modules/navigator/navigator.h @@ -217,6 +217,11 @@ private: bool _inside_fence; /**< vehicle is inside fence */ + bool _can_loiter_at_sp; /**< flags if current position SP can be used to loiter */ + bool _pos_sp_triplet_updated; /**< flags if position SP triplet needs to be published */ + bool _pos_sp_triplet_published_invalid_once; /**< flags if position SP triplet has been published once to UORB */ + bool _mission_result_updated; /**< flags if mission result has seen an update */ + NavigatorMode *_navigation_mode; /**< abstract pointer to current navigation mode class */ Mission _mission; /**< class that handles the missions */ Loiter _loiter; /**< class that handles loiter */ @@ -231,11 +236,6 @@ private: NavigatorMode *_navigation_mode_array[NAVIGATOR_MODE_ARRAY_SIZE]; /**< array of navigation modes */ - bool _can_loiter_at_sp; /**< flags if current position SP can be used to loiter */ - bool _pos_sp_triplet_updated; /**< flags if position SP triplet needs to be published */ - bool _pos_sp_triplet_published_invalid_once; /**< flags if position SP triplet has been published once to UORB */ - bool _mission_result_updated; /**< flags if mission result has seen an update */ - control::BlockParamFloat _param_loiter_radius; /**< loiter radius for fixedwing */ control::BlockParamFloat _param_acceptance_radius; /**< acceptance for takeoff */ control::BlockParamInt _param_datalinkloss_obc; /**< if true: obc mode on data link loss enabled */ diff --git a/src/modules/navigator/navigator_main.cpp b/src/modules/navigator/navigator_main.cpp index 0227a2ed07..cd6021a971 100644 --- a/src/modules/navigator/navigator_main.cpp +++ b/src/modules/navigator/navigator_main.cpp @@ -134,6 +134,10 @@ Navigator::Navigator() : _geofence{}, _geofence_violation_warning_sent(false), _inside_fence(true), + _can_loiter_at_sp(false), + _pos_sp_triplet_updated(false), + _pos_sp_triplet_published_invalid_once(false), + _mission_result_updated(false), _navigation_mode(nullptr), _mission(this, "MIS"), _loiter(this, "LOI"), @@ -143,10 +147,6 @@ Navigator::Navigator() : _dataLinkLoss(this, "DLL"), _engineFailure(this, "EF"), _gpsFailure(this, "GPSF"), - _can_loiter_at_sp(false), - _pos_sp_triplet_updated(false), - _pos_sp_triplet_published_invalid_once(false), - _mission_result_updated(false), _param_loiter_radius(this, "LOITER_RAD"), _param_acceptance_radius(this, "ACC_RAD"), _param_datalinkloss_obc(this, "DLL_OBC"), @@ -694,7 +694,7 @@ void Navigator::publish_mission_result() { _mission_result.instance_count = _mission_instance_count; - + /* lazily publish the mission result only once available */ if (_mission_result_pub != nullptr) { /* publish mission result */ From 5e9a8d0c039a6828b1c8ae70c299402620990831 Mon Sep 17 00:00:00 2001 From: Erik de Castro Lopo Date: Thu, 26 Nov 2015 09:39:24 +1100 Subject: [PATCH 2/3] Baro sim: Add missing initializers --- src/platforms/posix/drivers/barosim/baro.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/platforms/posix/drivers/barosim/baro.cpp b/src/platforms/posix/drivers/barosim/baro.cpp index 0bbb2c1c0d..ac5d31b6b9 100644 --- a/src/platforms/posix/drivers/barosim/baro.cpp +++ b/src/platforms/posix/drivers/barosim/baro.cpp @@ -202,6 +202,8 @@ BAROSIM::BAROSIM(const char *path) : _TEMP(0), _OFF(0), _SENS(0), + _P(0.0), + _T(0.0), _msl_pressure(101325), _baro_topic(nullptr), _orb_class_instance(-1), From 4f7ab6f4f359b7af3b91f87e1926fdc2069c97e6 Mon Sep 17 00:00:00 2001 From: Erik de Castro Lopo Date: Thu, 26 Nov 2015 12:18:42 +1100 Subject: [PATCH 3/3] uORBManager: Make `orb_check` fail safely The `orb_check` function takes a pointer to a `bool` which it then passes to `px4_ioctl`. However, if the call to `px4_ioctl` fails, the bool doesn't get updated (neither set nor cleared). Therefore, in `orb_check` we explicitly set the bool to `false` before passing the pointer to `px4_ioctl`. --- src/modules/uORB/uORBManager_posix.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/modules/uORB/uORBManager_posix.cpp b/src/modules/uORB/uORBManager_posix.cpp index 7fe3f6a687..3a01038b4a 100644 --- a/src/modules/uORB/uORBManager_posix.cpp +++ b/src/modules/uORB/uORBManager_posix.cpp @@ -164,6 +164,8 @@ int uORB::Manager::orb_copy(const struct orb_metadata *meta, int handle, void *b int uORB::Manager::orb_check(int handle, bool *updated) { + /* Set to false here so that if `px4_ioctl` fails to false. */ + *updated = false; return px4_ioctl(handle, ORBIOCUPDATED, (unsigned long)(uintptr_t)updated); }