2
votes

I have this function to read a triangular 2d array, but sometimes it crashes on realloc. Always on the 6th realloc (current_row = 7). Sometimes it runs fine. Cannot reproduce the error in gdb (works every time). What's wrong?

TRIANGLE *read_triangle(char *file_name)
{
std::ifstream fin(file_name);
int current_row = 0, current_column = 0, buffer;
TRIANGLE *triangle = new TRIANGLE();
triangle->triangle_values[0] = new int[1];

while (fin >> buffer)
{
    if (current_column == current_row+1)
    {
        current_column = 0;
        triangle->triangle_values = (int**)realloc(&((void*)triangle->triangle_values), (++current_row+1)*sizeof(int*));
        triangle->triangle_values[current_row] = new int[current_row];
    }
    triangle->triangle_values[current_row][current_column++] = buffer;
}
triangle->rows = current_row-1;
return triangle;
}

The TRIANGLE Definition

struct TRIANGLE 
{ 
    int **triangle_values; 
    int rows; 
    TRIANGLE(): triangle_values(NULL) 
    {
        triangle_values = new int*[1];
    } 
};

Input file example:

75
95 64
17 47 82
18 35 87 10
20 04 82 47 65
19 01 23 75 03 34
88 02 77 73 07 63 67
99 65 04 28 06 16 70 92
41 41 26 56 83 40 80 70 33
41 48 72 33 47 32 37 16 94 29
53 71 44 65 25 43 91 52 97 51 14
70 11 33 28 77 73 17 78 39 68 17 57
91 71 52 38 17 14 91 43 58 50 27 29 48
63 66 04 68 89 53 67 30 73 16 69 87 40 31
04 62 98 27 23 09 70 98 73 93 38 53 60 04 23
1
Don't use manual memory management. Use std::vector<int> which also has push_back method and resize method to increase in size as required. - Neil Kirk
Have to agree with Neil here, real C++ programmers don't generally use the legacy C stuff when C++ provides a better way. It's there if you need to use C code in C++ but otherwise should be avoided. And mixing new/realloc is asking for trouble :-) - paxdiablo
Don't mix realloc and new like that. - Dave Rager
I see your point. It did not occur to me that mixing them could cause problems. - user2524502
Not that it matters at this point, but could add the input file format to your question? Ideally with sample data. (post at the bottom of the question as an addendum). I've a suspicion of just how much this code can collapse, especially if you apply the comments thus far. - WhozCraig

1 Answers

3
votes

You're mixing the old style (realloc) and new style (new) memory allocation functions which is not guaranteed to work. By that, I mean they'll work provided you keep them separate but allocating memory with new and then trying to expand that same memory with realloc is a definite no-no.

From C++11 20.6.13, when talking about how the old style functions control the reachability of their blocks:

It also allows malloc() to be implemented with a separate allocation arena.

Hence, there's no guarantee that the memory arenas for old and new style are related to each other at all.

C++ provides all sort of wonderful collection classes with far better resizing methods than malloc/realloc. You should fully embrace the language by using them (such as vector).

Generally, C++ programmers don't use the legacy C stuff unless they're writing things that need to be usable in both C and C++, in which case they're probably better referred to as C programmers, at least temporarily :-)