Thread: Unix: Server does not works as intened

Hey,

given this code, I coded many mistakes:

Code:

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


#define BUFLEN 255
#define MAX_CONNECTIONS 128


void* job_read(void * p);
void* job_write(void * p);


int socket_ids[MAX_CONNECTIONS];
char endprogramm = 0;
int open_cnncts;
pthread_mutex_t mutex;


void error(const char* msg){
    perror(msg);
    exit(1);
}


int main(int argc, char* argv[]) {
    if(argc < 2){
        fprintf(stderr, "You must provide a port number");
        exit(EXIT_FAILURE);
    }
    int sockfd, portnum;
    
    if(pthread_mutex_init(&mutex, NULL)<0){
        error("Could not initialize Mutex");
    }
    pthread_t readthreads[MAX_CONNECTIONS];
    pthread_t writethreads[MAX_CONNECTIONS];
    
    struct sockaddr_in serv_add, cli_adr;
    socklen_t clilen;
    
    sockfd = socket(AF_INET, SOCK_STREAM, 0);
    if(sockfd < 0){
        error("Error opening socket.");
    }
    bzero((char*)&serv_add, sizeof(struct sockaddr_in));
    portnum = atoi(argv[1]);
    
    serv_add.sin_family = AF_INET;
    serv_add.sin_addr.s_addr = INADDR_ANY;
    serv_add.sin_port = htons(portnum);
    
    if(bind(sockfd, (struct sockaddr*) (&serv_add), sizeof(serv_add)) < 0){
        error("Binding failed.");
    }
    


    for(open_cnncts = 0; !endprogramm; open_cnncts++){
        listen(sockfd, MAX_CONNECTIONS);
        clilen = sizeof(cli_adr);
        
        int newsockfd = accept(sockfd, (struct sockaddr*) &cli_adr, &clilen);
        printf("Client connected.\n");
        socket_ids[open_cnncts] = newsockfd;
        pthread_t read_thread;
        pthread_t write_thread;
        pthread_create(&read_thread, NULL, job_read((void*)&newsockfd), NULL);
        pthread_create(&write_thread, NULL, job_write((void*)&newsockfd), NULL);
        
        readthreads[open_cnncts] = read_thread;
        writethreads[open_cnncts] = write_thread;
        
    }
    close(sockfd);
    for(; open_cnncts != 0; open_cnncts--){
        close(socket_ids[open_cnncts]);
        pthread_join(writethreads[open_cnncts], NULL);
        pthread_join(readthreads[open_cnncts], NULL);
    }
    pthread_mutex_destroy(&mutex);
    return 0;


}


void* job_read(void * p){
    int* socketp = (int*)p;
    int newsockfd = (*socketp);
    size_t n;
    char buffer[BUFLEN];
    while(!endprogramm){
        bzero(buffer, BUFLEN);
        n = read(newsockfd, buffer, BUFLEN);
        if(!n){
            error("Reading Failed");
        }
        printf("Client: %s\n", buffer);
        
        pthread_mutex_lock(&mutex);
        for(int i = 0; i < open_cnncts; i++){
            n = write(socket_ids[i], buffer, strlen(buffer));
            if(n < 0){
                error("Writing failed");
            }
        }
        pthread_mutex_unlock(&mutex);
    }
    return NULL;
}


void* job_write(void * p){
    int* socketp = (int*)p;
    int newsockfd = (*socketp);
    size_t n;
    char buffer[BUFLEN];
    while(!endprogramm) {
        bzero(buffer, BUFLEN);
        fgets(buffer, BUFLEN, stdin);
        n = write(newsockfd, buffer, strlen(buffer));
        if(n < 0){
            error("Writing failed");
        }
        int i = strncmp("Bye", buffer, 3);
        if(i == 0){
            break;
        }
    }
    endprogramm = 1;
    return NULL;
}

This code just does not yet work.
The first phenomenon I see here:

Code:

 for(open_cnncts = 0; !endprogramm; open_cnncts++){        listen(sockfd, MAX_CONNECTIONS);
        clilen = sizeof(cli_adr);
        
        int newsockfd = accept(sockfd, (struct sockaddr*) &cli_adr, &clilen);
        printf("Client connected.\n");
        socket_ids[open_cnncts] = newsockfd;
        pthread_t read_thread;
        pthread_t write_thread;
        pthread_create(&read_thread, NULL, job_read((void*)&newsockfd), NULL);
        pthread_create(&write_thread, NULL, job_write((void*)&newsockfd), NULL);
        
        readthreads[open_cnncts] = read_thread;
        writethreads[open_cnncts] = write_thread;
        
    }

"Client connected.", does not get printed, but the Server is able to receive messages from the client. However unable to send messages. When the server, however, receives a message, the message and "Client connected" gets printed...

What would your idea be to this particular bug.

Here is the Client:

'Untitled Post' | TextUploader.com

EDIT: I have spotted the bug.

Code:

for(open_cnncts = 0; !endprogramm; open_cnncts++){
        listen(sockfd, MAX_CONNECTIONS);
        clilen = sizeof(cli_adr);

int newsockfd = accept(sockfd, (struct sockaddr*) &cli_adr, &clilen);
        printf("Client connected.\n");
        socket_ids[open_cnncts] = newsockfd;
        pthread_t read_thread;
        pthread_t write_thread;
        pthread_create(&read_thread, NULL, job_read((void*)&newsockfd), NULL);
        pthread_create(&write_thread, NULL, job_write((void*)&newsockfd), NULL);

        readthreads[open_cnncts] = read_thread;
        writethreads[open_cnncts] = write_thread;

    }

Replace with:

Code:

for(open_cnncts = 0; !endprogramm; open_cnncts++){
        listen(sockfd, MAX_CONNECTIONS);
        clilen = sizeof(cli_adr);

int newsockfd = accept(sockfd, (struct sockaddr*) &cli_adr, &clilen);
        printf("Client connected.\n");
        socket_ids[open_cnncts] = newsockfd;
        pthread_t read_thread;
        pthread_t write_thread;
        pthread_create(&read_thread, NULL, job_read((void*)&socket_ids[open_cnncts]), NULL);
        pthread_create(&write_thread, NULL, job_write((void*)&socket_ids[open_cnncts]), NULL);

        readthreads[open_cnncts] = read_thread;
        writethreads[open_cnncts] = write_thread;

    }

However this does not make much sense, why this fixes the first bug?

These lines:

Code:

pthread_create(&read_thread, NULL, job_read((void*)&newsockfd), NULL); 
pthread_create(&write_thread, NULL, job_write((void*)&newsockfd), NULL);

Are WRONG. They should be:

Code:

pthread_create( &read_thread, NULL, job_read, &newsockfd );
pthread_create( &write_thread, NULL, job_write, &newsockfd );

Notice the 3rd argument... You are calling the function, but pthread_create requires a pointer to a function. The way it is pthread_create is receiving NULL after the function returns and, since you aren't checking for errors, no threads are created.