... free() heap corruption.. function works 1st time not second..

[braden87]

Hi there,

I'm using visual studio 2008 ( not my choice ).
It reports heap corruption if I choose delete a (the first) record from my simple DB of structs twice during a program execution. That is to say, I can delete the first record, write the DB to file, close the program, reopen it, delete another and it works fine.

I've narrowed down the crash to my free()s (the printfs are from my frustration lol)

	printf("pointer is: %p\n",g_ppRecords);
	if(g_ppRecords!=NULL){
		printf("pointer is: %p IN THE IF NULL\n",g_ppRecords);
		for(count=0;count<=g_numRecords;count++){
			printf("count is: %d\n",count);
			printf("fname pointer is: %p\n",g_ppRecords[count]->firstname);
			free(g_ppRecords[count]->firstname);
			g_ppRecords[count]->firstname=NULL;
			free(g_ppRecords[count]->lastname);
			g_ppRecords[count]->lastname=NULL;
			free(g_ppRecords[count]);
		}
		free(g_ppRecords);
		g_ppRecords=NULL;
	}

it crashes (if you choose delete the second time) trying to free g_ppRecords[count]->firstname.
The confusing (to me) thing is that I re-malloc the DB again the same way I did after the first delete, or before there even is a delete when the DB is loaded from txt file.
If I malloc and free the same way in a function, why is it crashing on the second call of the function but not the first? It's not like the function is changing... sigh



Thanks for any input

Braden...

the code i use for malloc follows: and then the whole source:

   	tempRecPtr=(StudentRecord**)malloc(g_numRecords * sizeof(StudentRecord*));
	if(tempRecPtr!=NULL)
    g_ppRecords=tempRecPtr;
	printf("g_numRecords: %d\n", g_numRecords);
   for(count=0;count<g_numRecords;count++){
		tempRecPtr[count]=(StudentRecord *)malloc(sizeof(StudentRecord));
		if(tempRecPtr!=NULL)
			g_ppRecords[count]=tempRecPtr[count];
		tempPtr=(char *)malloc(sizeof(char) * 256);//malloc fname 
		if(tempPtr!=NULL)
			g_ppRecords[count]->firstname=tempPtr;
				tempPtr=(char *)malloc(sizeof(char) * 256);//malloc fname 
		if(tempPtr!=NULL)
			g_ppRecords[count]->lastname=tempPtr;
   }

WHOLE SOURCE::

-----------------------------------------------------------------------------------------------------------
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <string>
#include <string.h>


// ** OLD VERSION BEFORE VALGRIND
struct StudentRecord
{
	char* firstname;
	char* lastname;
	int id;
	float mark;
};

StudentRecord** g_ppRecords;
int g_numRecords = 0;
int count=0;
int recProcessed=0;
int recPerLineCount=0;
int temp=0;
float temp2=0;
char* g_FileName;
char* tempstr;
char* piece=NULL;
FILE* g_pf;
unsigned int x=0;

bool OpenFile(void){
	char* tempPtr; // temp ptr to make sure malloc didn't return null
	StudentRecord** tempRecPtr; // temp ptr to make sure malloc didn't return null
	printf("\n Please enter the filename:");
	g_FileName =(char *)malloc(256);
	fgets(g_FileName, 250 ,stdin);
	printf("\nREADING RECORDS INTO MEMORY\n");
	g_FileName[strlen(g_FileName)-1]='\0'; // remove the new line char @ end of fgets
	g_pf = fopen (g_FileName , "r");

	tempstr = (char *)malloc(256);//malloc tempstr
	
   if (g_pf == NULL) perror ("Error opening file");
   else {
     	while (fgets(tempstr, 255, g_pf) != NULL) {
			g_numRecords++;
		}
     fclose (g_pf);
   }
   printf("Number of records in file: %d\n", g_numRecords);

   	tempRecPtr=(StudentRecord**)malloc(g_numRecords * sizeof(StudentRecord*));
	if(tempRecPtr!=NULL)
    g_ppRecords=tempRecPtr;
	printf("g_numRecords: %d\n", g_numRecords);
   for(count=0;count<g_numRecords;count++){
		tempRecPtr[count]=(StudentRecord *)malloc(sizeof(StudentRecord));
		if(tempRecPtr!=NULL)
			g_ppRecords[count]=tempRecPtr[count];
		tempPtr=(char *)malloc(sizeof(char) * 256);//malloc fname 
		if(tempPtr!=NULL)
			g_ppRecords[count]->firstname=tempPtr;
				tempPtr=(char *)malloc(sizeof(char) * 256);//malloc fname 
		if(tempPtr!=NULL)
			g_ppRecords[count]->lastname=tempPtr;
   }




	g_pf = fopen (g_FileName , "r");
	   if (g_pf == NULL) perror ("Error opening file");
   else {
	    recProcessed=0;
     	while (fgets(tempstr, 255, g_pf) != NULL) {
			// split line and add to struct
			piece = strtok (tempstr," ");
			while (piece != NULL)
			{
				if(recPerLineCount==0)
					strcpy(g_ppRecords[recProcessed]->firstname, piece);
				if(recPerLineCount==1)
					strcpy(g_ppRecords[recProcessed]->lastname, piece);
				if(recPerLineCount==2){
					temp=atoi(piece);
					g_ppRecords[recProcessed]->id=temp;
				}
				if(recPerLineCount==3){
					temp2=(float)atoi(piece);
					g_ppRecords[recProcessed]->mark=temp2;
				}
				piece = strtok (NULL," ");
				recPerLineCount++;
			}
			recProcessed++;
			recPerLineCount=0;
		}
     fclose (g_pf);
   }
	return true;
}

bool ListRecords(void){
	printf("\n"); // make it look more organized
	for(x=0; x<g_numRecords; x++){
		printf("RECORD #%d\n*******************************\n", x+1);
		printf("FIRST NAME: %s\n", g_ppRecords[x]->firstname);
		printf("LAST NAME: %s\n", g_ppRecords[x]->lastname);
		printf("ID: %d\n", g_ppRecords[x]->id);
		printf("MARK: %.2f\n\n", g_ppRecords[x]->mark);
	}
	return true;
}

bool AddRecord(void){
	char* tempPtr; // temp ptr to make sure malloc didn't return null
	StudentRecord** tempRecPtr; // temp ptr to make sure malloc didn't return null
	StudentRecord** tempRecords; // temporary copy of DB used to resize array when adding
    g_numRecords++;
	tempRecPtr=(StudentRecord**)malloc(g_numRecords * sizeof(StudentRecord*));
	if(tempRecPtr!=NULL)
    tempRecords=tempRecPtr;
	printf("g_numRecords: %d\n", g_numRecords);
   for(count=0;count<g_numRecords;count++){
		tempRecPtr[count]=(StudentRecord *)malloc(sizeof(StudentRecord));
		if(tempRecPtr!=NULL)
			tempRecords[count]=tempRecPtr[count];
		tempPtr=(char *)malloc(sizeof(char) * 256);//malloc fname 
		if(tempPtr!=NULL)
			tempRecords[count]->firstname=tempPtr;
				tempPtr=(char *)malloc(sizeof(char) * 256);//malloc fname 
		if(tempPtr!=NULL)
			tempRecords[count]->lastname=tempPtr;
   }
	for(count=0;count<g_numRecords-1;count++){
		printf("copying a firstname to temprec: %s\n", g_ppRecords[count]->firstname); 
		strcpy(tempRecords[count]->firstname, g_ppRecords[count]->firstname);
		strcpy(tempRecords[count]->lastname, g_ppRecords[count]->lastname);
		tempRecords[count]->id=g_ppRecords[count]->id;
		tempRecords[count]->mark=g_ppRecords[count]->mark;
	}
	if(g_ppRecords!=NULL){
		for(count=0;count<g_numRecords-1;count++){
			free(g_ppRecords[count]->firstname);
			g_ppRecords[count]->firstname=NULL;
			free(g_ppRecords[count]->lastname);
			g_ppRecords[count]->lastname=NULL;
			free(g_ppRecords[count]);
		}
		free(g_ppRecords);
		g_ppRecords=NULL;
	}

	printf("\n Please enter the first name: \n");
	fflush(stdin);
	fgets(tempstr, 250 ,stdin);
	temp=strlen(tempstr);
	tempstr[temp-1]='\0';  // remove new line to keep output formatting pretty
	strcpy(tempRecords[g_numRecords-1]->firstname,tempstr);
	printf("\n Please enter the last name:\n");
	fgets(tempstr, 250 ,stdin);
	temp=strlen(tempstr);
	tempstr[temp-1]='\0'; // remove new line to keep output formatting pretty
	strcpy(tempRecords[g_numRecords-1]->lastname,tempstr);
	printf("\n Please enter the ID:\n");
	fgets(tempstr, 250 ,stdin);
	temp=atoi(tempstr);
	tempRecords[g_numRecords-1]->id=temp;
	printf("\n Please enter the mark:\n");
	fgets(tempstr, 250 ,stdin);
	temp2=(float)atoi(tempstr);
	tempRecords[g_numRecords-1]->mark=temp2;

   	tempRecPtr=(StudentRecord**)malloc(g_numRecords * sizeof(StudentRecord*));
	if(tempRecPtr!=NULL)
    g_ppRecords=tempRecPtr;
	printf("g_numRecords: %d\n", g_numRecords);
   for(count=0;count<g_numRecords;count++){
		tempRecPtr[count]=(StudentRecord *)malloc(sizeof(StudentRecord));
		if(tempRecPtr!=NULL)
			g_ppRecords[count]=tempRecPtr[count];
		tempPtr=(char *)malloc(sizeof(char) * 256);//malloc fname 
		if(tempPtr!=NULL)
			g_ppRecords[count]->firstname=tempPtr;
				tempPtr=(char *)malloc(sizeof(char) * 256);//malloc fname 
		if(tempPtr!=NULL)
			g_ppRecords[count]->lastname=tempPtr;
   }

	for(count=0;count<g_numRecords;count++){
		strcpy(g_ppRecords[count]->firstname, tempRecords[count]->firstname);
		strcpy(g_ppRecords[count]->lastname, tempRecords[count]->lastname);
		g_ppRecords[count]->id=tempRecords[count]->id;
		g_ppRecords[count]->mark=tempRecords[count]->mark;
	}

	printf("before the freeeeees freeeze num records: %d\n", g_numRecords);

	for(count=0;count<g_numRecords;count++){
		free(tempRecords[count]->firstname);
		tempRecords[count]->firstname=NULL;
		free(tempRecords[count]->lastname);
		tempRecords[count]->lastname=NULL;
		free(tempRecords[count]);
	}
	free(tempRecords);
	tempRecords=NULL;
	return true;
}


bool DeleteRecord(void){
	char* tempPtr; // temp ptr to make sure malloc didn't return null
	StudentRecord** tempRecPtr; // temp ptr to make sure malloc didn't return null
	StudentRecord** tempRecords; // temporary copy of DB used to resize array
	if(g_numRecords==0)
		perror("tried to delete when there were no records!");
    g_numRecords--;
	tempRecPtr=(StudentRecord**)malloc(g_numRecords * sizeof(StudentRecord*));
	if(tempRecPtr!=NULL)
    tempRecords=tempRecPtr;

	printf("g_numRecords: %d\n", g_numRecords);
   for(count=0;count<g_numRecords;count++){
		tempRecPtr[count]=(StudentRecord *)malloc(sizeof(StudentRecord));
		if(tempRecPtr!=NULL)
			tempRecords[count]=tempRecPtr[count];
		tempPtr=(char *)malloc(sizeof(char) * 256);//malloc fname 
		if(tempPtr!=NULL)
			tempRecords[count]->firstname=tempPtr;
				tempPtr=(char *)malloc(sizeof(char) * 256);//malloc fname 
		if(tempPtr!=NULL)
			tempRecords[count]->lastname=tempPtr;
   }
	for(count=1;count<g_numRecords+1;count++){
		printf("copying a firstname to temprec: %s\n", g_ppRecords[count]->firstname); 
		strcpy(tempRecords[count-1]->firstname, g_ppRecords[count]->firstname);
		strcpy(tempRecords[count-1]->lastname, g_ppRecords[count]->lastname);
		tempRecords[count-1]->id=g_ppRecords[count]->id;
		tempRecords[count-1]->mark=g_ppRecords[count]->mark;
	}
	printf("pointer is: %p\n",g_ppRecords);
	if(g_ppRecords!=NULL){
		printf("pointer is: %p IN THE IF NULL\n",g_ppRecords);
		for(count=0;count<=g_numRecords;count++){
			printf("count is: %d\n",count);
			printf("fname pointer is: %p\n",g_ppRecords[count]->firstname);
			free(g_ppRecords[count]->firstname);
			g_ppRecords[count]->firstname=NULL;
			free(g_ppRecords[count]->lastname);
			g_ppRecords[count]->lastname=NULL;
			free(g_ppRecords[count]);
		}
		free(g_ppRecords);
		g_ppRecords=NULL;
	}
printf("pointer is: %p\n",g_ppRecords);
	g_ppRecords = (StudentRecord**)malloc(g_numRecords * sizeof(StudentRecord*));

   for(count=0;count<g_numRecords;count++){
		g_ppRecords[count]=(StudentRecord *)malloc(sizeof(StudentRecord));
		g_ppRecords[count]->firstname=(char *)malloc(sizeof(char));//malloc fname 
		g_ppRecords[count]->lastname=(char *)malloc(sizeof(char));//malloc lname
   }

	for(count=0;count<g_numRecords;count++){
		strcpy(g_ppRecords[count]->firstname, tempRecords[count]->firstname);
		strcpy(g_ppRecords[count]->lastname, tempRecords[count]->lastname);
		g_ppRecords[count]->id=tempRecords[count]->id;
		g_ppRecords[count]->mark=tempRecords[count]->mark;
	}

	for(count=0;count<g_numRecords;count++){
		free(tempRecords[count]->firstname);
		tempRecords[count]->firstname=NULL;
		free(tempRecords[count]->lastname);
		tempRecords[count]->lastname=NULL;
		free(tempRecords[count]);
	}
	free(tempRecords);
	tempRecords=NULL;
	return true;
}

void Quit(void){
	// OPEN FILE WITH "w" not a, so it will clear the file if exist
	int count=0;
	char* tempString;
	char* tempPtr;
  g_pf = fopen (g_FileName , "w");
 if (g_pf!=NULL)
  {
	for(count=0;count<g_numRecords; count++){
		tempPtr=(char *)malloc(sizeof(char) * 256);//malloc fname 
		if(tempPtr==NULL)
			  perror("could not get memory");
		else
			  tempString=(char *)malloc(sizeof(char)*10);
		
		fputs (g_ppRecords[count]->firstname ,g_pf);
		fputs (" " ,g_pf);
		fputs (g_ppRecords[count]->lastname ,g_pf);
		fputs (" " ,g_pf);
		sprintf(tempString,"%d",g_ppRecords[count]->id);
		fputs (tempString ,g_pf);
		fputs (" " ,g_pf);
		sprintf(tempString,"%.2f",g_ppRecords[count]->mark);
		fputs (tempString ,g_pf);
		fputs (" " ,g_pf);
		fputs("\n", g_pf);
		free(tempString);
	}
    fclose (g_pf);
  }


} 

int main()
{
	char response;
	if(!OpenFile())
		exit(1);

	while(true)
	{
		// The Menu
		printf(	"\n1. List records in memory\n" "2. Add a new record\n" "3. Delete a record\n" "4. Quit and Save records to file\n");
		fflush(stdin);
		scanf("%c", &response);
		switch(response)
		{
		case '1':
			if(!ListRecords())
				exit(1);
			break;
		case '2':
			if(!AddRecord())
				exit(1);
			break;
		case '3':
			if(!DeleteRecord())
				exit(1);
			break;
		case '4':
			Quit();
			return 0;
			break;
		default: printf("Invalid choice\n");
		}
	}
}

[braden87]

Please enter the filename:data.txt

READING RECORDS INTO MEMORY
Number of records in file: 2
g_numRecords: 2

1. List records in memory
2. Add a new record
3. Delete a record
4. Quit and Save records to file
1

RECORD #1
*******************************
FIRST NAME: SampleOne
LAST NAME: OneSample
ID: 45
MARK: 99.00

RECORD #2
*******************************
FIRST NAME: NotA
LAST NAME: Person
ID: 77
MARK: 88.00


1. List records in memory
2. Add a new record
3. Delete a record
4. Quit and Save records to file
3
g_numRecords: 1
copying a firstname to temprec: NotA
pointer is: 00791F40
pointer is: 00791F40 IN THE IF NULL
count is: 0
fname pointer is: 00794B50
count is: 1
fname pointer is: 00794E20
pointer is: 00000000

1. List records in memory
2. Add a new record
3. Delete a record
4. Quit and Save records to file
3
g_numRecords: 0
pointer is: 00791F40
pointer is: 00791F40 IN THE IF NULL
count is: 0
fname pointer is: 00794B50