issues with freeing memory properly

Question

Hi I am trying to free memory for this character pointer but because it is initialized as name I can't delete/free it. How can i handle this situation? My header file and implementation files look like below.

/// Header file.

#ifndef __DAY_H__
#define __DAY_H__


class day {
    private:
        char* name;
        int   nClasses;
        bool  status; //working day or a holiday

    public:
        day();
        day( char * name, int place, bool status);
        day( const day& );
        //operator=( const day& );
        ~day();

        char * getName () const;
        int    getClasses () const;
        bool   getStatus () const;

        void setName ( char * name );
        void setClasses ( int classes );
        void setStatus ( bool status );
};


#endif


/// .cpp file
#include <iostream>
#include "day.hpp"
#include <string.h>

day::day()
{
    name = new char[10];
    name = "Monday";
    nClasses = 1;
    status = true;
}

day::day(char * _name, int _classes, bool _status)
{
    name = new char[10];
    strncpy(name, _name, 10);
    nClasses = _classes;
    status = _status;
}

day::day(const day& _myday)
{
    name = new char[10];
    name = _myday.name;
    nClasses = _myday.nClasses;
    status = _myday.status;
}

char*
day::getName() const
{
    return name;
}

int
day::getClasses() const
{
    return nClasses;
}

bool
day::getStatus() const
{
    return status;
}

void
day::setName( char * _name )
{
    name = _name;
}

void
day::setClasses( int _classes )
{
    nClasses = _classes;
}

void
day::setStatus( bool _status )
{
    status = _status;
}

day::~day()
{
    if (name != 0 ) {
        std::cout << "Deleting name" << std::endl;
        delete name;
    }
}

Thanks for help.

Answer 1

If this isn't a homework exercise, stop right there, replace those char* with std::string, and your problems are solved.

(Plus you can remove your destructor and copy constructor, the compiler-generated ones are sufficient.)

---

If this is an exercise in manual memory management, then you need to fix several aspects.

You have this pattern in two places:

char *var = new char[N];
var = something_else;

This is a memory leak. The pointer new gave you is replaced in the second statements by something else. You don't have another pointer to that newly allocated block, so it has leaked.

What's more, your second assignment in the default constructor is of the form:

var = "string literal";

You're not allowed to delete a string literal, it has the lifetime of the program. So you end up in a situation where you've leaked memory and you can't tell if you must delete or not in your destructor.

To fix that, you must do an allocation (if necessary) then a copy from whatever source you're getting. This also has the benefit of not storing a pointer you've been passed directly, which will lead to other bugs if the caller decides to free it itself. So, even if the source is a string literal:

var = new char[N];
strncpy(var, source, N);
var[N-1] = 0; // check the docs for strncpy very carefully

Once you've fixed that, you'll need to abide by the Rule of Three, you really do need a copy-assignment operator. And it need to deal with self-assignment properly.

Then you have to fix your destructor too. You allocate name with new[]. You must deallocate with delete[]. These two go hands in hands. (Testing for name != 0 isn't necessary, delete and delete[] handle nulls nicely - i.e. they do nothing when handed a null pointer).

Also it's a good time to learn about std::shared_ptr and std::unique_ptr. Plain new and delete should be avoided in most cases.