Error using realloc inside function

This is a discussion on Error using realloc inside function within the C Programming forums, part of the General Programming Boards category; Happy holidays C gurus! I am running into a strange error using realloc to resize a 2D array within a ...

Error using realloc inside function

Happy holidays C gurus! I am running into a strange error using realloc to resize a 2D array within a function and could use some advice.

The function is designed to remove duplicate entries from a dynamically allocated 2D array of integers with 3 columns. The function is used inside a for loop and gives the illusion of working for the first 4 repeats. On the fifth repeat, malloc returns a NULL value at the point indicated in the code below. The error occurs at exactly the same point every time I run the simulation on a LINUX machine. However, I get no error messages running the same code on a Windows machine.

I know it is not a particularly elegant function, but are there any obvious memory leaks or is it something to do with the machine itself?

It would help if you posted a small, complete, and COMPILABLE sample of code that (when compiled, linked, and run) illustrates your problem, rather than copying and pasting snippets at random that you think are relevant.

As is, your code has a function implemented inside a function, which is illegal in C. You have not also provided any code which calls your functions, so nobody has a hope of recreating your symptoms. A main() function is important as you claim to have a runtime problem.

All I can do is guess.

My guess is that rewired_edges passed from the caller is not initialised before the first call. There is also a memory leak as, when reallocating rewired_edges, your code loses track of the elements of rewired_edges (i.e. memory leak). That does not typically cause a failure of malloc() directly - unless it causes memory exhaustion.

If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

You never check the return value of malloc/realloc, you just assume it works. That could result in seg faults should malloc/realloc fail and you try to access temp_edges[x] or some such.

You should

You have a memory leak in your realloc usage. You need a teporary variable to hold the return of realloc. If you do foo = realloc(foo, size), and realloc fails, then you lost the pointer to the original piece of memory. Not only did you leak memory, you lost the data you still need. See below for the correct idiom.

You should #define COLS 3, or some such, and use that instead of using magic numbers and manipulating array elements individually (arrays almost always should be used in conjunction with loops). For example, lines 28-30, 47-49, 65-67 and probably more, could be done in a loop. Better yet, you could create a function called something like copy_edge, that does all the work, and call that in place of those 3 individual assignments. It makes your code more flexible, should you decide to handle 4-column data, and even if you know you wont need 4 columns, it's good design and a good habit to get into.

This is a much simplified version of the way the function is currently called in the program, but compilable.

What you have posted is not compilable as C. It has a function implementation inside the body of another function. Either you are using another language that looks sort-of like C but isn't, or you are using a non-standard compiler extension that accepts code like that.

Originally Posted by NotAProgrammer

I am a little bit confused by:
"There is also a memory leak as, when reallocating rewired_edges, your code loses track of the elements of rewired_edges (i.e. memory leak)."

I thought realloc automatically released the memory when it resizes the array.

It does. However, if realloc() fails, it returns NULL and the original memory is not released. The caller needs to cope when that happens. Your code is not even testing for that occurrence, let alone recovering from it.

When you have memory leaks, which your code does (multiple malloc() calls with no corresponding realloc() or free()), such an occurrence is significantly more likely.

You might want to check the return value from malloc() inside main(). malloc(150000*sizeof(int *)) is not guaranteed to succeed. If you are using a 16-bit compiler, it WILL fail. Your code is not checking for that occurrence. Even worse, you're doing that in a loop, with other code that has several memory leaks.

You have also not corrected several of the concerns pointed out by anduril.

If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

I gave up trying to get realloc to work in the for loop without leaking memory and instead re-wrote the code using linked lists to temporarily store the new array elements. At the end of the for loop, I just malloc the 2D array once using the number of elements stored in the linked list, populate the array with the values stored in the structs, and then delete the linked list. Works like a charm and according to Valgrind, I am no longer leaking memory.

I am using GCC through Cygwin as my compiler, which does allow nested functions.

I am using GCC through Cygwin as my compiler, which does allow nested functions.

In your example, your comp_rewired function does not need to be nested. Why not just define it as a normal function? Make it a static function if your goal is to limit access to the function or to avoid name conflicts. Static functions are only callable from other functions in the same compilation unit. This standard technique negates the need for GCC nested functions.