Trying to use a queue to handle client messages on a server [C]

Question

Im trying to write a server in c that allows clients connected to send messages to each other. I tried doing this by receiving the message, placing it on a queue and have the process that handles the server-client communication (function dostuff) deliver that message if the username matches with the message.

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

/******Struct msg*********/
typedef struct msg
{
    char dest  [64]; 
    char origin[64];  
    char text  [256]; 
}msg;
/*********Queue*********/
/*Queue - Linked List implementation*/
#include<stdio.h>
#include<stdlib.h>
struct Node {
    msg *data;
    struct Node* next;
};
struct Node* front = NULL;
struct Node* rear = NULL;
void Enqueue(msg *x) {
    struct Node* temp = 
        (struct Node*)malloc(sizeof(struct Node));
    temp->data =x; 
    temp->next = NULL;
    if(front == NULL && rear == NULL){
        front = rear = temp;
        return;
    }
    rear->next = temp;
    rear = temp;
}
void Dequeue() {
    struct Node* temp = front;
    if(front == NULL) {
        printf("Queue is Empty\n");
        return;
    }
    if(front == rear) {
        front = rear = NULL;
    }
    else {
        front = front->next;
    }
    free(temp);
}
msg *Front() {
    if(front == NULL) {
        return 0;
    }
    return front->data;
}
void error(const char *msg)
{
    perror(msg);
    exit(1);
}

int interpret(char *line, char username[])
{
    //printf("Ready to interpret: %s\n",line);

    /*  
    char *command, *arg1 , *arg2,l[64];
    msg *message;
    strcpy(l,line);
    command = strtok(l," ");

    if(strcmp(command, "/tell") == 0)
    {
        printf("The command was vaid!\n");//chegou aqui
        printf("Ready to interpret: %s\n",l);
        arg1 = strtok(NULL," ");
        printf("I got this far!");
        arg2 = l + strlen(command) + strlen(arg1) + 2; //somamos dois por causa dos espaços

        message = (msg*)malloc(sizeof(msg));

        //printf("I got this far!"); nao está a chegar aqui

        strcmp(message->dest, arg1);
        strcmp(message->origin, username);
        strcmp(message->text, arg2);
        Enqueue(message);
        printf("(%s -> %s) -%s- was placed on the queue",message->origin,message->dest,message->text);
    }
    else
    {
        printf("comando inválido");
    }

    */
        //this is temporary because there is an error with the code above which made the message not be correct
        msg *message;
        message = (msg*)malloc(sizeof(msg));

        strcmp(message->dest, "dest");
        strcmp(message->origin, "origin");
        strcmp(message->text, "blahblahblah");
        Enqueue(message);

    return 1;
}
void dostuff (int sock, char username[])
{
    int n, pid;
    char buffer[256], dest[64]; 
    pid = fork();
    while(1)
    {
        if(pid > 0)
        {
            bzero(buffer,256);
            n = read(sock,buffer,255);

            if (n < 0) error("ERROR reading from socket");

            printf("Here is the full message: %s\n",buffer);

            //n = write(sock,"I got your message!\n",18);
            //if (n < 0) error("ERROR writing to socket");

            interpret(buffer, username);
        }
        else
        {
            sleep(1); 
            if(Front() != NULL)
            {
                strcpy(dest, Front()->dest);
                if(strcmp(dest, username) == 0)
                {
                    strcpy(buffer, Front()->text);
                    Dequeue();
                    n = write(sock,buffer,strlen(buffer));
                    if (n < 0) error("ERROR writing to socket");
                }
            }
        }   
    }
}
int main(int argc, char *argv[])
{
     int sockfd, newsockfd, portno, pid , f;
     int userCount;                                                       
     socklen_t clilen;

     struct sockaddr_in serv_addr, cli_addr;                                

     if (argc < 2)
     {
         fprintf(stderr,"ERROR, no port provided\n");
         exit(1);
     }

     sockfd = socket(AF_INET, SOCK_STREAM, 0);                            
     if (sockfd < 0) 
        error("ERROR opening socket");

     bzero((char *) &serv_addr, sizeof(serv_addr));                            

     portno = atoi(argv[1]); 

     serv_addr.sin_family = AF_INET;                                           
     serv_addr.sin_addr.s_addr = INADDR_ANY;                                   
     serv_addr.sin_port = htons(portno);                                      

     if (bind(sockfd, (struct sockaddr *) &serv_addr,
              sizeof(serv_addr)) < 0) 
              error("ERROR on binding");

     listen(sockfd,5);                                                        
     clilen = sizeof(cli_addr);

     userCount = 0;
     char dname[64];                                                          
     f = fork();

     while (1) 
     {
        if(f > 0)
        {
            newsockfd = accept(sockfd,(struct sockaddr *) &cli_addr, &clilen);  
            if (newsockfd < 0) error("ERROR on accept");

            userCount++;                                                        
            sprintf(dname, "%d", userCount);                                    

            pid = fork();
            if (pid < 0) error("ERROR on fork");  
            if (pid == 0)
            {
                close(sockfd);
                dostuff(newsockfd, dname);
                exit(0);
            }
            else close(newsockfd);

        }
    }

    close(sockfd);
    return 0;
}

The problem seems to be that this piece of code that is supposed to remove items from the queue :

if(Front() != NULL)
            {
                strcpy(dest, Front()->dest);
                if(strcmp(dest, username) == 0)
                {
                    strcpy(buffer, Front()->text);
                    Dequeue();
                    n = write(sock,buffer,strlen(buffer));
                    if (n < 0) error("ERROR writing to socket");
                }
            }

Is never being run. I never made anything like this before, so this approach might be totally wrong. If someone could explain me what I'm doing wrong, or suggest a different solution, that would be great.

Try to print the value of `front` or use debugger. The problem is that `front` seems to be NULL. And thus the function is never being run. — Mirakurun, Jun 01 '16 at 09:52
@Mirakurun I was printing a message if the queue was empty after I added an element, and it seems the queue isn't empty at that point, but it still doesn't pass that conditional. I removed the print statements in the op because I thought it would make it more readable — Lewis, Jun 01 '16 at 09:59
I thing you have a typo: all `strcmp` should be `strcpy` in `interpet` function body. — LPs, Jun 01 '16 at 10:03
Try to change `if(Front() != NULL)` into `if(Front() ==NULL)` just to see if the function would be called in that case. If yes, then `front` is definitely null. — Mirakurun, Jun 01 '16 at 10:06
@LPs that was a typo, thanks. I tested it again and the interpret function isn't being run at all — Lewis, Jun 01 '16 at 10:19
Excuse me if I'm going to say something too basic, but: do you have a client side code? — LPs, Jun 01 '16 at 10:23
Yes, I have the client code, but it doesn't seem to be a client side problem. The message is getting to the server just fine, the server just isn't delivering the message — Lewis, Jun 01 '16 at 10:54

LPs · Accepted Answer · 2016-06-01T12:31:30.890

0

You are misunderstanding how globals are managed after fork().

They are not shared; the processes can modify their own copies of the variables. Those modifications will not be notified to other childs/parents.

So what you are trying to do with Enqueue makes change to the local copy of front pointer but other childs copies of front pointer are not updated.

Reducing your code to minimum:

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

/******Struct msg*********/
typedef struct msg
{
    char dest  [64];
    char origin[64];
    char text  [256];
}msg;
/*********Queue*********/
/*Queue - Linked List implementation*/
#include<stdio.h>
#include<stdlib.h>
struct Node {
    msg *data;
    struct Node* next;
};
struct Node* front = NULL;
struct Node* rear = NULL;

void Enqueue(msg *x)
{
    struct Node* temp = malloc(sizeof(struct Node));

    temp->data =x;
    temp->next = NULL;

    if(front == NULL && rear == NULL)
    {
        front = rear = temp;
        printf("front: %p\n", (void *)(front));
        printf("rear: %p\n" , (void *)(rear));
        printf("temp: %p\n" , (void *)(temp));
        return;
    }

    rear->next = temp;
    rear = temp;
}

void Dequeue(void) {
    struct Node* temp = front;
    if(front == NULL) {
        printf("Queue is Empty\n");
        return;
    }
    if(front == rear) {
        front = rear = NULL;
    }
    else {
        front = front->next;
    }
    free(temp);
}
msg *Front(void) {
    if(front == NULL) {
        return 0;
    }
    return front->data;
}
void error(const char *msg)
{
    perror(msg);
    exit(1);
}

int interpret(void)
{

    //this is temporary because there is an error with the code above which made the message not be correct
    msg *message;
    message = (msg*)malloc(sizeof(msg));

    strcpy(message->dest, "dest");
    strcpy(message->origin, "origin");
    strcpy(message->text, "blahblahblah");
    Enqueue(message);

    return 1;
}

void dostuff (void)
{
    pid_t pid;

    pid = fork();

    if(pid > 0)
    {
        interpret();
    }
    else
    {
        sleep(1);
        printf("\nfront: %p\n", (void *)(front));
    }
}
int main(int argc, char *argv[])
{
    pid_t f, pid;

    f = fork();

    if(f > 0)
    {
        pid = fork();

        if (pid < 0) 
           error("ERROR on fork");

        if (pid == 0)
        {
            dostuff();
        }
    }

    return 0;
}

Output will be

front: 0x18371a0
rear: 0x18371a0
temp: 0x18371a0
root@mypc:~$
front: (nil)

edited Jun 01 '16 at 12:31

answered Jun 01 '16 at 09:53

LPs

16,045
8
30
61

Thank you for your explanation. I will try using threads so that each instance changes the same variable – Lewis Jun 01 '16 at 13:23
Hm concurrency ..? :/ Im not sure I know what that means... is it the program running the threads out of order? Is there an alternative way to achieve what I'm trying to do? I had never used forks before, and I thought the memory was the same, only the instructions changed from process to process.... – Lewis Jun 01 '16 at 13:40
I think you should study something before start typing... Using multithreading solution you have code that is executed "at the same time" so accessing a global variable must be well managed. [This is a starting guide]. But, i repeat, you need to study something about before to start with a so difficult stuff. – LPs Jun 01 '16 at 13:44
Thank you for being so patient, Im going to read more on threads before trying to fix the code. Just one more question, is this way of writing a message server acceptable? I couldn't find any examples online and just tried something on my own, should I look into other solutions or is this ok? – Lewis Jun 01 '16 at 14:12
It is a very broad matter. You can do that using fork, threads or with 2 compiled different application. It depends on what are you looking for. [This is a very plain and linear solution](http://www.tutorialspoint.com/unix_sockets/socket_server_example.htm) that does not involve fork or threads. – LPs Jun 01 '16 at 14:18
I read that one when I started this assignment, the problem is that I need to have client - client conversations, and that example only provides client server conversations. If there is a way to do that without using forks or threads that would be wonderful, since I never learned about that stuff in classes and I'm trying to keep it simple... this was the only way I thought I could treat multiple connections – Lewis Jun 01 '16 at 14:29
Well. it totally depends on many things. While using fork is "complicated" but force you to code taking care of "context", Multi-threading is simpler but concurrency is an hard matter. So choose one option and try to do your best. SO will help you with your problems. ;) – LPs Jun 01 '16 at 14:46
[Someone posted a good question on concurrency just now](http://stackoverflow.com/questions/37569571/is-a-mutex-necessary-when-only-inc-decrementing-a-variable). Take a look. – LPs Jun 01 '16 at 14:49
1

Thanks for the link, I read that and a bunch of other stuff, so I think it comes down to either learning to lock memory with mutex, so I don't get 2 threads trying to interact with the queue or I learn mmap() to make different forked processes share the queue... either way, gotta read and study some more :p – Lewis Jun 01 '16 at 16:18
That's the way. Good luck ;) – LPs Jun 01 '16 at 16:58

score 0 · Answer 2 · answered Jun 01 '16 at 12:58

0

Use threads instead of fork. Start two threads. One for listening and one for answering.

Fork creates new OS process that shares the same code but data segments are different. So what you change in one process will not be seen in another.

answered Jun 01 '16 at 12:58

Beka

97
5

Trying to use a queue to handle client messages on a server [C]

2 Answers2