Thread: Segmentation fault on TCP recv

  1. #1
    Registered User
    Join Date
    Jan 2011
    Posts
    41

    Segmentation fault on TCP recv

    Hi everyone,

    I have run into an issue where I need to transmit a custom struct I created to a program using TCP sockets and threads.

    I believe that I am allocating memory correctly but I receive a seg fault every time the recv() call is reached. So I am not really sure what is going on.

    Code:
            //struct definition
    	typedef struct
    	{
    		char *header;
    		char data[256];
    	}Message;
    
       void * respond(void * incomingSocketFD)
       {
            printf("in respond\n");
            char buffer[32];
            int newFD;
    	nt retval;
    		
    	//copy socket 
    	memcpy(&newFD, &incomingSocketFD, sizeof(incomingSocketFD));
    		
    	//allocate space for message
    	message = malloc(sizeof(Message *));
       	message->header = (char *)malloc(sizeof(char));
    		
          while(retval != 0)
          {
    	printf("inside while in respond\n");
          	/* Receive data from the client */
             retval = recv(newFD, message, sizeof(Message), 0); //!!!!!SEG FAULT HERE
    
             if(retval < 0)//error check
             {
                perror("NATbox:recv()");
                exit(1);
             }
           }//end while
    }//end respond

    Any help/pointers would be great,
    Thanks,
    Hunter

  2. #2
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    > //copy socket
    > memcpy(&newFD, &incomingSocketFD, sizeof(incomingSocketFD));
    Tell me, is this a thread function?
    Show us how you invoke this thread - in particular, details of the thread parameter pointer. Is this a pointer to a local variable (this is almost always a bug).

    Some points,
    1. the size you're copying is sizeof(void*), not the size of an integer
    2. &incomingSocketFD is a pointer to the parameter, not where it points to (drop the &)

    > message->header = (char *)malloc(sizeof(char));
    What is the point of allocating a single char, when you have a fixed sized array also in the struct?

    > retval = recv(newFD, message, sizeof(Message), 0);
    You just trashed your previous malloc.
    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.

  3. #3
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    Presumably message is a pointer to a Message. You should allocate sizeof(Message), not sizeof(Message *). You want enough bytes to store all the elements of the Message structure, not just 4 bytes for a pointer. The safest way to do this is:

    Code:
    message = malloc(sizeof(*message));
    You also have the first element of the Message structure declared as a char *. If you try to read from the socket into that, you wont get what you think. The header pointer on the sending system will not be valid on the receiving system. You allocate space for it, but you only allocate sizeof(char), which is one byte. Also, that byte you allocated doesn't live at the beginning of the struct. It lives somewhere else, and is only pointed to by the address in header. The same goes for your sending code. You probably want to make header a fixed-size array like data, or if you have to allocate memory, allocate sufficient space and read in chunks:
    Code:
    message->header = malloc(42);  // note I don't cast malloc
    
    recv(newFD, message->header, 42, 0);
    recv(newFD, message->data, sizeof(message->data), 0);
    Lastly, if you are talking across different architectures or compilers, you may find your send and receive structures don't match up due to padding or alignment issues.

  4. #4
    Banned
    Join Date
    Aug 2010
    Location
    Ontario Canada
    Posts
    9,547
    You've used sizeof(message*) when allocating memory... Most likely that got you 4 bytes. Try it without the *, like you did in recv.

    You did the same when allocating a single character to your message header.

    Then you went totally rogue and asked recv ... which expects a byte oriented buffer to load your message struct directly.

  5. #5
    Registered User
    Join Date
    Jan 2011
    Posts
    41
    In response to your question Salem, yes this is a thread function, I have included the portion of code where I create the thread and start the function, including the argument I pass the function.

    Code:
     /*
       * Recv the data from the sender
       */
          while (1)
          {
          /* accept a connection from a client program */
             int newFD;
             newFD = accept( socketFD, (struct sockaddr *)&srcAddr, (socklen_t *)&fromlen );
             if ( newFD < 0 )
             {
                perror( "NATbox:accept()" );
                return -1;
             }
          
             int threadError = pthread_create(&tid, NULL, respond, (void *) newFD); 
             if(threadError < 0)
             {
                perror("NATbox:spawn thread\n");
                return -1;
             }
          }
    		
          int status;
          int i = 0;
          while( i != -1 ) 
          {
             i = wait( &status );
             printf( "Finished pid = %d\n", i );
          }
       
       /*
       * Return a 0 to say it worked correctly from this end
       */
          return 0;
       }
    	
       void * respond(void * incomingSocketFD)
       {
          printf("in respond\n");
          char buffer[32];
          int newFD;
          int retval;
    		
          //copy socket 
    	memcpy(&newFD, &incomingSocketFD, sizeof(incomingSocketFD));
    		
    	//allocate space for message
    	message = malloc(sizeof(Message));
       	message->header = (char *)malloc(sizeof(char *));
    		
          while(retval != 0)
          {
    	printf("inside while in respond\n");
          	/* Receive data from the client */
             retval = recv(newFD, message, sizeof(Message), 0);
    
             if(retval < 0)//error check
             {
                perror("NATbox:recv()");
                exit(1);
             }
    I changed the struct as you suggested to have the header field be a fixed array of size 1, which along with your suggested changes to my malloc call have resolved my segmentation fault issue.

    The header field of the Message struct is just a single character which indicates what service the client program wants, "A", "M", or "C".

    Thanks for the help.
    Hunter

  6. #6
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    Quote Originally Posted by mcmillhj View Post
    The header field of the Message struct is just a single character which indicates what service the client program wants, "A", "M", or "C".
    I hope you meant 'A', 'M' or 'C', since those are character literals. The double " " is for string literals, meaning they
    1. Take up two bytes (one for the letter, one for the terminating '\0')
    2. Can't be assigned with = or compared with ==, you need strcpy and strcmp.

  7. #7
    Registered User
    Join Date
    Jan 2011
    Posts
    41
    That is what I meant, thanks for pointing that out, one of my Java habits I suppose.

    Hunter

  8. #8
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    Well your thread seems OK, if a little unconventional (and assuming pointers and ints are the same size).

    But you're still allocating memory, and then overwriting your allocation.

    Can you show us the send code as well?
    I hope you're not trying to send that structure "as is", complete with a pointer.
    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.

  9. #9
    Registered User
    Join Date
    Jan 2011
    Posts
    41
    The send is working fine, I am reluctant to post the send code as this is for a class project, that is why I only posted a segment of the recv code.

    Hunter

  10. #10
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    Code:
    	typedef struct
    	{
    		char *header;
    		char data[256];
    	}Message;
            Message msg;
    If you do
    send(fd,&msg,sizeof(msg),0);
    then your send is BROKEN.
    The 4 bytes of the pointer will arrive OK, but it will be completely meaningless to the receiver. send() certainly isn't going to dereference the pointer for you (how would it know) and send what it points to.

    You need to do at least
    send(fd,msg.header,strlen(msg.header)+1,0);
    send(fd,msg.data,strlen(msg.data)+1,0);

    This sends both strings, AND a separating \0.

    If your header/data are binary data, then send a length value in advance of the data itself.
    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.

  11. #11
    Registered User
    Join Date
    Jan 2011
    Posts
    41
    my send call looks like this:

    Code:
    send(fd, message, sizeof(message), 0);
    //message is a pointer to a struct of type Message
    Is that wrong?

    Hunter

  12. #12
    Banned
    Join Date
    Aug 2010
    Location
    Ontario Canada
    Posts
    9,547
    Quote Originally Posted by mcmillhj View Post
    my send call looks like this:

    Code:
    send(fd, message, sizeof(message), 0);
    //message is a pointer to a struct of type Message
    Is that wrong?

    Hunter
    If struct message includes pointers, per the definition you published above, yes it's wrong.

    What happens is that the pointer may be valid on the sending system but it is the pointer itself, not the data at the pointer that is transferred. On the receiving end attempting to access the pointer, which is not valid on that machine, causes a seg-fault.

    You explained that your header is a single character, so...

    Code:
    typedef struct tMessage
    {  char header;
        char data[256]; }
    Message, *pMessage;
    
    Message msg;
    That should fix the problem.
    Last edited by CommonTater; 03-17-2011 at 08:25 AM.

  13. #13
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    > //message is a pointer to a struct of type Message
    Yes, it's wrong twice.
    1. You use sizeof() on what appears to be a pointer, not what it points at.
    2. What it points at in itself also contains pointers.

    Check the return values of send()
    Does it look like you've sent two complete strings?
    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.

  14. #14
    Registered User
    Join Date
    Jan 2011
    Posts
    41
    I have redefined my struct so that it contains two members, a character, and a fixed size array of characters. You are correct about me only transferring sizeof(message) which was not sending the whole struct, just the header character.

    Code:
    typedef struct
    {
        char header;
        char data[256];
    }Message;
    
    //new send call
    send(fd, message, sizeof(Message), 0);
    Does that look better? It now sends the whole struct and my server is responding correctly.

    Thanks
    Hunter

  15. #15
    Banned
    Join Date
    Aug 2010
    Location
    Ontario Canada
    Posts
    9,547
    Quote Originally Posted by mcmillhj View Post
    Does that look better? It now sends the whole struct and my server is responding correctly.
    Then, yes it looks better...

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 4
    Last Post: 05-05-2009, 05:35 AM
  2. Segmentation fault
    By bennyandthejets in forum C++ Programming
    Replies: 7
    Last Post: 09-07-2005, 05:04 PM
  3. Segmentation fault
    By NoUse in forum C Programming
    Replies: 4
    Last Post: 03-26-2005, 03:29 PM
  4. Locating A Segmentation Fault
    By Stack Overflow in forum C Programming
    Replies: 12
    Last Post: 12-14-2004, 01:33 PM
  5. Segmentation fault...
    By alvifarooq in forum C++ Programming
    Replies: 14
    Last Post: 09-26-2004, 12:53 PM