Corrupting memory

This is a discussion on Corrupting memory within the C++ Programming forums, part of the General Programming Boards category; Code: if(temps[0] == 'l' && temps[1] == 'o' && temps[2] == 'o' && temps[3] == 'k') { int number = ...

  1. #1
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    319

    Corrupting memory

    Code:
    if(temps[0] == 'l' && temps[1] == 'o' && temps[2] == 'o' && temps[3] == 'k')
    	{
    
    		int number = 0;
    		int fix = 0;
    		char searchstr[30],tempstr[35];
    		char lookmsg[100];
    		for(int y=0;y<30;y++)
    		{
    			if(y == 0 && temps[y+5] - '0' < 10)	//if we are using a number to define the creature
    			{
    				fix = 2;
    			}
    			if(temps[y+5] == ' ')				//if we are done
    			{
    				searchstr[y-fix] = '\0';
    				break;
    			}
    			else								//if not next character in
    			{
    				searchstr[y-fix] = temps[y+5];
    			}
    		}
    		strcpy(tempstr," ");
    		strcat(tempstr,searchstr);
    		strcpy(searchstr,tempstr);
    		if(0 <= (temps[5] -'0') && (temps[5] -'0') <= 9) //is it a number?
    		{
    			for(int x=0;x<LNPCS;x++)
    			{
    				if(npcs[x].IsHere() == Players[ID].IsHere() && strcmp(npcs[x].GetName(),searchstr) == 0)
    				{
    					number++;
    					if(number == (temps[5] -'0'))
    					{
    						npcs[x].MakeLookMsg(lookmsg);
    						cons[ID].send((unsigned char)strlen("CHAT"));
    						cons[ID].send("CHAT");
    						cons[ID].send((unsigned char)strlen(lookmsg));
    						cons[ID].send(lookmsg);							
    					}
    				}
    			}
    		}
    		else
    		{
    			for(int x=0;x<LNPCS;x++)
    			{
    				if(npcs[x].IsHere() == Players[ID].IsHere() && strcmp(npcs[x].GetName(),searchstr) == 0)
    				{
    					npcs[x].MakeLookMsg(lookmsg);
    					cons[ID].send((unsigned char)strlen("CHAT"));
    					cons[ID].send("CHAT");
    					cons[ID].send((unsigned char)strlen(lookmsg));
    					cons[ID].send(lookmsg);				
    				}
    			}
    		}
    	}else
    Error message at runtime is:
    Stack around the variable 'tempstr' was corrupted.

    I figure it is occuring when I try to add a leading space to the search string. I don't understand what is making me overstep the allocated memory. Not sure if using pointers will help the situation any either. Anyone have some advice for me?

    edit:
    LNPCS is the current number of npcs in the game forgot I had that in...

  2. #2
    Registered User
    Join Date
    Apr 2003
    Posts
    2,662
    All <cstring> functions like strcpy() and strcat() only operate on null terminated char arrays, and these:

    char searchstr[30],tempstr[35];

    start off filled with junk values. In addition, when you use strcat() you have to have enough room in the destination array. The room left in the destination array is the room from the first null termination character to the end of the array. That has to be big enough to fit the source array from index position 0 to to the first null termination character(inclusive).
    Last edited by 7stud; 05-19-2005 at 03:02 AM.

  3. #3
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,892
    To build on what 7stud said, copying a string from a 35-char buffer to a 30-char buffer is just asking for trouble.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  4. #4
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    319

    Some more info I just noticed.

    Quote Originally Posted by CornedBee
    To build on what 7stud said, copying a string from a 35-char buffer to a 30-char buffer is just asking for trouble.

    Actually I started with two 30's figured I was overrunning the space in one to cause that error.... might go look into the null char. Mighta thought I had it in another part thanks for the help. Gonna go look now.

    edit:

    worked a bit more put some outputs in so I can log what contains what info. Here is the new code more comments some other things were changed nothing big.


    Code:
    if(temps[0] == 'l' && temps[1] == 'o' && temps[2] == 'o' && temps[3] == 'k')
    	{
    		log<<"Look was triggered"<<endl;
    		log<<"temps = "<<temps<<endl;
    		int number = 0;
    		int fix = 0;
    		char searchstr[30],tempstr[35];//changed code further down this is safe now.
    		char lookmsg[100];
    		for(int y=0;y<30;y++)
    		{
    			if(y == 0 && temps[y+5] - '0' < 10)	//if we are using a number to define the creature
    			{									//ex 2.Goblin
    				fix = 2;						//need to move the chars 2 to the left.
    			}
    			if(temps[y+5] == ' ')				//if we are done
    			{
    				searchstr[y-fix] = '\0';		//NULL char...
    				break;							//kick out of the loop
    			}
    			else								//if not next character in
    			{
    				searchstr[y-fix] = temps[y+5];
    			}
    		}
    		log<<"Search String = "<<searchstr<<endl;
    		strcpy(tempstr," \0");					//is this better than " "?
    		strcat(tempstr,searchstr);				//so I should have " Goblin" or what have you.
    		log<<"tempstr = \""<<tempstr<<"\""<<endl;
    		if(fix == 2)							//is it a number?  Found this is faster to check
    		{
    			for(int x=0;x<LNPCS;x++)			//loop through all the NPCS in the game
    			{
    				if(npcs[x].IsHere() == Players[ID].IsHere() && strcmp(npcs[x].GetName(),tempstr) == 0)
    				{//If the NPC is in the same spot as the player AND its name == what I sent.
    					number++;
    					log<<"Found a NPC with a matching name and is in correct spot."<<endl;
    					if(fix == 2 && number == (temps[5] -'0'))//ok weird... dunno what I was thinkin
    					{
    						npcs[x].MakeLookMsg(lookmsg);
    						cons[ID].send((unsigned char)strlen("CHAT"));
    						cons[ID].send("CHAT");
    						cons[ID].send((unsigned char)strlen(lookmsg));
    						cons[ID].send(lookmsg);							
    					}
    				}
    			}
    		}
    		else
    		{
    			for(int x=0;x<LNPCS;x++)
    			{
    				if(npcs[x].IsHere() == Players[ID].IsHere() && strcmp(npcs[x].GetName(),tempstr) == 0)
    				{
    					npcs[x].MakeLookMsg(lookmsg);
    					cons[ID].send((unsigned char)strlen("CHAT"));
    					cons[ID].send("CHAT");
    					cons[ID].send((unsigned char)strlen(lookmsg));
    					cons[ID].send(lookmsg);				
    				}
    			}
    		}
    	}else

    This is the relevant log file.
    Code:
    COMMAND: look Goblin
    Look was triggered
    temps = look Goblin
    Search String = Goblin
    tempstr = " Goblin"
    OK so I just now noticed the line it was giving me wasn't even in this whole if it is at the end of the function so does that mean it is having trouble freeing the memory or something?
    Last edited by ~Kyo~; 05-20-2005 at 03:07 AM. Reason: More info

  5. #5
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,334
    > if(temps[0] == 'l' && temps[1] == 'o' && temps[2] == 'o' && temps[3] == 'k')
    Try using
    if ( strncmp( temps, "look", 4 ) == 0 )

    > if(fix == 2)
    You also compare the same variable for the same value inside this condition, and since there is no obvious sign of fix changing, the inner test is useless.

    Already, this is looking like part of a much larger function. Split it up into more manageable parts.
    > for(int x=0;x<LNPCS;x++)
    You have two very similar looking blocks of code. Extract a parameter or two, and make the rest into a separate function.

    > for(int y=0;y<30;y++)
    It's possible for this loop to exit without ever appending a \0

    > strcpy(tempstr," \0");
    1. tempstr[0] = '\0'; is better
    2. Given that this is C++, why not use all the comfort and safety of C++ strings rather than the awkward and error prone C-style char arrays?
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  6. #6
    Registered User ~Kyo~'s Avatar
    Join Date
    Jun 2004
    Posts
    319
    Quote Originally Posted by Salem
    > if(temps[0] == 'l' && temps[1] == 'o' && temps[2] == 'o' && temps[3] == 'k')
    Try using
    if ( strncmp( temps, "look", 4 ) == 0 )
    Didn't know about that function but will use in the future

    > if(fix == 2)
    You also compare the same variable for the same value inside this condition, and since there is no obvious sign of fix changing, the inner test is useless.
    Yea... I got rid of that before I fell asleep last night.

    Already, this is looking like part of a much larger function. Split it up into more manageable parts.
    It is a large function called translate. It determines what the character is doing by interperating the data they send.

    > for(int x=0;x<LNPCS;x++)
    You have two very similar looking blocks of code. Extract a parameter or two, and make the rest into a separate function.
    could try that... I am looking into it.

    > for(int y=0;y<30;y++)
    It's possible for this loop to exit without ever appending a \0
    I think that is my problem... but still when I toss it into a file I don't see the extra junk that I usually see when I leave a '\0' off of a string.

    > strcpy(tempstr," \0");
    1. tempstr[0] = '\0'; is better
    2. Given that this is C++, why not use all the comfort and safety of C++ strings rather than the awkward and error prone C-style char arrays?
    IDK char arrays seem to come more natural to me as odd as that sounds. Also I need that leading whitespace.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. available memory from task manager
    By George2 in forum Tech Board
    Replies: 10
    Last Post: 01-18-2008, 01:32 AM
  2. Replies: 4
    Last Post: 01-13-2008, 01:14 AM
  3. Question regarding Memory Leak
    By clegs in forum C++ Programming
    Replies: 29
    Last Post: 12-07-2007, 12:57 AM
  4. Memory problem with Borland C 3.1
    By AZ1699 in forum C Programming
    Replies: 16
    Last Post: 11-16-2007, 10:22 AM
  5. Shared Memory - shmget questions
    By hendler in forum C Programming
    Replies: 1
    Last Post: 11-29-2005, 01:15 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21