Do I use dynamic memory correctly ?

[GodIsDead]

Hello,

I'm a new dynamic memory allocator :)

I'm working on projects for microcontrollers like Arduino, so I want to reduce memory usage of my code, as much as possible. I have a program where I must store numbers in arrays, but I don't know how much numbers, so naturally I need a dynamic array that will grow as needed. I don't want to use Vectors as they use more memory and more resources, and I want to keep my code as simple as possible.

Can you tell me if there is something wrong in this example code: https://ideone.com/mOS5aO

It works, but I'm not sure if I do it correctly, I'm affraid of memory leaks or other problems. Do I use delete[] correctly, especially in addToArray function ? Is there anything that could be improved ?

It's just an example to demonstrate what I want to do, in my real class, most of those things will be private and I won't be calling addToArray directly.

Thanks a lot already ;)

[JLBorges]

> I don't want to use Vectors as they use more memory and more resources,
> and I want to keep my code as simple as possible.

Using vectors would make the code simpler; and with a bit of care, vectors would not use more resources.

[jonnin]

can't access site.
but dynamic memory is really simple.
for every new, call a delete. If you allocate an array, delete [] array syntax, or alternately, always allocate as an 'array' even for 1 variable, eg int *x = new int[1]; delete[] x;
Another way to deal with it is to code the new and delete immediately, and 'drag' the delete statement along as you write code between them, until it finds the right spot in the code. you can leave the ; off of it until its in the right spot, so the compiler will 'remind' you to get it in the right spot. This is a type of defensive coding that I use... if I type a { I also type a } immediately, and fill in between, same for ( ) pairs and so on. Not that these things are hard to find and fix if you get lost in the code, but it saves time esp if you are prone to messing up paired constructs.

if you did that, it is correct.
if you have trouble keeping track, a micro-class or struct with nothing but a ctor (size of the array to allocate) and a dtor( self destruct upon going out of scope) will do it.

Vectors have been tuned. Long long ago they were a lot slower and a lot more careless with resources; I avoided them for a long time after they first came out. They are quite good now. You may want to rethink. All you really pay for, if you avoid frequent dynamic on the fly resize (which is easy for most code), is the integer size, which you probably were going to maintain on the side yourself for most arrays.

if you will post your sample in code tags, I will be happy to take a look.

[jlb]

In main() why are you deleting memory you didn't allocate in main? The memory allocated by the class should be deleted by the class destructor.

By the way if you're dealing with an embedded memory limited microcontroller you probably should not be using dynamic memory of any kind. Instead use fixed array sizes (of reasonable size) and if necessary deal with partial subsets of the data at one time.

[GodIsDead]

jonnin here is the code in the link:

#include <iostream>
#include <string.h>
using namespace std;
 
class myClass
{
    public :
 
        myClass();
        ~myClass();
 
        struct DynamicArray
        {
            uint8_t * data;
            size_t size;
        };
 
        void addToArray( DynamicArray & array, uint8_t * data, size_t len );
 
        DynamicArray myDynamicArray1; 
        DynamicArray myDynamicArray2;
};
 
myClass::myClass() :
    myDynamicArray1( { nullptr, 0 } ),
    myDynamicArray2( { nullptr, 0 } )
{
}
 
myClass::~myClass()
{
    if ( myDynamicArray1.data != nullptr )
    {
        delete[] myDynamicArray1.data;
    }
    if ( myDynamicArray2.data != nullptr )
    {
        delete[] myDynamicArray2.data;
    }
}
 
void myClass::addToArray( DynamicArray & array, uint8_t * data, size_t len )
{
    DynamicArray tmp;
    tmp.size = array.size + len;
    tmp.data = new (nothrow) uint8_t[ tmp.size ];
 
    if ( tmp.data != nullptr )
    {
        if ( array.data != nullptr )
        {
            memcpy( tmp.data, array.data, array.size );
            delete[] array.data;
        }
        memcpy( tmp.data + array.size, data, len );
        array.data = tmp.data;
        array.size = tmp.size;
    }
    else
    {
        printf("sorry, couldn't add to array, memory allocation failed!\n");
    }
}
 
 
 
myClass test;
 
int main()
{
    uint8_t a[] = { 10, 20, 30, 40, 50, 60, 70 };
    uint8_t b[] = { 80, 90, 100 };
 
    test.addToArray( test.myDynamicArray1, a, 7 );
    test.addToArray( test.myDynamicArray2, b, 2 );
    test.addToArray( test.myDynamicArray1, b, 3 );
    test.addToArray( test.myDynamicArray2, a, 5 );
 
    for ( size_t i = 0; i < test.myDynamicArray1.size; i++ )
    {
        printf( "test.myDynamicArray1.data[%d] = %d\n", i, test.myDynamicArray1.data[i] );
    }
    printf( "\n" );
    for ( size_t i = 0; i < test.myDynamicArray2.size; i++ )
    {
        printf( "test.myDynamicArray2.data[%d] = %d\n", i, test.myDynamicArray2.data[i] );
    }
    printf( "\n" );
 
    size_t totalSize = test.myDynamicArray1.size + test.myDynamicArray2.size;
    uint8_t * allData = new (nothrow) uint8_t[ totalSize ];
    if ( allData != nullptr )
    {
        memcpy( allData, test.myDynamicArray1.data, test.myDynamicArray1.size );
        memcpy( allData + test.myDynamicArray1.size, test.myDynamicArray2.data, test.myDynamicArray2.size );
 
        for ( size_t i = 0; i < totalSize; i++ )
        {
            printf("allData[%d] = %d\n", i, allData[i] );
        }
 
        delete[] allData;
    }
 
    delete[] test.myDynamicArray1.data;
    delete[] test.myDynamicArray2.data;
 
    return 0;
}

jlb, all the dynamic allocation will be done at the beginning of the program (in setup() if you are familiar with arduino), is it still a problem then?


Is it ok to do tmp.data = new (nothrow) uint8_t[ tmp.size ];
but never do delete[] tmp.data; ?
If I do so, the content of the arrays is garbage.

[jlb]

all the dynamic allocation will be done at the beginning of the program (in setup() if you are familiar with arduino), is it still a problem then?

No I'm not familiar with the Arduino.

Dynamic memory usually happens at runtime, you normally don't know how much memory you will need until some external event triggers the "need". How do you know how much memory you need when setup() is called? No matter what you need to insure that you don't exhaust the available memory, which can be very easy with many embedded processors.

Is it ok to do tmp.data = new (nothrow) uint8_t[ tmp.size ];
but never do delete[] tmp.data; ?

If you "new[]" memory you need to "delete[]" the memory when you're finished with it. Remember embedded processors tend to run for long times and leaking memory is a bad idea.

[GodIsDead]

But I do array.data = tmp.data; and then delete[] array.data; so it's like tmp.data is deleted anyway, correct ? Again, if I do delete[] tmp.data I get garbage in the arrays.

[Ganado]

Don't use (dereference) tmp.data after you delete[] tmp.data!

By doing array.data = temp.data, you're simply copying a pointer. So if you delete[] tmp.data, you're deleting the same memory that array.data points to. Don't dereference either after a delete call.

[Niccolo]

The Arduino Uno (Rev3) is listed as having 2K of RAM. That's 2048 bytes of RAM. Think about that a moment.

There's 32K of flash.

It's a VERY small machine. It is correct to contemplate extremely efficient strategies.

This profile is like the very early "personal computers" we used back in the 70's, though the ARM is more powerful.

I haven't used Arduino, but other very similar devices I have used (ARM processor, minimal RAM, small Flash, single CPU) use Flash in a way that you'd consider RAM, and use the 2K of static RAM as a cache. The machine, therefore, has no actual storage (no disk) in the programming model, and can only hold and run one program at a time. At least that's typical and what I'd expect of this.

I've not reviewed the code for specific issues, I'll leave that to the capable posts above. I'm thinking on more strategic notions.

What the code is doing is VERY similar to what std::vector does. By the time you get it working reliably it will be a simplified implementation of std::vector. You'll gain very little, and lose a lot of reliability (and time). At most you'd gain, under the best circumstances, a minor % of space savings. The copy of the dynamic array at expansion is the problem.

One strategy is to acknowledge the unfortunate fact that the storage limit is severe. Determine the maximum practical volume, give that as a limit to the user in documentation and allocate that at the start of the program. This approach will give you a larger upper maximum of items you can store, because any dynamic vector requires enough RAM (in this case flash, or 32 K) to perform the expansion copy, a time when TWO COPIES of every element will be stored in flash for the duration of the copy. You gain little in such cramped space with such a dynamic strategy. You'll never be able to squeeze out more use of the RAM that if you always had TWO COPIES of every element due to the expansion 'moment'. When you think about it, one allocation of the maximum (no dynamic expansion) is, in the context of such a small machine, the only strategy that offers the absolute maximum potential elements you'll be able to store, and has the least overhead.

Keep in mind that since this is (likely) to be a machine that can't run more than one program at a time, the only competition for RAM you have is your own. So, you simply need to figure out how much RAM you need for everything else, which leaves the maximum you can expect to allocate for this array (so just use it).

An alternative is a compromise between vectors and linked lists. Linked lists can't be reference with [], but do dynamically expand. Unfortunately they add overhead for each entry, and if the entry is small this can be up to double (or triple) the size per element making it unsuitable for this situation.

The compromise is to create a list of small arrays. This can be implemented many ways, but in this case I'd choose an array of arrays. This is somethin like a dequeue in the library. std::dequeue could be used as a pre-built solution, or you could build your own.

The idea is that you have one small array that stores pointers to an array of the element type to be stored. This forms a 2d grid of storage, where only this 'array of array pointers' is persistent (is allocated at initialization and stays put during execution until completed). It's small.

To be clear (and maybe too long), the 'array of arrays' is like the titles of the columns on a spreadsheet. The arrays of elements are the columns. You're adding columns as you expand, without any copy stage required. Only the 'titles' persist throughout the duration of execution. When a column is 'empty' (not allocated), it isn't taking up space. This makes it a dynamically expanding structure, where each expansion adds the size of this column of elements; an array of elements.

The structure can be accessed with a [] operator, the way dequeue does it, with simple math. If the element arrays were, say, 100 elements in size, then element 99 would be the last index of the first element array, while element 100 would be the first index of the second element array. For any index, say 150, you'd calculate (index / 100) to get the index in the 'array of arrays', then ( index % 100) to get the index within that element array, making it fairly quick and direct.

While dequeue could be used, you can't easily control the size of each "interior array". For such tight a squeeze, you may well need to construct your own to gain that control.

The only downside is the storage of the 'array of arrays', but it's much less than a copy stage of a 'dynamic array' (aka vector). The major upsides are that the maximum number of elements you'll be able to store is higher than with vector (or custom version like you're making), and performance is better overall because there's no copy (which is quite slow when flash is used as if it were RAM).

I didn't check the code above for this, but in all cases you must contend with the potential 'out of memory' condition this cramped space is likely to cause.

[GodIsDead]

Ok, thank you all for answers, especially Niccolo :)

I'm still not sure how to do what I want (because I didn't explain everything about what my class is doing, it's quite complicated and my example is just showing a little part of it). I will try different things.