0

I am having errors in string assignment here. This is a function we coded for storing data fetched from a url. edit : datanode structure

  struct node
  {
    string url;
    std::string* data;
    struct node* next;
    struct node* prev;
  };


  void RandomCache::cachePage(string* page_data, string url)
  {
    datanode *page_node= (datanode*)malloc(sizeof(datanode));
    page_node->url = url;
    page_node->data = page_data;
    page_node->next=NULL;
    page_node->prev=NULL;

    insertNode(page_node);

  }

the line page_node->url = url is causing Double free or corruption, the data is passed by value, and not by reference. could anybody point out whats going wrong?

Thank you, I made the structure into a class, and the problem is completely resoluctved. Thanks! but I am still wondering about this, as to why structure memory management with malloc is problamatic in C++. structures are used very frequently in c++.

sai pallavi
  • 147
  • 3
  • 10
  • for C++ use `new` instead of `malloc` – brokenfoot Mar 26 '14 at 05:02
  • why `malloc` in C++.... – Bryan Chen Mar 26 '14 at 05:03
  • may be the `malloc` isn't successful, should put a check there for the pointer `page_node` returned. – brokenfoot Mar 26 '14 at 05:04
  • 1
    You need to show more of your code for others to diagnose the problem. Show the definition of `datanode`. – R Sahu Mar 26 '14 at 05:05
  • `data` should most likely not be a pointer. – chris Mar 26 '14 at 05:08
  • could be that the datanode is using `free` on its insides on the `string`. This would cause `undefined behavior` – Claudiordgz Mar 26 '14 at 05:13
  • 3
    `datanode *page_node = new page_node;` You're object's `url` member is not initialized (constructed) correctly because you forgot you're programming in C++. For your dynamic object's *members* (which are also objects, in this case at least one `std::string`) to properly construct, you need to use `operator new` in one form or another. A general (not always the case; *general*) rule: Look at your C++ program. Are you using `malloc()`? If the answer is yes, you're probably doing something wrong. – WhozCraig Mar 26 '14 at 05:18

3 Answers3

1

Seeing as you are using malloc instead of new, the constructor for datanode will not be called, and hence the std::string at page_node-> url won't have it's constructor called.. Consequently, page_node->url will be messed up when its equals operator is called.

Specifically, internally, the std::string class includes a pointer to a chunk of memory that actually contains the string bytes. When a string assignment is performed, the overridden equals operator for string is called. It's first job is to deallocate the existing memory for the old string the object should have contained. If it were constructed properly, it would contain nothing, but here it will just contain random data, so it goes and deallocates some random piece of memory.

The solution is that if you want to store strings (or any C++ class with a constructor) inside the object, construct the object fully and use new.

Try this:

datanode *page_node= new datanode();

And replace your free() with delete .

On the other hand page_data has different issues. As you are passing a pointer to a string that is allocated elsewhere, you need to be careful of the memory management of this, that the original string doesn't go out of scope or destroyed while this pointer may be used.

sj0h
  • 902
  • 4
  • 4
1

Double free or corruption error with string assignment

If you are getting either of these error, it is most likely that your program is encountering heap memory corruption.

In general heap corruption is often detected after the real corruption has already occurred by some DLL/module loaded within your process.Hence it is quite possible that some other code is doing something wrong and the above code is just an victim. Hence I would recommend you to use some dynamic tool to understand the error quickly and and at the point where problem is happening.From your description it is also possible that your program has some sort of memory corruption.

I think my previous post might be useful for this problem as well. If your program is windows specific(WinDBG/PageHeap), you should see the following link:

https://stackoverflow.com/a/22074401/2724703

If your program is Gnu/Linux specific(Valgrind), you should see the following link:

https://stackoverflow.com/a/22085874/2724703

Community
  • 1
  • 1
Mantosh Kumar
  • 5,659
  • 3
  • 24
  • 48
  • Valid answer, but the most likely problem here is that his object datanode isn't constructed correctly because he used malloc instead of using new (or placement new). – DigitalEye Mar 26 '14 at 05:29
1

The code you've shown is perfectly correct C (and, consequently, correct C++).

Q: Where is your tree's root stored? As a member variable in class "RandomCache" that's visible to both "cachePage()" and "insertNode()"? A global variable?

SUGGESTIONS:

1) make sure the datanode pointer you malloc() space for in your "cachePage()" function is the same pointer you actually free. From the code you've shown, it's possible that "datanode *page_node" has NO visibility outside of "cachePage()" - and when you ultimately "free()" the pointer, you're actually freeing random, uninitialized memory.

2) Valgrind could be very useful here. Try it!

3) Always set your pointer to NULL after you've freed it.

FoggyDay
  • 11,962
  • 4
  • 34
  • 48
  • Assuming that datanode::url is a C++ std::string, this code is not correct C, because you can't have a std::string in C. This is important, as C++ objects such as std::string need their constructors to be called before other functions are called on them. – sj0h Mar 26 '14 at 05:33
  • @sj0h - you're absolutely correct. Q: What about "url" as a reference: `void RandomCache::cachePage(string* page_data, string &url)`? – FoggyDay Mar 26 '14 at 07:05
  • Hi, more or less similar code is working in another program I've written, and I was wondering what could be going wrong in this case. – sai pallavi Mar 26 '14 at 12:55