Heap corruption detected while freeing memory
CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Results 1 to 9 of 9

Thread: Heap corruption detected while freeing memory

  1. #1
    Join Date
    Jun 2010
    Posts
    50

    Question Heap corruption detected while freeing memory

    I have following code in which I am getting "Heap corruption detected" message while trying to freeing memory allocated while using strdup().
    Code:
    #include <iostream>
    #include <string>
    #include "Check.h"
    
    
    static void main()
    {
    
    	
    for (int i = 0;i< 5 ;i++)
    {
    char cname[32];
    cname = GetGroupName()//These function assigns name here
    bool exist;
    	exist =	GetGroupStatus(cname);
    }
    
    }
    
    bool GetGroupStatus(char *cname)
    {
    	char * cTemp = NULL;
    	cTemp = _strdup(cname);
    	bool bGrpStatus; 
    	size_t found;
    
    	//some code here
    
    	std::string sTemp = cTemp;
    	
    	found = sTemp.find("GROUP");
    
    	if (found != string::npos)
    		bGrpStatus = true;
    	else
    		bGrpStatus = false;
    
    	if (cTemp  != NULL)
    	free(cTemp);	//These causes heap corruption detected message when control comes here second time
    
    	return bGrpStatus; 
    }
    Please let me know what I am missing?
    Last edited by cilu; January 10th, 2011 at 03:17 AM. Reason: code tags

  2. #2
    Join Date
    Aug 2008
    Location
    Scotland
    Posts
    379

    Re: Heap corruption detected while freeing memory

    Hi,

    Can you show the content and prototype for GetGroupName()?

    Alan

  3. #3
    Join Date
    Oct 2009
    Posts
    577

    Smile Re: Heap corruption detected while freeing memory

    Please put your code between code tags [ c o d e ] ... [ / c o d e ].

    free(cTemp); //These causes heap corruption detected message when control comes here second time
    I can't see from your code how control could come twice to that point. But if you would set the pointer to NULL after freeing (every time) even recursive callings would be safe.

  4. #4
    Join Date
    Jun 2010
    Posts
    50

    Re: Heap corruption detected while freeing memory

    GetGroupStatus() is called in for loop from Main.So when i = 0 it works fine , after that when I tries to free the memory throws an error message.Would it be good practise to set again to NULL after freeing.

    if (cTemp != NULL)
    free(cTemp);


    *cTemp = NULL

  5. #5
    Join Date
    Jan 2006
    Location
    Singapore
    Posts
    6,250

    Re: Heap corruption detected while freeing memory

    That check for a null pointer is useless because calling free on a null pointer is already safe. The null pointer check that you should be performing is right after the _strdup call. Setting the pointer to be a null pointer after the free is also not very useful because the only line after that is the return statement.

    I suspect that the problem lies in the code that you did not show us, e.g., the "//some code here" part. Why do you need to use _strdup when you are able to use std::string?
    C + C++ Compiler: MinGW port of GCC
    Build + Version Control System: SCons + Bazaar

    Look up a C/C++ Reference and learn How To Ask Questions The Smart Way
    Kindly rate my posts if you found them useful

  6. #6
    Join Date
    Jun 2010
    Posts
    50

    Re: Heap corruption detected while freeing memory

    Code at that place will function something like this -
    //first check is added depending on one of the runtime value
    //as it is true cTemp will be assigned with new value


    I have restriction of using char * on that places,wherever possible I am using std::string.

  7. #7
    Join Date
    Oct 2009
    Posts
    577

    Smile Re: Heap corruption detected while freeing memory

    Quote Originally Posted by forstudy3 View Post
    GetGroupStatus() is called in for loop from Main.So when i = 0 it works fine , after that when I tries to free the memory throws an error message.Would it be good practise to set again to NULL after freeing.

    if (cTemp != NULL)
    free(cTemp);


    *cTemp = NULL
    It should be cTemp = NULL; cause cTemp is the pointer and not *cTemp.

    Setting to NULL after free won't solve the problem if the cTemp is a local variable and you would return after that. But it is not useless cause the old pointer value isn't valid after free, hence it is less error-prone for future enhancements to initialize the invalid pointer variable again.

    If the cTemp is a shared variable (either member variable or global variable) it is essential that you initialize it to NULL after free.

    Looking at the code you posted I would assume that you already freed the pointer somewhere else. You can avoid such mistakes by always freeing pointers on the same calling level where you made the malloc or strdup. So if you passed a pointer to a function never free it in the function called but always in the calling function.

  8. #8
    Join Date
    Jan 2006
    Location
    Singapore
    Posts
    6,250

    Re: Heap corruption detected while freeing memory

    Quote Originally Posted by forstudy3
    Code at that place will function something like this -
    //first check is added depending on one of the runtime value
    //as it is true cTemp will be assigned with new value
    Yes, that has the potential to contain a problem: you are messing around with cTemp there, yet you did not show the code. itsmeandnobodyelse's suspicion that "you already freed the pointer somewhere else" may well prove valid, even though it is all on the same calling level.

    EDIT:

    Quote Originally Posted by forstudy3
    I have restriction of using char * on that places,wherever possible I am using std::string.
    If feasible, you could use a std::vector<char> instead. You could also use a std::string and then access &cTemp[0] for a char*, but there's a slight risk that until the next version of the C++ standard comes out it might not be correct.

    By the way, the global main function should not be declared static.
    Last edited by laserlight; January 10th, 2011 at 04:26 AM.
    C + C++ Compiler: MinGW port of GCC
    Build + Version Control System: SCons + Bazaar

    Look up a C/C++ Reference and learn How To Ask Questions The Smart Way
    Kindly rate my posts if you found them useful

  9. #9
    Join Date
    Apr 1999
    Posts
    27,423

    Re: Heap corruption detected while freeing memory

    Quote Originally Posted by forstudy3 View Post
    I have restriction of using char * on that places,wherever possible I am using std::string.
    First, your program has a lot of issues:
    Code:
    static void main()
    Is this your invention? The main() function is not static, and returns an int, not void.

    Second, your GetGroupStatus function does not alter cname in any way. Therefore that parameter should be const char*, not char*.

    Once it's const char*, then you don't need to use char* within that function at all.
    Code:
    bool GetGroupStatus(const char *cname)
    {
        std::string sTemp = cname;
        bool bGrpStatus; 
        size_t found;
    
        //some code here (what is this?????)
    
        found = sTemp.find("GROUP");
    
        if (found != string::npos)
    	bGrpStatus = true;
        else
    	bGrpStatus = false;
    
    	return bGrpStatus; 
    }
    There is no need for strdup() or free().

    For practically all cases, there is no need to introduce 'C' functions, char pointers and dynamic memory when handling strings in C++. Show us the case in your code where you must do this -- chances are, you don't need to do anything of the sort.

    Regards,

    Paul McKenzie

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  


Azure Activities Information Page

Windows Mobile Development Center


Click Here to Expand Forum to Full Width

This is a CodeGuru survey question.


Featured


HTML5 Development Center