Storing variable-size pointers array as class variable

[activecat]

Hello guys,

This problem is best explained by looking at its output below.
I stored 15 different values but when retrieved some are wrong.

Stored value = 1
Stored value = 2
Stored value = 3
Stored value = 4
Stored value = 5

Stored value = 11
Stored value = 12
Stored value = 13
Stored value = 14
Stored value = 15

Stored value = 21
Stored value = 22
Stored value = 23
Stored value = 24
Stored value = 25


Retrieved value = 21
Retrieved value = 22
Retrieved value = 23
Retrieved value = 24
Retrieved value = 25

Retrieved value = 21
Retrieved value = 22
Retrieved value = 23
Retrieved value = 24
Retrieved value = 25

Retrieved value = 21
Retrieved value = 22
Retrieved value = 23
Retrieved value = 24
Retrieved value = 25

Now let's look at the code that produces it:

#include <iostream>
using namespace std;


class A
{
    public:
        int ** pointer;
};


A *a [3];


void f1()
{
    for (int i=0; i < 3; i++)
    {
        for ( int j=0; j < 5; j++)
            cout << "Retrieved value = " << *a[i]->pointer[j]  << endl;
        cout << endl;
    }
}



int main()
{
    for (int i=0; i < 3; i++)
      a[i] = new A;


    for (int i=0; i < 3; i++)
    {
        int *k[5];
        for (int j=0; j < 5; j++)
        {
            int value =j + i * 10 + 1;
            k[j] = new int;
            * k[j] = value;
            cout << "Stored value = " << value << endl;
        }
        cout << endl;

        a[i]->pointer = k;
    }

    cout << endl;
    f1();

    return 0;
}

I need to store and retrieve the 15 different values correctly.
How to solve this?
The main thing is, I need to store the pointer in class A objects.

[AbstractionAnon]

You've created only one instance of k containing 5 pointers (line 35).
Lines 36/42 you store your values, allocating pointers to ints and storing values into those ints. Second and third time through the loop, you're overwriting the previously allocated pointers (a memory leak) in k[] with the newly allocated pointers.
When you get line 45, you're storing the base address of k[] (local to the outter loop).

Frankly, I'm surprised this is even working and is not crashing. When you call f1() at line 49, both k[] and the pointers that were stored in k[] have gone out of scope are no longer valid, therefore what a[]->pointer[] points to isn't valid.

What you need to do is, after line 40:

  a[i]->pointer[j] = k[j];

Note: This does not address your memory leak.

[activecat]

Dear AbstractionAnon,

Thanks. You inspired me to find the solution.
I think this time there is no memory leak anymore, because:

i) all memory created for *int k[5]; was automatically reclaimed back after the for() loop exits

ii) all memory created by new int; were not wasted, they were used accordingly to store data.

The output of the program:

Iteration 1: With k = 0x7fff16363430 (this k[] will become invalid outside the scope of this for() loop)
 Data to store = 1, into  k[0] = 0xacf9a0 (space created by 'new int;')
 Data to store = 2, into  k[1] = 0xacf9c0 (space created by 'new int;')
 Data to store = 3, into  k[2] = 0xacf9e0 (space created by 'new int;')
 Data to store = 4, into  k[3] = 0xacfa00 (space created by 'new int;')
 Data to store = 5, into  k[4] = 0xacfa20 (space created by 'new int;')

Iteration 2: With k = 0x7fff16363430 (this k[] will become invalid outside the scope of this for() loop)
 Data to store = 11, into  k[0] = 0xacfa40 (space created by 'new int;')
 Data to store = 12, into  k[1] = 0xacfa60 (space created by 'new int;')
 Data to store = 13, into  k[2] = 0xacfa80 (space created by 'new int;')
 Data to store = 14, into  k[3] = 0xacfaa0 (space created by 'new int;')
 Data to store = 15, into  k[4] = 0xacfac0 (space created by 'new int;')

Iteration 3: With k = 0x7fff16363430 (this k[] will become invalid outside the scope of this for() loop)
 Data to store = 21, into  k[0] = 0xacfae0 (space created by 'new int;')
 Data to store = 22, into  k[1] = 0xacfb00 (space created by 'new int;')
 Data to store = 23, into  k[2] = 0xacfb20 (space created by 'new int;')
 Data to store = 24, into  k[3] = 0xacfb40 (space created by 'new int;')
 Data to store = 25, into  k[4] = 0xacfb60 (space created by 'new int;')


Iteration 1: With pointer = 0xacf010 ( *pointer[100] was automatically created for each instance of class A)
 Retrieved Data = 1,  from pointer[0] = 0xacf9a0 (pointer stored earlier)
 Retrieved Data = 2,  from pointer[1] = 0xacf9c0 (pointer stored earlier)
 Retrieved Data = 3,  from pointer[2] = 0xacf9e0 (pointer stored earlier)
 Retrieved Data = 4,  from pointer[3] = 0xacfa00 (pointer stored earlier)
 Retrieved Data = 5,  from pointer[4] = 0xacfa20 (pointer stored earlier)

Iteration 2: With pointer = 0xacf340 ( *pointer[100] was automatically created for each instance of class A)
 Retrieved Data = 11,  from pointer[0] = 0xacfa40 (pointer stored earlier)
 Retrieved Data = 12,  from pointer[1] = 0xacfa60 (pointer stored earlier)
 Retrieved Data = 13,  from pointer[2] = 0xacfa80 (pointer stored earlier)
 Retrieved Data = 14,  from pointer[3] = 0xacfaa0 (pointer stored earlier)
 Retrieved Data = 15,  from pointer[4] = 0xacfac0 (pointer stored earlier)

Iteration 3: With pointer = 0xacf670 ( *pointer[100] was automatically created for each instance of class A)
 Retrieved Data = 21,  from pointer[0] = 0xacfae0 (pointer stored earlier)
 Retrieved Data = 22,  from pointer[1] = 0xacfb00 (pointer stored earlier)
 Retrieved Data = 23,  from pointer[2] = 0xacfb20 (pointer stored earlier)
 Retrieved Data = 24,  from pointer[3] = 0xacfb40 (pointer stored earlier)
 Retrieved Data = 25,  from pointer[4] = 0xacfb60 (pointer stored earlier)

The code to produce it is:

#include <iostream>
using namespace std;


class A
{
    public:
        int * pointer[100];    // assuming 100 is the maximum
};


A *a [3];


void f1()
{
    for (int i=0; i < 3; i++)
    {
        cout << "Iteration " << i+1 << ": ";
        cout << "With pointer = " << a[i]->pointer ;
        cout << " ( *pointer[100] was automatically created for each instance of class A)" << endl;
        for ( int j=0; j < 5; j++)
        {
            cout << " Retrieved Data = " << *a[i]->pointer[j]  << ", ";
            cout << " from pointer[" << j << "] = " << a[i]->pointer[j] << " (pointer stored earlier)" << endl;
        }
        cout << endl;
    }
}



int main()
{
    for (int i=0; i < 3; i++)
      a[i] = new A;


    for (int i=0; i < 3; i++)
    {
        int *k[5];  // this will be automatically deleted outside the scope of this for(int i=0; i < 3; i++) loop.
        cout << "Iteration " << i+1 << ": ";
        cout << "With k = " << k << " (this k[] will become invalid outside the scope of this for() loop)" << endl;

        for (int j=0; j < 5; j++)
        {
            int value =j + i * 10 + 1;

            k[j] = new int;
            * k[j] = value;

            cout << " Data to store = " << value << ", ";
            cout << "into  k[" << j << "] = " << k[j] << " (space created by 'new int;')" << endl;

            a[i]->pointer[j] = k[j];     // proposed by AbstractionAnon
        }
        cout << endl;

        //a[i]->pointer = k;   // this doesn't work
    }

    cout << endl;
    f1();

    return 0;
}

But the disadvantage of this approach is that, we need to pre-allocate a fixed size of pointers array, in this case its size is 100:

int * pointer[100];


Is there any way to make it dynamic (rather than fixed at 100) ?
So far there is no solution.

Thank you very much.

[binarybob350]

Use a vector instead (which is sized dynamically if you use push_back)

vector< int * > pointer;

[AbstractionAnon]

ii) all memory created by new int; were not wasted, they were used accordingly to store data.

You still have a memory leak. Every memory allocation done with new should be matched with a delete.

Consider the following:

  for (int i=0; i<1000; i++)
  {  A  a;  //  a is constructed each time through the loop
      a.pointer[i] = new int;
  }  //  A is destructed each time through the loop
  //  What a.pointer[] pointed to is now lost 

Your destructor for A needs to release the memory owned by it.

A::~A ()
{  for (int i=0; i<1000; i++)
      delete pointer[i];
}

[activecat]

Hi AbstractionAnon,

Yes, I agree with you.

In fact all objects created by new shall be cleaned up using delete.

In this program we create objects using new int (line 49) and also new A (line 36).

So when cleaning up, we shall issue:

i). delete k[j] for those objects created by new int.

ii). delete a[i] for those objects created by new A.

But I forget to describe that, in the real environment, all dynamically created objects here will be needed all the way until the program exits. So when the main program exits, all dynamically created objects will be automatically cleaned up by the operating system.

No memory leaks.

[activecat]

To binarybob350,

Thanks.
Vector indeed is one of the solutions. I will implement it using vector.