Changing the pointer from malloc() memory leak

[Koepel]

The pointer to the heap is changed. The pointer to the memory in the heap that was allocted with malloc() should not be changed.

In the function setAll(), the pointer to the heap is changed when doing: _digitalValues = digitalValues;.
After that, the allocated memory in the heap can no longer be used.
When the user had the data on the stack, the _digitalValues-pointer could be pointing to a memory location that is no longer valid or is used for something else.
This can be avoided with memcpy() to store the new data in the heap.

For example, storing the new data in the heap, and after that, using the values from the heap for shiftOut.

    memcpy( _digitalValues, digitalValues, numberOfShiftRegisters);   // dest, src, size
    
    for (byte = _numberOfShiftRegisters - 1; byte >= 0; byte--) {
        shiftOut(_serialDataPin, _clockPin, MSBFIRST, _digitalValues[byte]);
    }

When that bug is fixed, you can add new things:

• It would be nice to add a destructor that releases the memory with free().
• A overloaded function could be added to support PROGMEM data for the setAll() function.

Note: It is possible to use a template to create a constructor runtime with an array of a certain size.

[Simsso]

Thank you very much, @Koepel, I value your comment. It's a memory leak (that's the line of code) that would have been easy to prevent. If you have time I would appreciate a PR.

I'm not 100% sure but I think using free is not a good idea on Arduino either. The memcpy option seems reasonable.

[Koepel]

Hi, I have created a pull request.

In Arduino a "byte" is a keyword and is defined as uint8_t. I have changed the variable with the name "byte".

A destructor is added which releases the allocated memory. Normally the constructor is in the global section of a sketch, and the destructor is not used. When a ShiftRegister74HC595 object was created runtime, then the destructor could be called.

The parameter of setAll() has now the keyword 'const' to allow to use 'const' data.

For me, it seems better that the updateRegisters() should update the data to the hardware shift register, and all the other functions should operate on the data. I don't know if you agree, but I think it is more straightforward.

I have added a "experimental" function that accepts data from PROGMEM. I ran into problems when I tried to make a overloaded function, so now it has its own name: setAll_P(). That function works, but only for basic Arduino boards with a AVR microcontroller.