# Server client for distributed system

This is the code I am using for a distributed system. The code will be used to allow the server to send parts of a (really big) document which will be processed by the connected clients. Please tell me what things would make it more efficient and if there are any security issues that I can correct without addition of an auth library.

#include<stdio.h>
#include<string.h> //strlen
#include<stdlib.h> //strlen
#include<sys/socket.h>
#include<unistd.h> //write

void *connection_handler(void *);

int main(int argc , char *argv[]) {
int socket_desc , client_sock , c;
//Create socket
socket_desc = socket(AF_INET , SOCK_STREAM , 0);
if (socket_desc == -1) {
printf("Could not create socket");
}
puts("Socket created");
server.sin_family = AF_INET;
server.sin_port = htons( 8888 );
//Bind
if( bind(socket_desc,(struct sockaddr *)&server , sizeof(server)) < 0) {
//print the error message
perror("bind failed. Error");
return 1;
}
puts("bind done");
//Listen
listen(socket_desc , 3);
//Accept and incoming connection
puts("Waiting for incoming connections...");

while( (client_sock = accept(socket_desc, (struct sockaddr *)&client, (socklen_t*)&c)) ) {
puts("Connection accepted");
if (pthread_create( &thread_id , NULL , connection_handler , (void*) &client_sock) < 0) {
return 1;
}
//Now join the thread , so that we dont terminate before the thread
puts("Handler assigned");
}

if (client_sock < 0) {
perror("accept failed");
return 1;
}
return 0;
}
/*
* This will handle connection for each client
* */
void *connection_handler(void *socket_desc) {
//Get the socket descriptor
int sock = *(int*)socket_desc;
char *message , client_message[2000];
//Send some messages to the client
message = "Greetings! I am your connection handler\n";
write(sock , message , strlen(message));
message = "Now type something and i shall repeat what you type \n";

write(sock , message , strlen(message));
while( (read_size = recv(sock , client_message , 2000 , 0)) > 0 ) {
//end of string marker
puts (client_message);
//Send the message back to client
write(sock , client_message , strlen(client_message));
//clear the message buffer
memset(client_message, 0, 2000);
}

// system ("cd; chmod +x script.sh;./script");

puts("Client disconnected");
fflush(stdout);
}
perror("recv failed");
}
return 0;
}

• Looks like a copy-pasta error. Accept and incoming connection and the following line are duplicated. – Jerry Coffin Jan 22 '16 at 16:51
• sorry about that! the code doesn't show any error on runtime though. I have put the code for review not for error checking though. – kneelb4darth Jan 22 '16 at 16:56
• That particular duplication would cause a real problem, beyond the fact that it would print "Waiting for incoming connections..." twice as it started up. – Jerry Coffin Jan 22 '16 at 16:58

I'm not really into networking, but I can say a little about your coding style:

Missing return

if (socket_desc == -1) {
printf("Could not create socket");
}
puts("Socket created");


If socket_desc is equal to -1. Could not create socket will be printed as well as Socket created, which is misleading. I think:

if (socket_desc == -1) {
printf("Could not create socket");
return -1;                           /* or some other error code */
}
puts("Socket created");


is appropriate and the code doesn't proceeds.

Outsource
I'm also not sure if it would be better to outsource this part (as void prepareServer(sockaddr_in *server):

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


I don't know who else will read this code but comments like //print the error message followed by this perror("bind failed. Error");  will just make the code more obscure, since the goal of the error message is pretty obvious.
Also it's not really clear to me, why you need to call strlen(client_message) here
write(sock , client_message , strlen(client_message));

when you store assign read_size a few lines above in the while-condition. strlen will just search for a '\0', which you assign by yourself with read_size...