Possible Memory Leakage: memmove() and memcpy() are C Functions and are Not Promised in C++ (#3)

* Added Memory.h. (#321)

* Bug fixed: unexpected behaviors of `ArrayList`. (#321)

* Updated test ArrayList.cpp. (#321)

* Matched Python APIs. (#321)

* Removed `arraymove()`. (#321)

* Code reformatted. (#321)

* Exported Memory.h. (#321)

* Code reformatted. (#321)

Author: ATATC

keywords.txt

@@ -7,6 +7,7 @@ Peer KEYWORD1
 pulseTriggered KEYWORD2
 returnFloat KEYWORD2
 returnDouble KEYWORD2
+arraycopy KEYWORD2
 LEFT_FRONT_WHEEL_SPEED_SENSOR LITERAL1
 RIGHT_FRONT_WHEEL_SPEED_SENSOR LITERAL1
 CENTER_REAR_WHEEL_SPEED_SENSOR LITERAL1

src/ArrayList.h

@@ -3,6 +3,7 @@
 
 
 #include "Arduino.h"
+#include "Memory.h"
 
 template<typename E>
 class ArrayList {
@@ -14,8 +15,7 @@ class ArrayList {
         if (newCapacity < minCapacity)
             newCapacity = minCapacity;
         E *newArray = new E[newCapacity];
-        memmove(newArray, _array, _size * sizeof(E));
-        delete[] _array;
+        arraycopy(_array, newArray, _size);
         _array = newArray;
         _capacity = newCapacity;
     }
@@ -24,16 +24,12 @@ class ArrayList {
             grow(minCapacity);
     }
     void ensureCapacityInternal(size_t minCapacity) { ensureExplicitCapacity(max(10, minCapacity)); }
-    void add(E *elements, size_t size) {
-        ensureCapacityInternal(_size + size);
-        memcpy(_array + _size, elements, size * sizeof(E));
-    }
 
 public:
     explicit ArrayList(size_t initialCapacity = 10) :
         _size(0), _capacity(initialCapacity), _array(new E[initialCapacity]) {}
-    ArrayList(E const *const initialArray, size_t size) : _size(0), _capacity(size), _array(new E[size]) {
-        memcpy(_array, initialArray, size * sizeof(E));
+    ArrayList(E const *const initialArray, size_t size) : _size(size), _capacity(size), _array(new E[size]) {
+        arraycopy(initialArray, _array, size);
     }
     ArrayList(const ArrayList<E> &other) : ArrayList(other._array, other._size) {}
     ~ArrayList() { delete[] _array; }
@@ -42,19 +38,18 @@ class ArrayList {
     void set(int index, const E &element) { _array[index] = element; }
     E get(int index) { return _array[index]; }
     void clear() {
-        delete[] _array;
         _array = new E[0];
         _size = _capacity = 0;
     }
+    void add(E const *const elements, size_t size) {
+        ensureCapacityInternal(_size + size);
+        arraycopy(elements, _array, size, 0, _size);
+        _size += size;
+    }
     void add(const E &element) {
         ensureCapacityInternal(_size + 1);
         _array[_size++] = element;
     }
-    ArrayList<E> add(const ArrayList<E> &other) {
-        ArrayList<E> result = copy();
-        result += other;
-        return result;
-    }
     bool contains(const E &element) { return indexOf(element) >= 0; }
     int indexOfInRange(const E &element, int start, int stop) {
         for (int i = start; i < stop; i++)
@@ -70,13 +65,18 @@ class ArrayList {
         return -1;
     }
     int lastIndexOf(const E &element) { return lastIndexOfInRange(element, 0, _size); }
-    ArrayList<E> copy() { return ArrayList<E>(reinterpret_cast<size_t>(this)); }
+    ArrayList<E> copy() { return ArrayList<E>(*this); }
+    E operator[](int index) { return get(index); }
     ArrayList<E> operator+(const E &element) {
         ArrayList<E> result = copy();
         result += element;
         return result;
     }
-    ArrayList<E> operator+(const ArrayList<E> &other) { return add(other); }
+    ArrayList<E> operator+(const ArrayList<E> &other) {
+        ArrayList<E> result = copy();
+        result += other;
+        return result;
+    }
     ArrayList<E> &operator+=(const E &element) {
         add(element);
         return *this;
@@ -88,8 +88,12 @@ class ArrayList {
     ArrayList<E> &operator=(const ArrayList<E> &other) {
         if (this != &other) {
             ensureCapacityInternal(other._capacity);
-            if (other._size > 0)
-                memcpy(_array, other._array, _size * sizeof(E));
+            if (other._size > 0) {
+                _array = new E[other._capacity];
+                _capacity = other._capacity;
+                arraycopy(other._array, _array, other._size);
+                _size = other._size;
+            }
         }
         return *this;
     }

src/Controller.h

@@ -9,18 +9,17 @@ class Controller : public Device<T> {
 protected:
     ArrayList<String> _device_tags;
     ArrayList<Device<E>> _devices;
-    void _attachDevice(const String &tag, Device<E> device) {
+    void _attachDevice(const String &tag, const Device<E> &device) {
         _device_tags.add(tag);
         _devices.add(device);
         device.tag(tag);
     }
 
 public:
     Controller() : Device<T>() {}
-    int level() { return this->_parentTags.size(); }
     const ArrayList<Device<E>> &devices() { return _devices; }
     void device(const String &tag, const Device<E> &device) { _attachDevice(tag, device); }
-    Device<E> device(const String &tag) { return _devices.get(_device_tags.indexOf(tag)); }
+    Device<E> device(const String &tag) { return _devices[_device_tags.indexOf(tag)]; }
     void initialize(const ArrayList<String> &parentTags) override {
         Device<E>::initialize(parentTags);
         for (Device<E> d: _devices)

src/Device.h

@@ -15,6 +15,7 @@ class Device {
     explicit Device(const ArrayList<int> &pins) : _pins(pins) {}
     Device() : Device(ArrayList<int>(0)) {}
     ~Device() = default;
+    int level() { return _parentTags.size(); }
     void tag(const String &tag) { _tag = tag; }
     String tag() { return _tag; }
     const ArrayList<String> &parentTags() { return _parentTags; }

src/LEADS.h

@@ -1,5 +1,6 @@
 #include "Controller.h"
 #include "Device.h"
+#include "Memory.h"
 #include "Peer.h"
 #include "PredefinedTags.h"
 #include "Utils.h"

src/Memory.h

@@ -0,0 +1,12 @@
+#ifndef MEMORY_H
+#define MEMORY_H
+
+
+template<typename E>
+void arraycopy(const E *const from, E *to, int size, int fromOffset = 0, int toOffset = 0) {
+    for (int i = 0; i < size; i++)
+        to[i + toOffset] = from[i + fromOffset];
+}
+
+
+#endif // MEMORY_H

src/Peer.cpp

@@ -1,14 +1,15 @@
 #include "Peer.h"
 
 
-Peer::Peer(unsigned int baudRate, String separator, String remainder) :
+// std::move() is not available in the C library
+Peer::Peer(unsigned int baudRate, const String &separator, const String &remainder) : // NOLINT(*-pass-by-value)
     Controller<String, String>(), _baudRate(baudRate), _separator(separator), _remainder(remainder) {}
 void Peer::initialize(const ArrayList<String> &parentTags) {
     Controller<String, String>::initialize(parentTags);
     Serial.begin(_baudRate);
 }
 String Peer::read() {
-    char c = Serial.read();
+    char c = char(Serial.read());
     if (c > 0) {
         _remainder += c;
         if (!_remainder.endsWith(_separator))

src/Peer.h

@@ -11,7 +11,7 @@ class Peer : public Controller<String, String> {
     String _separator, _remainder;
 
 public:
-    explicit Peer(unsigned int baudRate = 9600, String separator = ";", String remainder = "");
+    explicit Peer(unsigned int baudRate = 9600, const String &separator = ";", const String &remainder = "");
     void initialize(const ArrayList<String> &parentTags) override;
     String read() override;
     void write(String payload) override;

src/Utils.cpp

@@ -2,6 +2,6 @@
 
 bool pulseTriggered(int pin) { return digitalRead(pin) == LOW; }
 
-void returnFloat(Peer peer, const String &tag, float n) { peer.write(tag + ":" + n); }
+void returnFloat(Peer &peer, const String &tag, float n) { peer.write(tag + ":" + n); }
 
-void returnDouble(Peer peer, const String &tag, double n) { peer.write(tag + ":" + n); }
+void returnDouble(Peer &peer, const String &tag, double n) { peer.write(tag + ":" + n); }

src/Utils.h

@@ -7,9 +7,9 @@
 
 bool pulseTriggered(int pin);
 
-void returnFloat(Peer peer, const String &tag, float n);
+void returnFloat(Peer &peer, const String &tag, float n);
 
-void returnDouble(Peer peer, const String &tag, double n);
+void returnDouble(Peer &peer, const String &tag, double n);
 
 
 #endif // UTILS_H