Buffer overflow error. please help

**Matt Hintzke** · 02-01-2012

For some reason I am getting a BO error right when the searchAction() function is exiting itself back to main. the weird thing is that the overflow is around the sArtist string and it only does this when I call the editSong() function. It works perfectly fine with the deleteSong() and rateSong() functions which all use the same string, sArtist, to run a search on my linked list. So what is it about the editSong() function that causes it to do this? Heres my code for these two functions:

Code:

void searchAction(Node *pList, int searchType)
{
    char sArtist[32], sSong[32];


    printf("Search:\n");
    scanf("%c");
    printf("Artist: ");
    fgets(sArtist, 32, stdin);
    printf("Song: ");
    fgets(sSong, 32, stdin);


    removeNewline(sArtist);
    removeNewline(sSong);


    switch(searchType)
    {
    case 1://delete
        deleteSong(sArtist, sSong, &pList);
        break;
    case 2://edit
        editSong(sArtist, sSong, &pList);
        break;
    case 3://rate
        rateSong(sArtist, sSong, &pList);
        break;
    }
    
}

void editSong(char Artist[32], char Song[32], Node **pList)
{
    Node *pMem = *pList;
    int check = 0;
    while(pMem != NULL && check==0)
    {
        if(strcmp(Artist, (pMem->data)->artist)==0 && strcmp(Song, (pMem->data)->title)==0)
        {


            printf("-------------------------------------------\n");
            printf("%s-%s\n", (pMem->data)->artist, (pMem->data)->title);
            printf("Album: %s\tGenre: %s\n", (pMem->data)->album, (pMem->data)->genre);
            printf("Song length: %d:%d\t", (pMem->data)->length.minutes, (pMem->data)->length.seconds);
            printf("Times Played: %d\n", (pMem->data)->num_plays);
            printf("Rating: %d\n", (pMem->data)->rating);
            printf("-------------------------------------------\n\n");
            //delete node
            if(pMem->pNext!=NULL)
            {
            (pMem->pPrev)->pNext = pMem->pNext;
            (pMem->pNext)->pPrev = pMem->pPrev;
            }else{
            (pMem->pPrev)->pNext = NULL;
            }
            free(pMem);
            //insert new song
            insertSong(pList, getInfo());
            
            printf("Edit Successful..");
            check = 1;
        }else{
            pMem = pMem->pNext;
        }
    }
    if(check == 0)
    {
    printf("Edit Failed..No song found");
    }
}

**stahta01** · 02-01-2012

Code:

void searchAction(Node *pList, int searchType)

Code:

void editSong(char Artist[32], char Song[32], Node **pList)

I guess that this is wrong; I suggest not passing "&pList" to editSong. Instead change searchAction to use "Node **pList" instead of "Node *pList".

I think that passing the address of a pass by value parameter is not always a legal thing to do.

Tim S.

**Salem** · 02-01-2012

> scanf("%c");
Mmm, where are you writing a character?

If you just want to remove it, use getchar().

> void searchAction(Node *pList, int searchType)
Since this function calls functions which modify the list, it too should have a Node** parameter.

**memcpy** · 02-01-2012

Initialization Error..? Help Please

Duplicate thread? You still didn't remove the random "%c"..

**Matt Hintzke** · 02-02-2012

no its not a duplicate, i have dont everything you guys said and it still gives me the overflow error when searchAction() is ending, after editSong() is called.

**MK27** · 02-02-2012

Originally Posted by Matt Hintzke

i have dont everything you guys said and it still gives me the overflow error when searchAction() is ending, after editSong() is called.

You should post your updated code, and explain more precisely the error (where did it come from? what exactly did it say?).

**Matt Hintzke** · 02-02-2012

Here are all the functions that are involved with this error. I have changed searchAction so that now I don't get the same Overflow error. Instead, now when I edit a song and go back to main, the pointer for pList is no longer valid and the changes arent kept, so when I try to printList() after editing, the program crashes. do I need a (***) triple dereference operator for this or something? How can I keep the pointer from becoming invalid once I edit the song and the program returns back to main.

Code:

int main(void)
{
	Node *pList = NULL;
	int option;
	
	do
	{
	option = displayMenu();
		switch(option)
	{
		case 1://load
			LoadSongs(&pList);
			break;
		case 2://store
			storeSongs(pList);
			break;
		case 3://display
			printList(pList);
			break;
		case 4://insert
			insertSong(&pList, getInfo());
			break;
		case 5://delete
			searchAction(pList, 1);//1 is for delete
			break;
		case 6://edit
			searchAction(pList, 2);//2 is for edit
			break;
		case 7://sort
			break;
		case 8://rate
			searchAction(pList, 3);//3 is for rate
			break;
		case 9://exit
			option = 0;
			break;
		default:
			printf("Invalid choice..\n");
			option = 10;
			getch();
	}
	}while(option != 0);
	return 0;
}

Code:

void searchAction(Node *pList, int searchType)
{
	char sArtist[128] = {0}, sSong[128] = {0};
	
	printf("Search:\n");
	scanf("%c");
	printf("Artist: ");
	fgets(sArtist, 128, stdin);
	printf("Song: ");
	fgets(sSong, 128, stdin);


	removeNewline(sArtist);
	removeNewline(sSong);
	
	switch(searchType)
	{
	case 1://delete
		deleteSong(sArtist, sSong, &pList);
		break;
	case 2://edit
		if(editSong(sArtist, sSong, &pList)==1)
		{
			//insert new song
			insertSong(&pList, getInfo());
			
			printf("Edit Successful..");
			getch();
		}
		break;
	case 3://rate
		rateSong(sArtist, sSong, &pList);
		break;
	}
}

Code:

int editSong(char sArtist[128], char sSong[128], Node **pList)
{
	Node *pMem = *pList, *pTemp = NULL;
	int check = 0;


	while(pMem != NULL && check==0)
	{
		
		if(strcmp(sArtist, (pMem->data)->artist)==0 && strcmp(sSong, (pMem->data)->title)==0)
		{


			printf("-------------------------------------------\n");
			printf("%s-%s\n", (pMem->data)->artist, (pMem->data)->title);
			printf("Album: %s\tGenre: %s\n", (pMem->data)->album, (pMem->data)->genre);
			printf("Song length: %d:%d\t", (pMem->data)->length.minutes, (pMem->data)->length.seconds);
			printf("Times Played: %d\n", (pMem->data)->num_plays);
			printf("Rating: %d\n", (pMem->data)->rating);
			printf("-------------------------------------------\n\n");
			//delete node


			if(pMem->pNext != NULL && pMem->pPrev != NULL)
			{
				(pMem->pPrev)->pNext = pMem->pNext;
				(pMem->pNext)->pPrev = pMem->pPrev;
			}else if(pMem->pNext == NULL && pMem->pPrev != NULL)
			{
				(pMem->pPrev)->pNext = NULL;
			}else if(pMem->pNext !=NULL & pMem->pPrev == NULL)
			{
				(*pList) = pMem->pNext;
				(pMem->pNext)->pPrev = (*pList);
			}
				
			free(pMem);
			check = 1;
		}else{
			pMem = pMem->pNext;
		}
	}
	if(check == 0)
	{
	printf("Edit Failed..No song found");
	}
	return check;
}

Code:

Boolean insertSong(Node **pList, Song_struct *mySong)
{
	Boolean status = FALSE;
	Node *pMem = NULL, *tempPrev = NULL, *tempCur = NULL;
	
	pMem = makeNode(mySong);


	if(pMem != NULL)
	{
		status = TRUE;


		if(*pList != NULL)//if list is not empty
		{
			tempCur = (*pList);
			while(((tempCur)!=NULL) && strcmp((pMem->data)->artist, ((tempCur->data)->artist))>0)
			{
				tempPrev = tempCur;
				tempCur = tempCur->pNext;
			}
			if(tempPrev == NULL)
			{
				pMem->pNext = tempCur;
				pMem->pPrev = NULL;
				*pList = pMem;
			}else if(tempCur == NULL){


				pMem->pNext = NULL;
				pMem->pPrev = tempPrev;


				tempPrev->pNext = pMem;
			}else
			{
				pMem->pNext = tempCur;
				pMem->pPrev = tempPrev;


				tempPrev->pNext = pMem;
				tempCur->pPrev = pMem;
			}
		}else{
			(*pList) = pMem;
			pMem->pNext = NULL;
			pMem->pPrev = NULL;
		}


	}
	return status;	
}

**Matt Hintzke** · 02-02-2012

this may help...if I comment out line 25 in the searchAction function it works and does not crash. It just never adds the new updated song to my linked list. Therefore it must ahve something to do with my call for insertSong().

Ok, so I just made my program work by moving my insertSong() function that is supposed to be called after editSong() to main. I created a boolean variable to check whether the edit worked and if so, insertSong() is now called from main(). So I am not sure why I have to do this. I am not a pro at pointers yet, so I am guessing I was doing something wrong with that.

**stahta01** · 02-02-2012

I repeat my prior comment.

Originally Posted by stahta01

I think that passing the address of a pass by value parameter is not always a legal thing to do.

Tim S.

**MK27** · 02-02-2012

Originally Posted by Matt Hintzke

I am not a pro at pointers yet, so I am guessing I was doing something wrong with that.

Because of what Tim aka. stahta01 said. Try this:

Code:

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

void test (int *p) {
	printf ("Address of x in test() %p\n", &p);
}

void test2 (int **p) {
	printf ("Address of x in test2() %p\n", p);
}

int main(void) {
	int n = 5, *x = &n;
	printf("Address of x in main: %p\n", &x);
	test(x);
	test2(&x);

	return 0;
}

The address in main() and the address in test2() are the same, so if you then pass this value (an address) on to another function from test2, it will have that original address, so you can affect the original. But if you pass the address from test(), that is an address which is local to test()'s stack -- it contains a copy of a value passed in, which means passing that on will not affect the original. Using & inside a function to refer to a variable passed in will give you the local address where the value of that variable is stored, not the address of the place where it came from. If the value of the address where it came from was not passed in, & will not get it for you! If that isn't clear, ask and I can try to clarify.

By moving the action back to main, you have circumvented the problem. That's not the only solution, but it's probably a pretty decent one, because a function should perform a task, not perform a task and then take over the primary flow control of the program (that's just moving "spagetti" around on the plate

).

You still have not explained in detail what the nature of the error is. Initially you referred to a "buffer overflow error" -- which is not something a standard C compiler, or an executable, normally reports. Later you refer to "crashing" which is ambiguous (it could be caused by a buffer overflow, but how do you know?). Since you are new to this stuff, it is important you include details like that so more experienced people can determine what information is important. You may easily be assuming some things about the nature of the "crash" wrongly.

**Matt Hintzke** · 02-02-2012

Ok thanks a lot, I will try to fix it using what you said. By crash, I mean the program freezes and the Not Responding pop up appears saying it is trying to find a solution. Just like when any other windows program stops responding. It isn't giving me any error report, it just crashes

**Matt Hintzke** · 02-02-2012

so I changed the searchAction(Node *pList) to have Node **pList, but now if I want to call insertSong() from searchAction, do I send in pList or &pList?

**Salem** · 02-02-2012

Like I said in post #3
> > void searchAction(Node *pList, int searchType)
> Since this function calls functions which modify the list, it too should have a Node** parameter.
If ANY function modifies your list, you need a ** parameter, and you call it with &pList from main

Thread: Buffer overflow error. please help

Thread Tools

Search Thread

Display