AP_Beacon: Corrected possible use of nullptr memory.

The check for UART port pointer is not sufficient
to know if the update() was possible or not. When
MarvelmindHedge construction failed, the hedge pointer
might be a nullptr and there are no checks to avoid
nullptr dereference.

The MarvelmindHedge structure had complex initialization
but was done in a C style, with intermixed initialization functions.
malloc() was changed to cleaner new operator. Given that the
file already contained new operator calls it didn't make
sense to have a mix. The files are cpp so C++ operators
are used.

libraries/AP_Beacon/AP_Beacon_Marvelmind.cpp

@@ -22,6 +22,11 @@
 
 #include "AP_Beacon_Marvelmind.h"
 
+#define AP_BEACON_MARVELMIND_POSITION_DATAGRAM_ID 0x0001
+#define AP_BEACON_MARVELMIND_POSITIONS_DATAGRAM_ID 0x0002
+#define AP_BEACON_MARVELMIND_POSITION_DATAGRAM_HIGHRES_ID 0x0011
+#define AP_BEACON_MARVELMIND_POSITIONS_DATAGRAM_HIGHRES_ID 0x0012
+
 extern const AP_HAL::HAL& hal;
 
 AP_Beacon_Marvelmind::AP_Beacon_Marvelmind(AP_Beacon &frontend, AP_SerialManager &serial_manager) :
@@ -32,14 +37,12 @@ AP_Beacon_Marvelmind::AP_Beacon_Marvelmind(AP_Beacon &frontend, AP_SerialManager
         uart->begin(serial_manager.find_baudrate(AP_SerialManager::SerialProtocol_Beacon, 0));
         hedge = new MarvelmindHedge();
         last_update_ms = 0;
-        if (hedge) {
-            create_marvelmind_hedge();
+        if (hedge && hedge->position_buffer != nullptr) {
             parse_state = RECV_HDR; // current state of receive data
             num_bytes_in_block_received = 0; // bytes received
             data_id = 0;
-            start_marvelmind_hedge();
         } else {
-            // initialising beacon failed
+            hal.console->printf("MarvelMind: MarvelmindHedge failed\n");
         }
     }
 }
@@ -213,7 +216,7 @@ void AP_Beacon_Marvelmind::process_beacons_positions_highres_datagram()
 
 void AP_Beacon_Marvelmind::update(void)
 {
-    if (uart == nullptr) {
+    if (uart == nullptr || hedge == nullptr || hedge->position_buffer == nullptr) {
         return;
     }
     // read any available characters
@@ -329,37 +332,32 @@ void AP_Beacon_Marvelmind::update(void)
 //////////////////////////////////////////////////////////////////////////////
 // Create and initialize MarvelmindHedge structure
 //////////////////////////////////////////////////////////////////////////////
-void AP_Beacon_Marvelmind::create_marvelmind_hedge()
+AP_Beacon_Marvelmind::MarvelmindHedge::MarvelmindHedge() :
+    max_buffered_positions{3},
+    position_buffer{nullptr},
+    positions_beacons{},
+    pause{false},
+    receive_data_callback{nullptr},
+    _last_values_count{0},
+    _last_values_next{0},
+    _have_new_values{false}
 {
-    hedge->max_buffered_positions = 3;
-    hedge->position_buffer = nullptr;
-    hedge->verbose = false;
-    hedge->receive_data_callback = nullptr;
-    hedge->_last_values_count = 0;
-    hedge->_last_values_next = 0;
-    hedge->_have_new_values = false;
-    hedge->termination_required = false;
-}
-
-//////////////////////////////////////////////////////////////////////////////
-// Initialize and start work
-//////////////////////////////////////////////////////////////////////////////
-void AP_Beacon_Marvelmind::start_marvelmind_hedge()
-{
-    hedge->position_buffer = (PositionValue*) malloc(sizeof(struct PositionValue) * hedge->max_buffered_positions);
-    if (hedge->position_buffer == nullptr) {
-        if (hedge->verbose) {
-            hal.console->printf("MarvelMind: Not enough memory");
-        }
-        hedge->termination_required = true;
+    position_buffer = new PositionValue[max_buffered_positions];
+    if (position_buffer == nullptr) {
+        hal.console->printf("MarvelMind: Not enough memory\n");
         return;
     }
-    for (uint8_t i = 0; i < hedge->max_buffered_positions; i++) {
-        hedge->position_buffer[i].ready = false;
-        hedge->position_buffer[i].processed = false;
+    for (uint8_t i = 0; i < max_buffered_positions; i++) {
+        position_buffer[i].ready = false;
+        position_buffer[i].processed = false;
     }
-    hedge->positions_beacons.num_beacons = 0;
-    hedge->positions_beacons.updated = false;
+    positions_beacons.num_beacons = 0;
+    positions_beacons.updated = false;
+}
+
+AP_Beacon_Marvelmind::MarvelmindHedge::~MarvelmindHedge() {
+    if (position_buffer != nullptr)
+        delete[] position_buffer;
 }
 
 bool AP_Beacon_Marvelmind::healthy()

libraries/AP_Beacon/AP_Beacon_Marvelmind.h

@@ -23,10 +23,6 @@
 
 #pragma once
 
-#define AP_BEACON_MARVELMIND_POSITION_DATAGRAM_ID 0x0001
-#define AP_BEACON_MARVELMIND_POSITIONS_DATAGRAM_ID 0x0002
-#define AP_BEACON_MARVELMIND_POSITION_DATAGRAM_HIGHRES_ID 0x0011
-#define AP_BEACON_MARVELMIND_POSITIONS_DATAGRAM_HIGHRES_ID 0x0012
 #define AP_BEACON_MARVELMIND_BUF_SIZE 255
 
 #include "AP_Beacon_Backend.h"
@@ -72,12 +68,12 @@ private:
 
     struct MarvelmindHedge
     {
+        MarvelmindHedge();
+        ~MarvelmindHedge();
         uint8_t max_buffered_positions;   // maximum count of measurements of coordinates stored in buffer, default: 3
         PositionValue * position_buffer;  // buffer of measurements
         StationaryBeaconsPositions positions_beacons;
-        bool verbose;   // verbose flag which activate console output, default: False
         bool pause;     //  pause flag. If True, class would not read serial data
-        bool termination_required;  //  If True, thread would exit from main loop and stop
         void (*receive_data_callback)(PositionValue position); //  receive_data_callback is callback function to receive data
 
         uint8_t _last_values_count;
@@ -103,8 +99,6 @@ private:
     void process_beacons_positions_highres_datagram();
     void process_position_highres_datagram(PositionValue &p);
     void process_position_datagram(PositionValue &p);
-    void create_marvelmind_hedge();
-    void start_marvelmind_hedge();
     void set_stationary_beacons_positions_and_distances();
     void order_stationary_beacons();