0

Consider the following code:

#include <Windows.h>
#include <stdio.h>

void print2d(int** A,int Y,int X)
{
    for(int i=0;i<Y;i++)
    {
        for(int j=0;j<X;j++)
            printf("%d ", A[i][j]);
        printf("\n");
    }
}

int main()
{
    int Y=2;
    int X=4;
    int** tab2d=new int*[Y];
    for(int i=0;i<Y;i++)
    {
        tab2d[i]=new int[X];
        for(int j=0;j<X;j++)
            tab2d[i][j]=0;
    }

    int* row=new int[X];
        for(int i=0;i<X;i++)
        row[i]=i;

    int option=0;
    switch(option)
    {
    case 0:
        tab2d[Y-1]=row;

    case 1:
        for(int i=0;i<X;i++)
            tab2d[Y-1][i]=i;
    }

    print2d(tab2d,Y,X);
    free(tab2d);
    free(row);

    system("pause");
}

Given that in real code, there will be a lot of iterations on 2d array, is there any advantage to either case 0 or case 1? Assigning new row pointer seems like a nicer approach, but wouldn't it space my array rows all over the memory?

Zibi
  • 350
  • 1
  • 13
  • 1
    Your switch statement has a fall through, which is a problem. case 0 and case 1 do very different things. Case 0 results in a memory leak (as the memory buffer assigned to tab2d[Y-1]) and a segfault if you were cleaning up properly (add `for (int i = 0; i < Y; i++) {delete []tab2d[i];}` before `free(tab2d);`) – IdeaHat Jun 03 '14 at 14:31
  • Don't mix `new` and `free`. Consider to use `std::vector` or `std::array`. – Jarod42 Jun 03 '14 at 15:03

3 Answers3

1

These two solutions are NOT the same.

case 0: Here the array element is changed to point at a new row that you have created (that is the row allocated when you initialize row). This is almost certainly NOT what you because a) it leave a memory leak in that the array element that you originally created is never deallocated, additionally when you delete row the memory that tab2d points at is no longer valid (which will cause undefined behavior once you access it - most probably a crash).

case 1: Does what you expect it to.

Also I think you need to know that in general your final delete doesn't delete the objects that the pointers reference in the 2D array, just the array itself. So even in case 1 you are generating an pretty serious memory leak.

Elemental
  • 7,365
  • 2
  • 28
  • 33
1

A few things:

1) If your rows will all be the same number of columns, then your general approach to creating and destroying the 2d array is suboptimal. See the answer to another approach here:

Delete 2D array C++

2) As to creating a new row just to replace values -- why create a brand new row? Why needlessly call the allocator (and deallocator to delete the old row)? Why not just change the values in the existing row of the 2d array?

I can see if you're removing or adding rows to your 2d array at runtime, where then you would be justified in creating rows and destroying individual rows. But if you don't change the size of the 2d array and instead, just change values within the 2d array, don't create new rows.

Community
  • 1
  • 1
PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45
-1

First , do not use new/delete and malloc/free together. otherwise the behavior is undefined . Second , your 3rd for statement must not be indented . About your question : For every new/malloc in the code , there should exist a matching delete/free . in option 0 (and even option 1 ) , you are losing the reference to tab2d[Y-1] , causing a memory leak , AND a double delete at the end when you delete 'row' AND 'tab2d[Y-1]' while both of them point to the same address , in which the behavior is undefined .

Here , what you do in option 0 is incorrect , but if you do what you meant to do correctly , and depending on the situation , it could the more efficient way .

ColonelMo
  • 134
  • 1
  • 9
  • 1
    Um...no. Option 0, even if correct, is the much more inefficient way. You have to allocate a different memory block and assign it, which takes the same amount of time as assigning the existing memory. But you have to do the memory management, which takes time and is a pain in the butt. – IdeaHat Jun 03 '14 at 14:33
  • No . He does not state that in what way he gains access to that array . Maybe it is just a reference from another part of the program , which would making option 0 (if it's the correct thing to do) the more efficient way . as you can see , my answer is not mainly about which way is the more efficient , but about how each of them needs to be done correctly – ColonelMo Jun 03 '14 at 14:35