Thread: Destructor Issue

  1. #1
    Registered User verbity's Avatar
    Join Date
    Nov 2006
    Posts
    101

    Destructor Issue

    I'm having problems with a destructor. It says my heap is corrupted and after debugging I'm assuming that this is the problem. It is after a certain function so here's that function:

    Code:
    #include "stdafx.h"
    #include <string>
    #include <iostream>
    #include <fstream>
    #include <stdlib.h>
    #include "string.h"
    
    #define NULL "\0"
    
    
    extern std::ofstream csis;
    
    //Default Ctor
    
    String::String()
    {
    	m_buf = new char[1];
    	m_length = 1;
    	m_buf[0] = '\0';
    	
    	m_name = "";
    }
    
    //Ctor with string literal param
    String::String(char* s)
    {
    	int inputString = strlen(s);
    	inputString++;
    
    	m_length = inputString;
    
    	m_buf = new char[inputString];
    
    	strcpy_s(m_buf, m_length, s);
    	
    	m_name = "";
    }
    
    //Ctor with single char as param
    String::String(char c)
    {
    	m_buf = new char[2];
    
    	m_buf[0] = c;
    	m_buf[1] = '\0';
    
    	int inputString = strlen(m_buf);
    
    	m_length = inputString;
    	
    	m_name = "";
    }
    
    String::String(int x)
    {
    	m_buf = new char[sizeof(x)];
    
    	_itoa(x, m_buf, 10);
    
    	m_length = strlen(m_buf);
    
    	m_name = "";
    }
    
    //Ctor with pointer to data member
    String::String(const String& s)
    {
    	m_length = s.m_length;
    	m_length;
    	m_buf = new char[m_length];
    
    	strcpy_s(m_buf, m_length, s.m_buf);
    
    	m_name = "";
    }
    
    
    //Ctor with repeating char
    String::String(char c, int x)
    {
    	m_buf = new char[x];
    	
    	for(int i =0; i < x; i++)
    	{
    		m_buf[i] = c;
    	}
    
    	m_buf[x] = '\0';
    
    	m_length = strlen(m_buf);
    
    	m_name = "";
    }
    
    //Destructor
    String::~String()
    {
    	if(m_name)
    		delete[]m_name;
    	if(m_buf)
    		delete[]m_buf;
    
    }
    ......
    The error occurs after the last constructor function and at delet[]m_buf in the destructor. The actual error is: Heap corruption detected.....detected that the program wrote to the memory after the end of heap buffer....

    Any thoughts please and thank you?

  2. #2
    Registered User Noir's Avatar
    Join Date
    Mar 2007
    Posts
    218
    This is probably it:
    Code:
    m_buf = new char[x];
    	
    // loop through m_buf
    
    m_buf[x] = '\0';
    m_buf has x characters, but x isn't a safe index. It's one past the memory you asked for.

  3. #3
    Registered User
    Join Date
    Aug 2005
    Location
    Austria
    Posts
    1,990
    that one is wrong
    Code:
    String::String(int x) {
    	m_buf = new char[sizeof(x)];
    	_itoa(x, m_buf, 10);
    	m_length = strlen(m_buf);
    	m_name = "";
    }
    sizeof(x) doesn't tell you how many chars are needed to represent an int as string.
    that one as well
    Code:
    String::String(char c, int x) {
    	m_buf = new char[x];
    	for(int i =0; i < x; i++) {
    		m_buf[i] = c;
    	}
    	m_buf[x] = '\0';
    	m_length = strlen(m_buf);
    	m_name = "";
    }
    you allocate only x chars but then you assigne '\0' to the n+1 char.

    same thing with the copy constructor. the buffer is too small.

    also you have to implement an assignement operator. the compiler generated one will create memory-leaks and double deletes in the destructor.
    Kurt

  4. #4
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,659
    > m_name = "";
    Well you can't delete this.

    You're better off saying
    m_name = NULL;

    > if(m_name)
    You don't need this either, because deleting NULL is safe.

    > int inputString = strlen(s);
    I find inputString to be a very confusing variable name, given its context.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  5. #5
    Registered User verbity's Avatar
    Join Date
    Nov 2006
    Posts
    101
    I fixed the x char issue..... I debugged again and at my assignment operator is where i"m having the problem I guess. I have something assigns a string to two others to String s7, t7, u7....then String s7("whatever")....then s7 = t7 = u7; Then when my destructor is called at m_buf it says there is writing past the heap buffer. Here's that assignment operator and my destructor again:

    Code:
    String& String::operator=(const String& rhs)
    {
    	m_length = rhs.m_length;
    
    	strcpy_s(m_buf, m_length, rhs.m_buf);
    	
    	return *this;
    }
    
    //Destructor
    String::~String()
    {
    	delete[]m_buf;
    	delete[]m_name;
    
    }

  6. #6
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,659
    And what about all your constructors?
    Are they still assigning bogus pointers which can't be deleted?
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  7. #7
    Registered User
    Join Date
    Aug 2005
    Location
    Austria
    Posts
    1,990
    try something like this
    Code:
    String& String::operator=(const String& rhs) {
            // we don't want self assignement
            if ( this != &rhs ) {
                // free old buffer
                delete [] m_buf;
    	    m_length = rhs.m_length;
                // need space for '\0'
    	    m_buf = new char[m_length + 1];
                // don't know that one. looks strange
    	    strcpy_s(m_buf, m_length, rhs.m_buf);
      	   // what about m_name ?
    	}
    	return *this;
    }
    Kurt

Popular pages Recent additions subscribe to a feed