reason for segmentation fault in client server chatbox program

Question

i am receiving segmentation fault here is the code

client.c

#include <stdio.h>      
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include<stdlib.h> 
#include<string.h>
#include<unistd.h>
#include <arpa/inet.h>
#define PORT 10001

main

int main(int argc ,char *argv[])
{
  struct sockaddr_in address;
  int sock = 0, valread,i=1;
  struct sockaddr_in serv_addr;
  char *hello = "Hello from client";
  char buffer[1024] = {0};
  if ((sock = socket(AF_INET, SOCK_STREAM, 0)) == -1)
  {
    printf("\n Socket creation error \n");
    return -1;
  }
  memset(&serv_addr,'0',sizeof(serv_addr));

  serv_addr.sin_family = AF_INET;
  serv_addr.sin_port = htons(PORT);
  if(inet_pton(AF_INET, "127.0.0.1", &serv_addr.sin_addr)<=0) 
  {
    printf("\nInvalid address/ Address not supported \n");
    return -1;
  }

  if (connect(sock, (struct sockaddr *)&serv_addr, sizeof(serv_addr)) == -1)
  {
    printf("\nConnection Failed \n");
    return -1;
  }
  send(sock , hello , strlen(hello) , 0 );
  //printf("Hello message sent\n");
  valread = read( sock , buffer, 1024);
  printf("%d\t %s \n",i,buffer);

  **here i am getting segmentation fault**
  while(1)
  {
    valread = read( sock , buffer, 1024);
    printf("%d\t %s \n",i,buffer);
    scanf("%s",hello);
    send(sock , hello , strlen(hello) , 0 );

    i++;
    if(strcmp(buffer,"bye")==1)
    {
      break;
    }

  }

  return 0;
}

Note that `memset(&serv_addr,'0',sizeof(serv_addr))` doesn't set the memory to zeroes, instead all bytes will have the value `48` (assuming [ASCII](http://en.cppreference.com/w/c/language/ascii) encoding). Perhaps you meant `memset(&serv_addr,0,sizeof(serv_addr))`? — Some programmer dude, Aug 04 '17 at 07:41
`char *hello = "Hello from client";` => `char hello[] = "Hello from client";` — Jean-François Fabre, Aug 04 '17 at 07:42
`scanf("%s",hello);` is attempting to write to a *string literal* which is read-only. — Weather Vane, Aug 04 '17 at 07:42
`char hello[1024] = "Hello from client";` and restrict the input length too. — Weather Vane, Aug 04 '17 at 07:44
'valread = read( sock , buffer, 1024);' and then ignoring 'valread'. You must correctly and completely handle the results returned by system calls, especially recv(). — Martin James, Aug 04 '17 at 08:23
'printf("%d\t %s \n",i,buffer);' printf("%s.."..) on a char array that is not guaranteed NUL terminated, (1024 chars may be received). — Martin James, Aug 04 '17 at 08:26
'if(strcmp(buffer,"bye")==1)' assumes that a complete C string wil be received by a single call to recv(). — Martin James, Aug 04 '17 at 08:27
''printf("%d\t %s \n",i,buffer);' printf("%s.."..)' will output extra data, repeated from previos recv() calls, if recv() loads fewer bytes than its previous call, (NUL terminator/recv result misuse). — Martin James, Aug 04 '17 at 08:29

score 2 · Answer 1 · answered Aug 04 '17 at 07:43

char *hello = "Hello from client";

is okay until you use scanf to write to hello. As hello is declared as a pointer to literal, it is read only.

Leave your hello message alone, and declare a bigger buffer (in case you have something longer to tell):

char answer[100];

and scan using limit:

scanf("%99s",answer);

Aside: strcmp returns 0 when matches. So comparing to "bye" won't work as you wish.

Gerhardh · Answer 2 · 2017-08-04T07:46:55.630

You have a pointer to a string literal:

char *hello = "Hello from client";

Then you use this pointer for scanf:

scanf("%s",hello);

String literals mustn't be modified.

You can either solve this by using an array instead as suggested in the comments:

char hello[] = "Hello from client";

or even better with specifying an apropriate size:

#define MY_BUFFER_SIZE 500
char hello[MY_BUFFER_SIZE] = "Hello from client";

Or you could also use dynamic memory allocation as well.

In any case you should be aware that your scanf is not limited in size.

reason for segmentation fault in client server chatbox program

2 Answers2