Search code examples
pointersesp32arduino-c++arduino-esp32

Pass array of pointers to objects as constructor parameter


I am in need of passing a pointer to an array with pointers to a list of objects as constructor argument of a newly-generated object. If I manage to get it to compile and run on an ESP32, it seems that there are out of bounds memory accesses.

Don't worry about the interrupt wording; the system is reading a register of an external MCP23017 GPIO extender which only internally handles interrupts. The routine is designed to periodically check, whether an interrupt has occurred to one of the MCP23017s pins and on which pin the interrupt occurred. The object which corresponds to the pin should then be selected and the action() function of this object called.

Here is code that compiles:

class StateManager {
    private:
        MCP *mcp1;
        StateSensor * arrSensor;
    public:
        StateManager(MCP * pMcp1, StateSensor * pArrSensor);
        void checkInterrupt();
};

//Constructor
StateManager::StateManager(MCP * pMcp1, MCP * pMcp2, StateSensor * pArrSensor) {
    mcp1 = pMcp1;
    mcp2 = pMcp2;
    arrSensor = pArrSensor;
}

//Function in which the faulty access happens
void StateManager::checkInterrupt() {
    uint8_t pinLI1 = mcp1->getLastInterruptPin();
    ESP_LOGI(TAG, "pinLI1: %d", pinLI1);
    if ( pinLI1 != 255 ) {
        ESP_LOGI(TAG, "If clause reached");
        for( uint8_t i = 0; i < 16; i++ ) {
            ESP_LOGI(TAG, "for: %d, getPin(): %d", i, arrSensor[i].getPin());
            if( arrSensor[i].getPin() == pinLI1 ) {
                ESP_LOGI(TAG, "Hit: %d", i);
                arrSensor[i].action();
                break;
            }
        }
    }
}

StateSensor sens01(1,  &mcp02, 7);
StateSensor sens02(2,  &mcp02, 6);
StateSensor sens03(3,  &mcp02, 5);
StateSensor sens04(4,  &mcp02, 4);
StateSensor * arrSens[32] = { &sens01, &sens02, &sens03, &sens04 }

StateManager sm1(&mcp02, &mcp03, *arrSens);

void setup() {
    //...
}

void loop() {
    sm1.checkInterrupt();
}

Here is log output of (1) the first element of the array is triggered and (2) the second element of the array is triggered which results in the random return values. Expected values for getPin() are 0 to 15. 255 is returned by mcp1->getLastInterruptPin() when no interrupt has occurred.

[stateSensor.cpp:55] checkInterrupt(): [StateSensor] pinLI1: 255
[stateSensor.cpp:55] checkInterrupt(): [StateSensor] pinLI1: 255
(1)
[stateSensor.cpp:55] checkInterrupt(): [StateSensor] pinLI1: 7
[stateSensor.cpp:57] checkInterrupt(): [StateSensor] If clause reached
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 0, getPin(): 7
[stateSensor.cpp:61] checkInterrupt(): [StateSensor] Hit: 0
[stateSensor.cpp:35] action(): [StateSensor] 1 action!
[stateSensor.cpp:55] checkInterrupt(): [StateSensor] pinLI1: 255
[stateSensor.cpp:55] checkInterrupt(): [StateSensor] pinLI1: 255
[stateSensor.cpp:55] checkInterrupt(): [StateSensor] pinLI1: 255
[stateSensor.cpp:55] checkInterrupt(): [StateSensor] pinLI1: 255
(2)
[stateSensor.cpp:55] checkInterrupt(): [StateSensor] pinLI1: 6
[stateSensor.cpp:57] checkInterrupt(): [StateSensor] If clause reached
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 0, getPin(): 7
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 1, getPin(): 0
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 2, getPin(): 4
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 3, getPin(): 0
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 4, getPin(): 10
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 5, getPin(): 0
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 6, getPin(): 8
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 7, getPin(): 28
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 8, getPin(): 223
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 9, getPin(): 211
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 10, getPin(): 1
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 11, getPin(): 131
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 12, getPin(): 251
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 13, getPin(): 122
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 14, getPin(): 32
[stateSensor.cpp:59] checkInterrupt(): [StateSensor] for: 15, getPin(): 112
[stateSensor.cpp:55] checkInterrupt(): [StateSensor] pinLI1: 255
[stateSensor.cpp:55] checkInterrupt(): [StateSensor] pinLI1: 255

When the first or the last element of the array are triggered, the right pointer to the object is read from the array. For all elements in-between these, the operation arrSensor[i].getPin() returns a random number between 0 and 255.

How can I efficiently handle the handover of the array containing the elements, while also fixing the problems with out-of-bounds memory accesses? Or am I overlooking a fundamental mistake in my approach?


Solution

  • StateSensor * arrSensor; is a pointer to a single StateSensor object or it is an array of StateSensor objects (not pointers - like StateSensor * my_array = new StateSensor[10]).

    However, it looks like you want an array of StateSensor pointers, which you can declare as StateSensor ** and change the constructor's signature to StateManager(MCP * pMcp1, StateSensor ** pArrSensor);. (I guess you are not using std::vector because this is a micro-processor).

    Also, StateManager sm1(&mcp02, &mcp03, *arrSens); is passing arrSens[0] to the constructor, not the entire array. To pass the array use StateManager sm1(&mcp02, &mcp03, arrSens);.

    Then you need to change the checkInterrupt function to deal with an array of pointers, using -> to dereference them.

    for (uint8_t i = 0; i < 16; i++) {
        ESP_LOGI(TAG, "for: %d, getPin(): %d", i, arrSensor[i]->getPin());
        if (arrSensor[i]->getPin() == pinLI1) {
            ESP_LOGI(TAG, "Hit: %d", i);
            arrSensor[i]->action();
            break;
        }
    }
    

    Which brings up the last point: the loop can iterate up to 16 times but you only have 4 pointers in the array. Not a problem if the break is guaranteed to be hit in time, but if not, you need a way to make sure you don't read past the end of the array or dereference a nullptr.