0

I am trying to read the data from the file /tmp/k on the server side and copying the data to the message and sending to the client side. Here i am able to send the data to the client and display it on the client. But when i am adding header fields in the client and server, client is showing core dump .Can anyone help me with the problem or can propose a better solution to display the data at the client side

Thanks

client.c

#include <stdio.h>
#include <string.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h> 
#include <stdlib.h>
#include <unistd.h>
#include <pthread.h>
#include <sys/syscall.h>
#include <netdb.h>

#define MAX_SIZE 100000
#define NUM_CLIENT 5
void *connection_handler(void *);

int main(int argc , char *argv[])
{
   int sockfd;
   long i;
   pthread_t sniffer_thread;
   printf("memory allocated for the creation of socket \n");
   for (i=1; i<=NUM_CLIENT; i++) {
        if( pthread_create( &sniffer_thread , NULL ,  connection_handler , (void *) i) < 0)
    {
        perror("could not create thread");
        return 1;
    } 
    printf("Thread created \n");
    sleep(2); 
} 
pthread_exit(NULL);
return 0;
}
void *connection_handler(void *threadid)
{   
    struct head
    {
       float version;
       int   msg_length;
       int   header_length;
       char  msg_type[50];
    };

    long etid; /*each thread id */
    etid = (long)threadid;
    pthread_t tid;
    tid = pthread_self();
    int sockfd , n;
    struct sockaddr_in serv_addr;
    struct hostent *server;
    void *buf;
    buf=(struct head *)malloc((sizeof(struct head)));    
    char sbuff[MAX_SIZE] , rbuff[MAX_SIZE];                             
    if((sockfd = socket(AF_INET, SOCK_STREAM, 0)) < 0){
    printf("Failed creating socket\n");
    perror("could not create thread");
    return 0;
    }
    memset(&serv_addr, 0 , sizeof (serv_addr));
    printf("the bytes of memory area pointed by serv_addr is filled with constant '0' \n");
    serv_addr.sin_family = AF_INET;
    bcopy((char *)server->h_addr, 
     (char *)&serv_addr.sin_addr.s_addr,
     server->h_length);
    serv_addr.sin_port = htons(8888);

    if (connect(sockfd, (struct sockaddr *)&serv_addr, sizeof (serv_addr)) < 0) {
         perror("Failed to connect to server. Error");
         return 0;
         }
    n = read(sockfd,buf,((sizeof(struct head))));
    printf("The socket contains %d \n ",n);
    if (n < 0) 
     perror("ERROR reading from socket");
     printf("version -->%f \n msg length -->%d \n header length %d \n  msg type--> %s  \n ",*(float*)buf,*(int*)((unsigned int)buf+sizeof(float)),*(int*)((unsigned int)buf+sizeof(float)+sizeof(int)),(char*)((unsigned int)buf+sizeof(float)+3*sizeof(int)));
    printf("Connected successfully client:%ld \n", tid);
    printf("before calling pthread_create getpid: %d getpthread_self: %lu tid:%lu\n",getpid(), pthread_self(), syscall(SYS_gettid)); 
    while(1)
    {
       printf("For thread : %ld\n", etid);
       printf("thread id given by kernel: %ld\n", tid);
       fgets(sbuff, MAX_SIZE , stdin);
       send(sockfd,sbuff,strlen(sbuff),0);

        if(recv(sockfd,rbuff,MAX_SIZE,0)==0)
           printf("Error");
       else
          fputs(rbuff,stdout);
       bzero(rbuff,MAX_SIZE);
       sleep(2);
       } 
       close(sockfd);
       return 0;
}

server.c

 #include <stdio.h>
 #include <string.h>
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <pthread.h>
 #include <sys/syscall.h>
 #include<pthread.h> //for threading , link with lpthread
 #include <fcntl.h>

 #define PROCESSLIST "/tmp/k"
 struct head
 {
     float version;
     int msg_length;
     int header_length;
     char msg_type[50];
 };
 void *connection_handler(void *);
 int main(int argc , char *argv[])
 {
     int sockfd , new_socket , c , *new_sock;
     struct sockaddr_in serv_addr , client;
     char *message;
     void *msgptr;
     struct head * hdptr;
     int n;
     hdptr=(struct head *)malloc(sizeof(struct head));
     msgptr=malloc((sizeof(struct head)));
     printf("msgptr1 %u  size = %d\n",(unsigned int) msgptr,sizeof(struct head));
     ((struct head *)msgptr)->version=5.01;
     ((struct head *)msgptr)->msg_length=1234;
     ((struct head *)msgptr)->header_length=2;
     strcpy((((struct head *)msgptr)->msg_type),"process list, directory list, OS Name and Version");
     ((struct head *)msgptr)->flag=0;
     printf("intialized \n");
     printf("version -->%f \n msg length -->%d \n header length %d \n  msg type--> %s  \n   ",*((float *)msgptr),*((int *)((unsigned int)msgptr+sizeof(float))),*((int *)((unsigned int)msgptr+2*sizeof(int))),(char *)((unsigned int)msgptr+3*sizeof(int)));

     //Create socket
     sockfd = socket(AF_INET , SOCK_STREAM , 0);
     if (sockfd == -1)
     {
         printf("Could not create socket");
     }

     //Prepare the sockaddr_in structure
     server.sin_family = AF_INET;
     server.sin_addr.s_addr = INADDR_ANY;
     server.sin_port = htons( 8888 );

     //Bind
     if( bind(sockfd,(struct sockaddr *)&server , sizeof(server)) < 0)
    {
       puts("bind failed");
       return 1;
     }
     puts("bind done");

     //Listen
     listen(sockfd , 3);

    //Accept and incoming connection
    puts("Waiting for incoming connections...");
    c = sizeof(struct sockaddr_in);
    while( (new_socket = accept(sockfd, (struct sockaddr *)&client, (socklen_t*)&c)) )
    {
        puts("Connection accepted");

        pthread_t sniffer_thread;
        new_sock = malloc(1);
       *new_sock = new_socket;

      if( pthread_create( &sniffer_thread , NULL ,  connection_handler , (void*) new_sock) < 0)
      {
        perror("could not create thread");
        return 1;
       }
    puts("Handler assigned");
   }

   if (new_socket<0)
   {
    perror("accept failed");
    return 1;
    }

    return 0;
}

  /* This will handle connection for each client */
void *connection_handler(void *sockfd)
{
       //Get the socket descriptor
      int sock = *(int*)sockfd;
      int read_size;
      char message[100000], client_message[2000];
      int fd;
      void *buff;
      //Receive a message from client
     while( (read_size = recv(sock , client_message , 2000 , 0)) > 0 )
     {
        system ("ps -ef > /tmp/k");
        puts("temp file getting created");
        fd = open (PROCESSLIST, O_RDONLY);
        puts("file open has taken place");
        read(fd, buff, sizeof(*buff));
        puts("reading the file");
        write(sock, buff, 1);
        puts("writing to the buffer");
        puts("copied data from buffer to message");
        //Send the message back to client
        send(sock,message,strlen(message),0);
        puts("data has been sent");
        }

       if(read_size == 0)
       {
       puts("Client disconnected");
       fflush(stdout);
       }
       else if(read_size == -1)
       {
          perror("recv failed");
        }

//Free the socket pointer
free(sockfd);

return 0;
}
nandan
  • 133
  • 1
  • 8
  • You don't need to [cast `malloc()`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). And you can declare `msgptr` as `strcut head` it doesn't matter if you return `void *` you don't even need to cast it. This `((struct head *)msgptr)->version=5.01` is completely unnecessary. And you know, compilers ignore white spaces, so you could write more readable code if you want to. – Iharob Al Asimi Sep 22 '15 at 16:06
  • `(struct head *)malloc(sizeof(struct head))` -> `malloc(sizeof(*hdptr))` is ok. – Iharob Al Asimi Sep 22 '15 at 16:08
  • Have you allocated any space the `server` pointer points to in `connection_handler` from `client.c`? – cadaniluk Sep 22 '15 at 16:10
  • But the problem is at client side@iharob server is running fine. – nandan Sep 22 '15 at 16:10
  • In general, just be consistent, the `*` goes near the type name, or in the middle or near the identifier? Y have a combination. Be consistent or, you will regret it in the long term. Also, that helps you find your own mistakes quickly. – Iharob Al Asimi Sep 22 '15 at 16:10
  • @nandan It doesn't matter, I am trying to point out common practices that are old or unnecessary just to inform you, it's evident that you can write programs you just need to learn some good practices that could probably prevent the current problem completely. Did you know that `malloc()` returns `NULL` on failure? If so, why didn't you check for it? Also, don't mix declarations with code, it's hard to track the `server` variable in your code because of the mess. – Iharob Al Asimi Sep 22 '15 at 16:12

1 Answers1

1

The main problem is you have used indirection on an uninitialized pointer.

This

struct hostent *server;

is almost immediately followed by

bcopy(server->whatever
/*           ^ this is wrong, because `server' is not initialized */

It's really hard to see that in your mess (sorry I mean, code). You need to organize your code better. And check for errors to make it robust.

Here is a slight improvement on your code, and I also fixed the issue

#include <stdio.h>
#include <string.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <stdlib.h>
#include <unistd.h>
#include <pthread.h>
#include <sys/syscall.h>
#include <netdb.h>

#define MAX_SIZE 100000
#define NUM_CLIENT 5
void *connection_handler(void *);

int main(int argc , char *argv[])
{
    long i;
    pthread_t sniffer_thread;

    printf("memory allocated for the creation of socket \n");
    for (i = 1 ; i <= NUM_CLIENT ; i++) /* Be consistent, either break braces or don't */
    {
        if (pthread_create(&sniffer_thread , NULL,  connection_handler, (void *) &i) < 0)
        {
            perror("could not create thread");
            return 1;
        }
        printf("Thread created \n");
        sleep(2);
    }
    pthread_exit(NULL);

    return 0;
}
void *connection_handler(void *threadid)
{
    struct head
    {
        float version;
        int   msg_length;
        int   header_length;
        char  msg_type[50];
    };
    long etid; /*each thread id */
    pthread_t tid;
    int sockfd;
    int n;
    struct sockaddr_in serv_addr;
    struct hostent server;
    /*            ^ you don't need a pointer */
    void *buf;
    char sbuff[MAX_SIZE];
    char rbuff[MAX_SIZE];

    tid = pthread_self();
    etid = *(long *) threadid;
    buf = malloc((sizeof(struct head)));
    if((sockfd = socket(AF_INET, SOCK_STREAM, 0)) < 0)
    {
        printf("Failed creating socket\n");
        perror("could not create thread");
        return 0;
    }
    memset(&serv_addr, 0 , sizeof (serv_addr));

    printf("the bytes of memory area pointed by serv_addr is filled with constant '0' \n");

    serv_addr.sin_family = AF_INET;
    serv_addr.sin_port = htons(8888);

    bcopy((char *) server.h_addr, (char *) &serv_addr.sin_addr.s_addr, server.h_length);

    if (connect(sockfd, (struct sockaddr *)&serv_addr, sizeof (serv_addr)) < 0)
    {
        perror("Failed to connect to server. Error");
        return 0;
    }

    n = read(sockfd,buf,((sizeof(struct head))));
    printf("The socket contains %d \n ",n);
    if (n < 0)
        perror("ERROR reading from socket");
    printf(
        "version -->%f \n msg length -->%d \n header length %d \n  msg type--> %s  \n ",
        *(float *) buf,
        *(int *) ((unsigned int *) buf + sizeof(float)),
        *(int *) ((unsigned int *) buf + sizeof(float) + sizeof(int)),
        (char *) ((unsigned int *) buf + sizeof(float) + 3 * sizeof(int))
         /*                    ^ you need to cast to a pointer type */
    );

    printf("Connected successfully client:%ld \n", tid);
    printf("before calling pthread_create getpid: %d getpthread_self: %lu tid:%lu\n",
        getpid(), pthread_self(), syscall(SYS_gettid));

    while (1)
    {
        printf("For thread : %ld\n", etid);
        printf("thread id given by kernel: %ld\n", tid);

        fgets(sbuff, MAX_SIZE , stdin);
        send(sockfd,sbuff,strlen(sbuff),0);

        if(recv(sockfd,rbuff,MAX_SIZE,0)==0)
            printf("Error");
        else
            fputs(rbuff,stdout);
        bzero(rbuff,MAX_SIZE);
        sleep(2);
    }
    close(sockfd);
    return 0;
}

The server variable is delcared as a pointer, but at the time you dereference it it's not yet pointing to anything.

Since it's uninitialized the behavior is undefined when you dereference it. Also, you should be careful when casting pointers, it's not trivial and you already had mistakes like this

*(int *) ((unsigned int) buf + sizeof(float) + sizeof(int))

You are casting buf to an integer and there is a chance that the conversion truncates the pointer for example on x86_64 platforms. With compiler warnings enabled the compiler would warn about pointer to integer conversion.

Iharob Al Asimi
  • 52,653
  • 6
  • 59
  • 97