Thread: Are there any memory leaks here??

  1. #1
    Registered User
    Join Date
    Jan 2007
    Posts
    18

    Are there any memory leaks here??

    Code:
    bool AddRecord(StudentRecord**& Records, int& numRecords){
    /*Please tell me if I have memory leaks.*/	
    /*	
    	struct StudentRecord
    {
    	char* firstName;			// student key
    	char* lastName;				// student name
    	int id;						// student id
    	float mark;					// course mark
    };*/
    	
    	char FirstName[MaxRecord];char LastName[MaxRecord];int id; float mark;
    
    	
    	
    StudentRecord**Temp; /*Declare array of pointers to a struct*/
    	Temp=new StudentRecord*[numRecords];
    	
    
    	cout<<"Please enter First Name:\n";
    	cin>>FirstName;
    	cout<<"Please enter Last Name:\n";\
    	cin>>LastName;
    	cout<<"Please enter ID:\n";
    	cin>>id;
    	cout<< "Please enter mark\n";
    		cin>>mark;
    	numRecords++;
    	
    	for (int i=0; i<numRecords;i++)
    	{
    	 /*dynamically allocate memory*/
    	Temp[i] = new StudentRecord; // allocate space for student record
    	Temp[i]->firstName=new char[strlen(FirstName)+1]; 
    	Temp[i]->lastName=new char[strlen(LastName)+1];
    	strcpy(Temp[i]->firstName,FirstName);
    	strcpy(Temp[i]->lastName,LastName);
    	Temp[i]->id=id;
    	Temp[i]->mark=mark;
    	}
    	if (numRecords==1)
    	{
    		Records=new StudentRecord*[numRecords];
    		for	(int j=0;j<numRecords;j++)
    		{
    			
    			Records[j] = new StudentRecord;
    			Records[j]->firstName=new char[strlen(Temp[j]->firstName)+1]; 
    			Records[j]->lastName=new char[strlen(Temp[j]->lastName)+1];
    			strcpy(Records[j]->firstName,Temp[j]->firstName);
    			strcpy(Records[j]->lastName,Temp[j]->lastName);
    			Records[j]->id=Temp[j]->id;
    			Records[j]->mark=Temp[j]->mark;
    		}
    	}
    	else if (numRecords>1)
    	{
    		for (int k=numRecords-1, l=0; l<numRecords, k<numRecords;k++,l++)
    			/*get ready to write from last records in studentrecord** Records by copying
    			 studentrecord **temp starting at 0*/
    		{
    			Records[k] = new StudentRecord;// allocate space for student record
    			Records[k]->firstName=new char[strlen(Temp[l]->firstName)+1];
    			Records[k]->lastName=new char[strlen(Temp[l]->lastName)+1];
    			strcpy(Records[k]->firstName,Temp[l]->firstName);
    			strcpy(Records[k]->lastName,Temp[l]->lastName);
    			Records[k]->id=Temp[l]->id;
    			Records[k]->mark=Temp[l]->mark;
    		}
    	}
    	
    
    
    	//delete []Temp; /*crashes when I use this*/
    	return true;
    }

  2. #2
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    >> Temp=new StudentRecord*[numRecords];
    When that line executes, what is the value of numRecords?

    You don't really have a memory leak (at least, that's probably not the most pressing problem), but you do have many issues you have with pointers and memory allocation in the posted code. In C++, you don't have to do all that work yourself (and risk having so many errors that could lead to crashes). You can use the standard C++ vector and string classes.

    If you use a vector instead of your dynamic array, then you can use push_back to add new records to the vector instead of having to identify how many records will be created from the start. There are a bunch of other benefits as well. Is there a reason you aren't using the C++ features available?

  3. #3
    Registered User
    Join Date
    Jan 2007
    Posts
    18
    Quote Originally Posted by Daved
    >> Temp=new StudentRecord*[numRecords];
    When that line executes, what is the value of numRecords?

    You don't really have a memory leak (at least, that's probably not the most pressing problem), but you do have many issues you have with pointers and memory allocation in the posted code. In C++, you don't have to do all that work yourself (and risk having so many errors that could lead to crashes). You can use the standard C++ vector and string classes.

    If you use a vector instead of your dynamic array, then you can use push_back to add new records to the vector instead of having to identify how many records will be created from the start. There are a bunch of other benefits as well. Is there a reason you aren't using the C++ features available?
    it is an assignment requirement to do it this way. It's the prof's specification. What would it look like in vectors anyway?

  4. #4
    Registered User
    Join Date
    Jan 2007
    Posts
    18
    Quote Originally Posted by cmasri
    it is an assignment requirement to do it this way. It's the prof's specification. What would it look like in vectors anyway?
    the value of num records is 0 when that line executes.

  5. #5
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    >> the value of num records is 0 when that line executes.
    Exactly. You are allocating space for an array and specifying 0 as the number of objects you want in that array.

    Since this is an AddRecord function, maybe you meant to increase the value of numRecords first, and then allocate space for the temp array? So if you add a single record and numRecords is 0, then you would increment numRecords first to get to 1, then allocate space for it. Does that make sense?

    >> for (int i=0; i<numRecords;i++)
    What is this for loop supposed to do? Is it copying the old data, or just inserting the new record?

  6. #6
    Registered User
    Join Date
    Jan 2007
    Posts
    18
    the loop that you are referring to is copying old data from a temporary array.

    made improvements according to your suggestion, is that better?
    Code:
     StudentRecord**Temp; /*Declare array of pointers to a struct*/
    	
    	cout<<"Please enter First Name:\n";
    	cin>>FirstName;
    	cout<<"Please enter Last Name:\n";\
    	cin>>LastName;
    	cout<<"Please enter ID:\n";
    	cin>>id;
    	cout<< "Please enter mark\n";
    		cin>>mark;
    	numRecords++;
    	Temp=new StudentRecord*[numRecords];

  7. #7
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    >> is that better?
    Yes, I think that should fix that particular problem.

    >> the loop that you are referring to is copying old data from a temporary array.
    I don't see where you access the old data. It looks like you are copying the data input by the user over and over again into each slot in the temporary array, so that when the loop is done your temp array will be full of the same data in each spot. I think you need to change it to copy over the data from the old array itself.

  8. #8
    Registered User
    Join Date
    Jan 2007
    Posts
    18
    Quote Originally Posted by Daved
    >> is that better?
    Yes, I think that should fix that particular problem.

    >> the loop that you are referring to is copying old data from a temporary array.
    I don't see where you access the old data. It looks like you are copying the data input by the user over and over again into each slot in the temporary array, so that when the loop is done your temp array will be full of the same data in each spot. I think you need to change it to copy over the data from the old array itself.
    thank you that works.
    another question....

  9. #9
    Registered User
    Join Date
    Jan 2007
    Posts
    18
    Code:
    void ListFileMem(StudentRecord** Records, int numRecords){
    	cout<<"LIST RECORDS FROM MEMORY";
    	string fullname, firstname, lastname,space(" ");
    	if (numRecords>0)
    	{
    		for (int k=0; k<numRecords;k++)
    		{
    			firstname=Records[k]->firstName;
    			firstname+=" ";
    			lastname=Records[k]->lastName;
    			fullname=firstname+lastname; /*compiler says i can't do this, why??*/
    			cout<<"Record"<<k<<"\n"<< "First Name: "<<fullname<<"\t"<<
    				"Last Name: "<< Records[k]->lastName<<"\t"<<"ID:"<<Records[k]->id
    				<<"\t"<<"Mark:"<<Records[k]->mark<<endl;;
    		}
          }
    	else
    	
    		cout<<"memory empty, unable to list\n";
    
    
    
    
    }
    Code:
    error from compiler
    
    n:\assignment0.cpp(265): error C2784: 'std::reverse_iterator<_RanIt> std::operator +(_Diff,const std::reverse_iterator<_RanIt> &)' : could not deduce template argument for 'const std::reverse_iterator<_RanIt> &' from 'std::string'
    the name comes in by 2 char* char *firstname and char*lastname. I want to have the name contatenated when I display it.

    example display
    Code:
    Name: Lester Pearson
    Last edited by cmasri; 01-17-2007 at 07:55 PM.

  10. #10
    Registered User
    Join Date
    Jan 2007
    Posts
    330
    Quote Originally Posted by cmasri
    it is an assignment requirement to do it this way. It's the prof's specification. What would it look like in vectors anyway?
    indeed, this is not the modern way to code in C++ and if I ever got to work with you and you'd write code like this I would beat you with a rubber dildo. But of course if you're learning about pointers and dynamic memory it's a good way.

    In modern C++ it would look more like:

    Code:
    // even better a full fledged class
    struct StudentRecord
    {
    	std::string firstName;			
    	std::string lastName;			
    	int id;						
    	float mark;				
    };
    
    int main()
    {
      std::vector<StudentRecord> students;
    
      for(...)
      {
        StudentRecord student;
        // read a new student or even overload operator>> for StudentRecord
        students.push_back(student);
      }
      
      return 0;
    }
    you dont see any pointers, new's and deletes in this code anymore

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Checking for memory leaks
    By Bladactania in forum C Programming
    Replies: 5
    Last Post: 02-10-2009, 12:58 PM
  2. memory leaks
    By TehOne in forum C Programming
    Replies: 4
    Last Post: 10-10-2008, 09:33 PM
  3. Tons of memory leaks
    By VirtualAce in forum C++ Programming
    Replies: 11
    Last Post: 12-05-2005, 10:19 AM
  4. COM Memory Leaks
    By subdene in forum Windows Programming
    Replies: 0
    Last Post: 06-07-2004, 11:57 AM
  5. about memory leaks with this simple program
    By Unregistered in forum C++ Programming
    Replies: 5
    Last Post: 04-07-2002, 07:19 PM