Error with free function after malloc

Question

The code below results in a crash of C++ on the line: free(arg). I am trying to prevent a memory leak from happening but i can't manage to free the data i stored in heap memory. Can someone help me with this problem?

Note that free(args) works fine.

#include "stdafx.h"
#include <process.h>
#include <iostream>

#include <windows.h>

using namespace std;

typedef struct {
    int StartNode;
    int EndNode;
}t;
t *arg;

void myFunc(void *param) {
    t *args = (t*)param;
    int x = args->StartNode;
    int y = args->EndNode;
    printf("x=%d, y=%d\n", x, y);
    free(args);
    free(arg);
}

int main()
{
    HANDLE handle;
    arg = (t *)malloc(sizeof(t));
    arg->StartNode = 101;
    arg->EndNode = 103;
    handle = (HANDLE)_beginthread(myFunc, 0, (void*)arg);
    cin.get();
    return 0;
}

Looks to me like `args` and `arg` point to the same allocation. You're freeing it twice. — user4581301, Mar 28 '18 at 16:55
By the way, can I talk you into using `new`? A Smart pointer like `std::unique_ptr` should be appropriate here as well. — user4581301, Mar 28 '18 at 16:57
It seems you are totally right, when i comment the line: free(args), the free(arg) works fine. I'll look into your suggestion, i am a bit new to pointers so i'll have to try it out a little more. Thanks for your help :) — Jop Hoeijmakers, Mar 28 '18 at 17:00
As you pass in the parameter as function argument anyway, there is no need at all for the global variable... — Aconcagua, Mar 28 '18 at 17:24

score 0 · Answer 1 · answered Nov 30 '20 at 19:53

0

Both args and arg are pointing to same memory location. Calling free on any one should be enough.

answered Nov 30 '20 at 19:53

Yashwanth Yash

1
1

Abhijit Pritam Dutta · Accepted Answer · 2018-03-29T09:05:09.313

Your both pointers respectively args and arg are pointing to the same memory location and you are trying to release same memory location twice, and that creating the issue here. Please see below:-

 free(args); //args->arg here args is pointing to arg you have just type cast it from void
 free(arg);//you have already release the memory in the above call so this is wrong

Just try like this for understanding, below example is not a solution but for your understanding. Here you assign args = NULL and that will be reflected in arg = NULL hence if(arg != NULL) will be false and hence free(arg); will not be call.:-

free(args); 
args = NULL;
if(arg != NULL)
  free(arg);

score -2 · Answer 3 · answered Mar 28 '18 at 17:06

The number of free calls needs to be the same as malloc. you malloc the conde only once, in

arg = (t *)malloc(sizeof(t));

but you free the same address twice:

free(args);
free(arg);

Now, this is C code, not C++ (as C++ you would use new / delete, or even better, you would not use nor new nor delete, and pass the variables via reference in the stack like this:

#include <iostream>
#include <windows.h>

struct MyType {
    int StartNode;
    int EndNode;
};

void myFunc(const MyType &param) {
    const auto x = args.StartNode;
    const auto y = args.EndNode;
    std::cout << "x=" << x << ", y=" << std::endl;
}

int main()
{
    auto arg = MyType{};
    arg.StartNode = 101;
    arg.EndNode = 103;
    std::thread thread(myFunc, arg);
    thread.join();
    cin.get();
    return 0;
}

Some random notes:

you are mixing C with C++, they are not the same language
you are using windows-only calls, use std (like in the thread example)
don't use using namespace std; this makes code unreadable in no time.

Error with free function after malloc

3 Answers3