Pointer being freed was not allocated with realloc and malloc in construct function

Question

I tried dataPoolBuffer = realloc(dataPoolBuffer, sizeof(char)*(dataPoolSize)); already, but Xcode reports:assigning to 'char *' from imcompatible type 'void'.

I create a class:

class solutionBuffer{ 
private:


char * dataPoolBuffer;
char * consumerBuffer;
char * flagBuffer;
int dataPoolSize;
int consumerBufferSize;
mutex safe;

public:
solutionBuffer(){
    safe.lock();
    dataPoolSize = 0;
    consumerBufferSize = 0;
    
    dataPoolBuffer = (char*)malloc(sizeof(char)*1);
    consumerBuffer = (char*)malloc(sizeof(char)*1);
    flagBuffer = (char*)malloc(sizeof(char)*1);
    
}
int add(char* data, int length)
{
   
    dataPoolSize += length;
    
    realloc(dataPoolBuffer, sizeof(char)*(dataPoolSize));
    
    realloc(flagBuffer, sizeof(char)*(dataPoolSize));
    
    memcpy(dataPoolBuffer + dataPoolSize - length, data, sizeof(char)*(length));
    
    return 0;
}

~solutionBuffer(){
    printf("%d",strlen(dataPoolBuffer));
    free(dataPoolBuffer);
    free(consumerBuffer);
    free(flagBuffer);
    safe.unlock();
}

};

Every time when we call .add function, it will realloc memory for the variable. However, when I do that in main():

char data[] = "0123456789";
char data2[] = "01234567890123456789";
solutionBuffer buffer;
buffer.add(data, 10);
buffer.add(data2, 20);

The xoce shows:pointer being freed was not allocated in ~solutionBuffer() when it was trying to free dataPoolBuffer . Why it does like that? How to fix that ?

There's so many things that are wrong here, I don't even know where to start: `malloc` with a size of 0; using `malloc` and `free` in C++ code; needless dynamic allocation in the first place. The list just goes on, and on, and on... — Sam Varshavchik, Jul 16 '16 at 03:35
Even I revise malloc it with size of 1, it still shows the error msg. C++ is compatible with C style. Why cannot use that in C++ — beasone, Jul 16 '16 at 03:41
I understand the reason for using `realloc`. But I want to know what possible reason you think there is for latching a mutex (incorrectly) in a class' constructor, where said-same modifies only instance-member variables. Are you expecting multiple threads to be constructing the same object simultaneously? Or that you ignore said-mutex in a method I would expect it to be latched (but not how you're doing it) such as your `add` method. If it isn't needed to repro your findings, get rid of it in your post. — WhozCraig, Jul 16 '16 at 03:44
And regarding your actual problem, what do you think the *result* of `realloc` is *for* ? I think you should review what it returns, and why ignoring it is a *bad* idea. Read how [**`realloc`**](http://en.cppreference.com/w/cpp/memory/c/realloc) works, because you're using it *wrong*. — WhozCraig, Jul 16 '16 at 03:46
Just because it's possible to invoke malloc/realloc/free from C++ doesn't mean that it's a good idea to do so. — Sam Varshavchik, Jul 16 '16 at 03:46
@beasone I highly suggest you learn proper C++ programming before you jump to multithreaded program. — PaulMcKenzie, Jul 16 '16 at 03:48
I want to make a class, the function inside the class is thread safe, so I use mutex to lock it. Each time when someone call .add to add more data into the dataPoolBuffer, it will realloc more memory to contain the new data. — beasone, Jul 16 '16 at 03:48
@beasone Why not simply use `std::vector` instead of `char *`, and `vector::resize()` instead of those `realloc` calls? — PaulMcKenzie, Jul 16 '16 at 03:50
@WhozCraig Are you expecting multiple threads to be constructing the same object simultaneously? Yes. Or I shoud say, I need to make sure all these functions are thread safe. — beasone, Jul 16 '16 at 04:04
@beasone a mutex on a constructor as you have it isn't the way to do that. You're not updating some external data; it's all member stuff. And seriously, how do you expect two threads to be *constructing* the **same** object *simultaneously* ? "Thread-safe" doesn't mean hang mutexes on stuff and latch them, particularly how you're doing it, as you needlessly latch a mutex, and its never unlatched again. — WhozCraig, Jul 16 '16 at 04:07
@PaulMcKenzie .. and then throw out `dataPoolSize`, since the vector already has a member that reports it =P — WhozCraig, Jul 16 '16 at 04:09
@beasone [Updated example](http://ideone.com/0ZJXnv). See how much simpler and saner that looks, compared to your code? — PaulMcKenzie, Jul 16 '16 at 04:11
@PaulMcKenzie Thanks for your code. Actually I need three functions inside this class, I didn't post all of them. the add function will never block the process or thread, another function will be blocked if the dataPoolBuffer is empty and it will be released when dataPoolBuffer is filled with new data. — beasone, Jul 16 '16 at 04:13
@bassone - As I stated, start with that example. As for checking for an empty buffer -- `if ( dataPoolBuffer.empty()) { }` -- can anything be more simpler than that? — PaulMcKenzie, Jul 16 '16 at 04:15
@PaulMcKenzie Yes, I can do that. How to make each function thread safe? — beasone, Jul 16 '16 at 04:20
That's a majorly destructive edit of your post after the question has been answered. — kfsone, Jul 16 '16 at 06:43

Barmar · Answer 1 · 2016-07-16T04:00:37.823

1

When you call realloc(), you need to assign the result back to the pointer variable. realloc() often needs to move the memory to a new location, and it returns that location. Your code leaves the variable pointing to the old location, and you get undefined behavior when you try to use it after that.

So it should be:

dataPoolBuffer = (char*)realloc(dataPoolBuffer, sizeof(char)*(dataPoolSize));

flagBuffer = (char*)realloc(flagBuffer, sizeof(char)*(dataPoolSize));

edited Jul 16 '16 at 04:00

answered Jul 16 '16 at 03:51

Barmar

741,623
53
500
612

I tried "ataPoolBuffer = realloc(dataPoolBuffer, sizeof(char)*(dataPoolSize));" already, but Xcode reports:assigning to 'char *' from imcompatible type 'void'. – beasone Jul 16 '16 at 03:54
Forgot that C++ requires you to use an explicit cast there. – Barmar Jul 16 '16 at 04:00

score 1 · Accepted Answer · edited May 23 '17 at 12:08

1

According to the documentation,

realloc(dataPoolBuffer, sizeof(char)*(dataPoolSize));

reallocates dataPoolBuffer, but doesn't change where dataPoolBuffer points. So odds are pretty good that dataPoolBuffer is now pointing to invalid memory.

dataPoolBuffer = (char*)realloc(dataPoolBuffer, sizeof(char)*(dataPoolSize));

will do you what you want, but rethink how you are doing this. You're setting yourself up for a lot of pain. Your class violates The Rule of Three, for one thing. std::vector will handle all of container resizing and memory management for you with no muss and no fuss.

edited May 23 '17 at 12:08

Community

1
1

answered Jul 16 '16 at 03:51

user4581301

33,082
7
33
54

I tried "ataPoolBuffer = realloc(dataPoolBuffer, sizeof(char)*(dataPoolSize));" already, but Xcode reports:assigning to 'char *' from imcompatible type 'void'. – beasone Jul 16 '16 at 03:52
@beasone Good point. I fell back on C where this gag is legal. Needs an extra cast. Fixing. – user4581301 Jul 16 '16 at 03:54

Pointer being freed was not allocated with realloc and malloc in construct function

2 Answers2