Thread: fork()

so i have a client server program, and everything works perfectly, but now i have to modify my server code to allow for multiple clients, im not really sure if im using fork() correctly as i really dont understand what fork() does.

ive tried googling for about an hour and still dont understand it. although my program still runs im not sure if im testing it correctly since it runs no differently than before i added fork(),(other than the fact that typing q in my client no longer makes the server program end).

Code:

#include <sys/types.h>

#include <sys/socket.h>

#include <netinet/in.h>

#include <arpa/inet.h>

#include <stdio.h>

#include <stdlib.h>

#include <errno.h>

#include <string.h>

#include <ctype.h>



#define  PORT	3132		//port for server being used

main(){

	int rc;			//system cals return value

	int s;			//socket descriptor
	int connected;		//new connection's socket descriptor

	int bytes_received;	//number of bytes received

	int size_csa;		//size of clients address struct

	int pid;

	int i;			//iterator

	char received[1024];	//array that stores received string

	char changed[1024];	//array to store the changes of the received data

	struct sockaddr_in sa;	//internet address struct for server

	struct sockaddr_in ca;	//internet address struct for client

	

        s = socket(AF_INET, SOCK_STREAM, 0);

	//checks for an error with socket()

	if (s < 0){

		perror("Socket");

		exit(1);

	}



	sa.sin_family = AF_INET;	//using internet address family

	sa.sin_port = htons(PORT);	//copy port number in network byte order

	sa.sin_addr.s_addr = INADDR_ANY;	//INADDR_ANY = Wild Card



	//checks for an error with socket()

	rc = bind(s, (struct sockaddr *)&sa, sizeof(sa));

	if (rc){

		perror("bind");

		exit(1);

	}

	

	//checks to see if there is an error with listen()

	rc = listen(s,5);

	if (rc){

		perror("listen");

		exit(1);

	}

	

        while(1){

		//accept a connection using the address of the client

		size_csa = sizeof(struct sockaddr_in);

		connected = accept(s, (struct sockaddr *)&ca, &size_csa);
		if (connected < 0){
			perror("connect");
			exit(1);
		}

		pid = fork();
		if(pid < 0){
			printf("fork error\n");
			exit(1);
		}
		if(pid == 0){
			while(1){

				//clears the arrays received and changed

				for(i = 0; i < 1024; i++){

					received[i] = '\0';

					changed[i] = '\0';

				}

				//adds a null terminator to the end of the received data

				bytes_received = recv(connected,received,1024,0);

				received[bytes_received] = '\0';

	
				//changes the data in the array received to store data with all caps

				for(i = 0; i < strlen(received); i++)

					received[i] = toupper(received[i]);

	
				//takes the data from the array received in reverse order and stores it into the array changed 

				for(i = 0; i < strlen(received); i++)

					changed[i] = received[strlen(received)-i-1];

	
				//if the string that was received was "q" the program ends, otherwise it continues

				if (strcmp(changed , "Q\n") == 0){

					close(connected);

					break;

				}

			

				else

					send(connected, changed,strlen(changed), 0);  

			}//end while loop
		}//end if statement
		else
			continue;

	}//end while loop

	close(s);

	return 0;

}//end function main()

1) Am i using fork() correctly? if not can someone explain it to me?
2) when testing this in a linux terminal how can i test this program to make sure fork is working correctly?

EDIT: if necessary i can post my client code

I think to address anduril462's concern over open descriptors, you could begin your child process with
close(s);

Also, start to think about program structure as well. The whole inner while loop should be carved out into another function.

Code:

		pid = fork();
		if(pid < 0){
			printf("fork error\n");
			exit(1);
		}
		if(pid == 0){
			close(s);  // child doesn't want this
			handleNewClient(connected);
		} else {
			close(connected);  // parent doesn't want this
		}

As a rough "rule of thumb", if you can't see the outermost open/close braces of a function on screen at the same time, then it is almost certainly too long.
Another guide is to write down what the function does. Normally, it should be "This function does ....". If you find yourself writing "This function does ... and .... and ...", then that's another sign you should be at least thinking about a bit of reorganisation.

Finally, a bug

Code:

	bytes_received = recv(connected,received,1024,0);
	received[bytes_received] = '\0';

1. The size parameter passed to recv() should be sizeof(received)-1
The -1 is important, as it ensures there is always room to append a \0 to make it a string.
2. ALWAYS check the return result before appending the \0 - it will be negative on error (and negative array subscripts are bad voodoo!)

Well the basic idea is that when accept() returns, you now have TWO connections that need processing. There are various ways of dealing with this, of which fork() is just one of them.

So after a fork(), you have two processes, to handle two connections; so one process takes one connection, and the other process takes the other connection.

By convention, the parent process keeps 's', because this is assumed to be a persistent socket, and the spawned child process takes 'connected' because this will close when the remote disconnects.

> if that is the case should i be calling handleNewClient(s) in the parent part?
No - your outer while loop is just responsible for calling accept() to detect new connections. When there is a new connection, you call fork() to create a separate process to handle just that connection (and nothing else).

Compare your "while accept" loop as being the mail room guy in a company. He doesn't open letters, he just opens the bag and figures out where to send each letter to inside the company. The employees who get the letters are the forked processes who actually get to deal with it.

> They think nothing of 1000-line functions.
Show them this -> Cyclomatic complexity - Wikipedia, the free encyclopedia
Then ask them how many paths exist in such large functions, and how well they think they're being tested.

Or even just run gcov over such large functions with your real tests to see how inadequate it all is.