char array/strcat() problem

This is a discussion on char array/strcat() problem within the C++ Programming forums, part of the General Programming Boards category; i'm trying to break a char array up and process it and then return the processed string. the unpack function ...

  1. #1
    Registered User
    Join Date
    Nov 2005
    Posts
    137

    char array/strcat() problem

    i'm trying to break a char array up and process it and then return the processed string. the unpack function will be at the bottom, if you don't know what its doing just by looking at the code.

    PHP Code:
    char buffer[1024], flag[5];
                
    int rc recv(engine::esock.sockbuffersizeof(buffer), 0);

                if ((
    rc != -1) && (rc != 0))
                {
                    
    unsigned int value unpack(buffer[0], buffer[1]);
                    
    itoa(valueflag10);
                    
                    
    char instruction[rc 2], rebuff[rc 3];
                    
    int i 0;
                    
    int c rc 2;

                    while (
    c)
                    {
                        
    instruction[i] = buffer[2];
                        
    i++;
                    }

                    switch (
    value)
                    {
                        case(
    0): // creation accept
                            
    strcat(rebuffflag);
                            
    strcat(rebuffinstruction);
                            break;
                    }

                    
    engine::sockRcvErr 0;

                    return(
    rebuff);
                } 
    PHP Code:
    unsigned int unpack(char byteAchar byteB)
    {
        return ((
    static_cast<unsigned int>(byteA) << 8) | byteB);

    what the code is suppose to do is take a string turn the first two bytes of the string into a 5 digit number and make that a string. then take the rest of the string and make that into another string and then add the two of them into one string and return that string.

    i know i have a switch statement with only one case. this is because there will be thousands of cases later.

  2. #2
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    > strcat(rebuff, flag);
    You should change this to strcpy, because rebuff has not been initialized:
    strcpy(rebuff, flag);

    > strcat(rebuff, instruction);
    In order to use strcat, the char arrays need a string terminator ('/0') at the end, so you could add this after your while loop:
    Code:
                    while (i < c)
                    {
                        instruction[i] = buffer[i + 2];
                        i++;
                    }
                    instruction[i] = '\0';

  3. #3
    Registered User
    Join Date
    Nov 2005
    Posts
    137
    thats still not working, its returning an empty string (i'm guessing somehow its setting the first charactedr in the string to a 0 byte)

  4. #4
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    > char instruction[rc - 2], rebuff[rc + 3];
    .
    .
    > return(rebuff);
    Here's another problem. You have rebuff declared local to this function. You can't return a pointer to a char array, since the char array is local to this function, and is destroyed upon return from the function. You have one of two choices: either make the char array a parameter to the function, or use new/delete to allocate it dynamically. The first choice is usually best, unless you don't know how big to make rebuff.

    Also, if you haven't done so already, it's always a good idea to add some print statements to verify, for example, print out buffer to see if recv() put anything in it.
    Last edited by swoopy; 02-14-2006 at 09:01 AM.

  5. #5
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    Now what you could do is return a string instead:
    Code:
                    return string(rebuff);

  6. #6
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    In which case you wouldn't need strcpy() and strcat(), so instead of:
    Code:
                            strcpy(rebuff, flag); 
                            strcat(rebuff, instruction);
    You would instead have:
    Code:
    string rebuff;
    .
    .
       rebuff = flag;
       rebuff = rebuff + instruction;
    .
    .
       return rebuff;

  7. #7
    Registered User
    Join Date
    Jan 2005
    Posts
    847
    You also have a problem with the way you use recv. Data is offen split up while it travels across the network so it may not all arive in one call to recv. You could just get one byte or you could get half the data you expected.

    There are two typical solutions. You send the length of the data first so you know how much to expect and then keep receiving data untill you reach that amount or you use deliminators and you keep receiving data untill you reach a deliminator.

  8. #8
    Registered User
    Join Date
    Nov 2005
    Posts
    137
    ok thank you all for your help. i thought i could return a local char pointer to an array because i was writing this code for a dll for an aplication, but that really doesn't make any sense so...
    anyways i was having some "random" problems with recv(), so quantum thank you for solving that problem. i thought tcp sent everything in one send and in one receive. so i guess i have to make sure everything sends and everything receives as well. basically i think i just need to learn to use sockets better. once again, thank you all for your help.

  9. #9
    Registered User
    Join Date
    Nov 2005
    Posts
    137
    sorry for the double post, but i've got a follow up and there is no sense starting a new topic.

    i changed my code to this
    Code:
    			char buffer[1024], flag[4];
    			engine::exbuff[0] = '\0';
    			
    			int rc = 1;
    			while (buffer[rc - 1] != 0x01)
    			{
    				char tbuff[1024];
    				rc = recv(engine::esock.sock, tbuff, sizeof(tbuff), 0);
    				
    				if ((rc == -1) || (rc == 0))
    				{
    					engine::sockRcvErr = 1;
    					return ("");
    				}
    
    				strcat(buffer, tbuff);
    			}
    
    			int pcount = 0;
    
    			while (pcount < rc)
    			{
    				unsigned int value = unpack(buffer[pcount], buffer[pcount + 1]);
    				itoa(value, flag, 10);
    
    				pcount += 2;
    
    				switch (value)
    				{
    					case(0): // creation accept
    						char instruction[0];
                            			instruction[0] = buffer[pcount];
    						strcat(engine::exbuff, flag);
    						strcat(engine::exbuff, instruction);
    						break;
    				}
    			}
    
    			engine::sockRcvErr = 0;
    
    			return (engine::exbuff);
    basically i added a delimeter to the messages, and then a loop to make sure everything is received before it starts processing. i know i have an array of size 0, but thats just because the message received is only suppose to be 1 character. the code that sends a message to this program is this:

    Code:
    void player::send_instruction(char *sbuff, int size)
    {
    	sbuff[size] = 0x01;
    
    	int sent = send(get_sock(), sbuff, len, 0);
    
    	while (sent < len)
    	{
    		sent = send(get_sock(), sbuff + sent, len - sent, 0);
    	}
    }
    i'm not getting random results like before, but i'm really not getting any results now. can anyone tell me what i've done wrong? thank you for your help.
    [/code]

    basically all i do is change the null terminator

  10. #10
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    A couple of things I see:

    > strcat(buffer, tbuff);
    You strcat to buffer but never initialized buffer to anything. You should set it to an empty string:
    Code:
    char buffer[1024] = "";
    >flag[4];
    Since flag is being stored from a two byte int, it could be a number as large as 32767 or -32768. So unless you know flag will only be as large as 999, enlarge it to hold seven chars (6 + the string terminator).

    > char instruction[0];
    You do realize that holds exactly 0 bytes don't you? There's not even an instruction[0]. Arrays start at 0, and if you declare an array of five, it runs 0-4, an array of one would run 0, and that's it. An array of 0, well, that's empty.

    Declare it as char instruction[2]. This will make room for one char, plus a string terminator.

    > instruction[0] = buffer[pcount];
    After this you use strcat(), which requires a string to have a string terminator ('/0') at the end. So add a line here so you have:
    Code:
                           			instruction[0] = buffer[pcount];
                           			instruction[1] = '/0';
    Last edited by swoopy; 02-17-2006 at 10:49 PM.

  11. #11
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    > while (buffer[rc - 1] != 0x01)
    I would guess this should be:
    Code:
    			while (buffer[strlen(buffer) - 1] != 0x01)
    And I may not understand how send works, but wouldn't you need a running count of bytes sent, something more like:
    Code:
    void player::send_instruction(char *sbuff, int size)
    {
    	sbuff[len++] = 0x01;
    
    	int n = send(get_sock(), sbuff, len, 0);
    
    	while (n < len)
    	{
    		int sent = send(get_sock(), sbuff + n, len - sent, 0);
    		n += sent;
    	}
    }
    Also, I don't see len declared anywhere, so I'm assuming somewhere it gets set to the length of the data in sbuff.

  12. #12
    Registered User
    Join Date
    Nov 2005
    Posts
    137
    hmm. i always thought an array of size 0 was really one index, but i guess not. oh well, i didn't think about needing room for the '\0' either. so good thing you pointed that out. thank you.

    anyways i just realized that i can't use a delimeter for the message because there isn't a byte that is guarenteed to never be sent. so i changed around the code some more, and got some results that i don't even think are possible (i'm guessing something might be wrong with my compiler or at least how i'm trying to compile).

    here is the entire function:
    Code:
    exStr sock_receive()
    {
    	engine::fdset = engine::fds;
    
    	engine::sr = select(1, &engine::fdset, NULL, NULL, &engine::selectTime);
    	
    	if (engine::sr == 1)
    	{
    		if (FD_ISSET(engine::esock.sock, &engine::fdset))
    		{
    			char 
    			buffer[1024] = "", 
    			flag[6]      = "";
    
    			engine::exbuff[0] = '\0';
    			
    			int 
    			rc     = 0,
    			size   = 0,
    			pcount = 0;
    
    			do
    			{
    				char tbuff[1024];
    
    				rc = recv(engine::esock.sock, tbuff, sizeof(tbuff), 0);
    
    				tbuff[rc] = '\0';
    
    				if (rc == -1)
    				{
    					engine::sockRcvErr = -1;
    					return ("-1");
    				}
    				if (rc == 0)
    				{
    					engine::sockRcvErr = 0;
    					return ("0");
    				}
    
    				strcat(buffer, tbuff);
    
    				if (rc > 1)
    				{
    					size = unpack(buffer[pcount], buffer[pcount + 1]);
    					pcount += 2;
    				}
    				
    			}
    			while ((rc > 1) && (sizeof(buffer) == size));
    
    			while (pcount < rc)
    			{
    				unsigned int value = unpack(buffer[pcount], buffer[pcount + 1]);
    				itoa(value, flag, 10);
    
    				pcount += 2;
    
    				switch (value)
    				{
    					case(0): // creation accept
    						char instruction[2];
    
    						instruction[0] = buffer[pcount];
    						instruction[1] = '\0';
    
    						strcat(engine::exbuff, flag);
    						strcat(engine::exbuff, instruction);
    					break;
    				}
    			}
    
    			engine::sockRcvErr = 0;
    
    			return ("wtf");
    		}
    		else
    		{
    			engine::sockRcvErr = 2;
    		}
    	}
    	else
    	{
    		engine::sockRcvErr = 3;
    	}
    
    	return ("1");
    }
    Code:
    void player::send_instruction(char *sbuff, int size)
    {
    	char buffer[size + 2];
    
    	pack(size, buffer);
    	
    	buffer[2] = '\0';
    
    	strcat(buffer, sbuff);
    
    	int sent = send(get_sock(), buffer, size + 2, 0);
    
    	while (sent < size + 2)
    	{
    		int n = send(get_sock(), buffer + sent, size - sent + 2, 0);
    		sent += n;
    	}
    }
    i added the return "-1", "0", "wtf", and "1" because i purposely added screwed up my code one time just to see if the errors were working. it turned out that they wern't so i added the returns in and it is still returning an empty string. now how is this possible? there is nowhere in this code that its not returning a constant string, and none of those strings are empty strings.

    anyways i'm really greatful that you all have been so helpful. if you could help me get this working i would really appreciate it. thank you for all your help so far.

  13. #13
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,048
    What's exStr? A typedef for string?

    it is still returning an empty string.
    How are you printing the return value?
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Need help understanding info in a header file
    By hicpics in forum C Programming
    Replies: 8
    Last Post: 12-02-2005, 12:36 PM
  2. How do i un-SHA1 hash something..
    By willc0de4food in forum C Programming
    Replies: 4
    Last Post: 09-14-2005, 06:59 AM
  3. char problem
    By ForlornOdium in forum C++ Programming
    Replies: 10
    Last Post: 10-29-2003, 02:39 PM
  4. simulate Grep command in Unix using C
    By laxmi in forum C Programming
    Replies: 6
    Last Post: 05-10-2002, 05:10 PM
  5. String Processing Problem
    By muffin in forum C Programming
    Replies: 0
    Last Post: 08-24-2001, 11:13 AM

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