0

I have a program where i ask the user for the name of a text file, I open the text file do stuff with it (read, write), then I close the file and exit the program.

Program.h

class Program
{
     char* fileName;
public:
     Program();
     ~Program();
     void ReadFile(void);
};

Program.cpp

Program::Program(){
     //contstructor
     fileName=NULL;
}

Program::~Program(){
     cout << "in destructor" ;
     delete []fileName;
}

void Program::ReadFile(void){
     fileName = new char[40];

     cout <<"Please enter the name of the file to open: ";
     cin.clear();
     cin.getline(fileName, 40);

     ifstream file (fileName);

     if(file.is_open()){
          //do stuff
     }
     file.close();
}

right now when I put delete []fileName; in the destructor it outputs ""in destructor" on the screen but fileName does not get deleted. If I take delete []fileName; and put it in ReadFile() after file.close() fileName gets deleted. Why is that?

The rest of my program works perfectly which is why none of that code is pasted. I am just trying to rid any memory leaks and fileName is the only one I am having trouble with so therefore I only pasted the code where fileName is used.

Any help is appreciated.

Additional information: I am using Visual Studio to write this and am using the Memory Leak Detection. This is what it outputs:

Detected memory leaks!
Dumping objects ->
{132} normal block at 0x005D49A0, 40 bytes long.
Data: 6E 61 6D 65 73 2E 74 78 74 00 CD CD CD CD CD CD
Object dump complete.
The program '[10772] program1.exe: Native' has exited with code 0 (0x0).

which is why I suspect delete []fileName; didn't work.

also, this is what the int main() looks like

int main(){
     Program abc;
     abc.ReadFile();
}

Oh, and Program.h can not be changed. Only .cpp can be change it's part of my requirements.

Improve this question
6
  • not allowed to >_>, i have to use a char array Commented Oct 20, 2012 at 23:31
  • 1
    How do you know fileName isn't being deleted? Commented Oct 20, 2012 at 23:35
  • how did you check that filename is not deleted in destructor? Commented Oct 20, 2012 at 23:35
  • posted more info in the original post. Commented Oct 20, 2012 at 23:43
  • Add <<(void*)fileName to the logging in the destructor. Either it will report the same address as the leak detector (in which case either the leak detector is broken or else you've corrupted the heap), or else it will report a different address (in which case you've overwritten fileName with a different value somewhere between allocating the memory and the destructor call). Commented Oct 21, 2012 at 0:21

3 Answers 3

Reset to default
3

If filename is used only in readFile - then I advise you to remove it from Program class and make it automatic variable in that function:

void Program::ReadFile(void){
     char fileName[40];
     ...
     file.close();
     // no delete [] necessary
}

Your problem might be related to

  1. Not initializing filename to nullptr in constructor
  2. Not defining copy c-tor, assignment operator
  3. Your are not deleting old filename in readFile

So, do not use your member variables as automatic variables to your methods.

If you have to have this member varible - change it to array - do not allocate it:

class Program {
private:
  // char* filename;
  char filename[40];
};

[UPDATE]

Your .h file is incorrect - it breaks rule of three (see http://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)) - copy constructor and assignment operator is missing. So be aware not to copy your Program class in any way of you cannot change this header.

After update only one thing is missing in your program:

Either add delete[] filename at the beginning of your readFile:

void Program::ReadFile(void){
     delete [] filename;
     fileName = new char[40];

Or (better) - do not re-allocate every time readFile is called:

void Program::ReadFile(void){
     if (!filename)
         fileName = new char[40];

Or (best) - allocate this memory in constructor only:

Program::Program() : filename(new char[40]) {}
void Program::ReadFile(void){
   // fileName = new char[40];
Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

i have to use it as a member variable cause it was part of my requirements. but gonna double check the null thing. i have it initialized to NULL atm, going to see if nullptr will make a difference.
@livelaughlove Then see list of current problems in your code. Start from point 3, then 1, then 2
@livelaughlove or see my update - just do not do dynamic allocation - if it is not absolutely necessary...
tried the nullptr, it gave the same memory leak msg. there is no old fileName because the user only inputs the fileName once in the entire program. so fileName=NULL, in the beginning, then fileName="user input" and that's it. not sure about 2. and fileName is a char pointer.
ReadFile() is only called once ever, so fileName is only allocated once. I tired allocating the memory in constructor it still gives that memory leak message. Really thanks for all your ideas. I might just have to resort to putting delete [] filename; at the end of ReadFile() not understanding why when I do that it doesn't give the memory leak message.
1

Are you sure your main is exactly as posted here? If you just define abc globally, it will be freed after memory dump report memory leaks and you may see invalid reports! You can insert a break point in the destructor and see if memory leaks reported after or before destructor

Improve this answer

1 Comment

ty! exactly what the problem was. it did get free cept the leak msg printed before entering the destructor
0

fileName clearly is getting deleted: the code in the destructor says so. But if the code you didn't show calls ReadFile more than once, the class will leak memory because each call to ReadFile allocates a new memory block and overwrites the pointer to the previous block.

Think RAII: Resource Allocation Is Initialization. In the constructor, allocate the memory block. In the destructor, delete it. Then ReadFile doesn't have to worry about allocating the block.

Or, even better, do as @PiotrNycz says, and change the pointer into an array. No need for dynamic allocation.

Improve this answer

4 Comments

i cannot change the header file because it is the requirements I have to follow. which is why fileName cannot be a char array, but a char pointer. and ReadFile() is called only once in the entire program. and fileName is only ever used in ReadFile().
oh and fileName is not getting deleted for sure. because when I take delete []fileName; out of the destructor and put it at the end of ReadFile() after file.close() i don't get any memory leak msg.
@livelaughlove - you've just logically proved that your problem does not exist. So you have to figure out which of your premises is wrong. Sorry, but handwaving assertions won't get you any meaningful help. Post real code, the smallest example you can put together that compiles, runs, and demonstrates the problem.
the only code I didn't post is what I do with the file. but that has nothing to do with the fileName. the fileName is only used to store what the user inputs and use that to open the file. like i said the rest of the program works fine and I am just trying to get rid of memory leaks. when delete []fileName; is in destructor, I get a memory leak msg. when delete []fileName; is at the end of ReadFile() I get no memory leak msg. that is what I don't understand.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.