4

I'm trying to free the memory of the allocated array inside struct _Stack, but the program keeps crashing

typedef struct _Stack
{
    int top;
    unsigned int capacity;
    int* arr;
}_Stack;

_Stack* createStack(int capacity)
{
    _Stack* stack = (_Stack*) malloc(sizeof(_Stack));
    stack->capacity = capacity;
    stack->top = -1;
    stack->arr = (int*) malloc(sizeof(stack->capacity * sizeof(int)));
    return stack;
}

I'm using this function to free the memory, but the program crashes here.

// I have a problem here.
void stack_free(_Stack* stack)
{
    free(stack->arr);
    free(stack);
}

Here's the error message

cbuchart
  • 10,847
  • 9
  • 53
  • 93
xSuperMu
  • 374
  • 2
  • 5
  • 16
  • 5
    If you program in C++ then why are you using `malloc` and `free`? If you have to use pointers as a requirement use first of all a *smart pointer* like e.g. [`std::unique_ptr`](http://en.cppreference.com/w/cpp/memory/unique_ptr), and you should definitely use `new[]` and `delete[]` instead of `malloc` and `free`. However, if pointers are not a requirement then you should be using [`std::vector`](http://en.cppreference.com/w/cpp/container/vector) instead. – Some programmer dude Oct 11 '17 at 06:17
  • 1
    Show us the entire code that causes this and can be run (MCVE). Also, do not use underscore-capital in names (reserved). – lorro Oct 11 '17 at 06:18
  • 2
    I also recommend you take some time to read [What are the rules about using an underscore in a C++ identifier?](http://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier) Symbols beginning with an underscore followed by an uppercase letter (like e.g. `_Stack`) are reserved. – Some programmer dude Oct 11 '17 at 06:18
  • 1
    are you positive that every call to `stack_free` is a pointer created with `createStack`? – kmdreko Oct 11 '17 at 06:19
  • 1
    You can std::stack to handle the memory for you. http://en.cppreference.com/w/cpp/container/stack – RichS Oct 11 '17 at 06:20
  • 2
    As for your problem, please try to create a [Minimal, Complete, and Verifiable Example](http://stackoverflow.com/help/mcve) and show us. Also please [read about how to ask good questions](http://stackoverflow.com/help/how-to-ask). Lastly, please read [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) by Eric Lippert, and learn how to use a debugger (including memory debuggers like e.g. [Valgrind](http://valgrind.org/) or similar). – Some programmer dude Oct 11 '17 at 06:21

2 Answers2

3

Change this:

stack->arr = (int*) malloc(sizeof(stack->capacity * sizeof(int)));

to this:

stack->arr = (int*) malloc(stack->capacity * sizeof(int));

since you want the size of the array to be equal to stack->capacity * sizeof(int), and not equal to the size of that expression.

Your program must have invoked Undefined Behavior somewhere in code not shown in the question (because of the wrong size malloc'ed), that's why it crashes later.


PS: Since you use C++, consider using new instead (and delete, instead of free()).

gsamaras
  • 71,951
  • 46
  • 188
  • 305
2

sizeof(stack->capacity * sizeof(int)) in your call to malloc is wrong. Instead of the size of the array, it gives the size of the number used to represent the size of the array. You probably want stack->capacity * sizeof(int).

Another possible problem is that in C you should not cast the return value of malloc, since it can hide other mistakes and cause a crash. See Do I cast the result of malloc? In C++ you will have to do it, because of the stricter type checking in C++, but it can still hide problems.

These are the problems I see with the code you have shown. However, remember that errors in malloc and free are not necessarily caused by the actual line where they are detected. If some part of your program damages the malloc system's internal data structures, for example by a buffer overrun, the problem can manifest in a much later call to malloc or free, in a completely different part of the program.

Thomas Padron-McCarthy
  • 27,232
  • 8
  • 51
  • 75