Good afternoon, all!

I'm in the process of writing a chat program. The server is completed, and if I simply telnet into the server with 2 different windows, 2 people can chat in fluid fashion.

However, when I connect 2 clients, the chat appears in lock-step fashion. (ie. Client 1 types a msg to Client 2, but Client 2 doesn't see the msg until Client2 types a message first.)

Here is a snippet of the code. Any pointers would be great!

And thank you in advance for any assistance!

- Jim

// Open an unbound TCP socket
     
      if ((sock = socket(AF_INET, SOCK_STREAM, 0)) < 0)
         pdie("Opening stream socket");

      
      // Prepare for connection to server

      bzero((char *) &server, sizeof(server));
      server.sin_family = AF_INET;
      if ((hp = gethostbyname(argv[1])) == NULL) 
      
      {
         sprintf(buf, "%s: unknown host\n", argv[1]);
         die(buf);
      }//end preparation

      bcopy(hp->h_addr, &server.sin_addr, hp->h_length);
      server.sin_port = htons((u_short) SERVER_PORT);
      
      // Establish connection to server

      if (connect(sock, (struct sockaddr *) &server, sizeof(server)) < 
0)
         pdie("Connecting stream socket");
      
      // What socket number am I talking on?

      clientLen = sizeof(client);
      if (getsockname(sock, (struct sockaddr *) &client, &clientLen))
         pdie("Getting socket name");
      
      if (clientLen != sizeof(client))
         die("getsockname() overwrote name structure");
      
      printf("Client socket has port %hu\n", ntohs(client.sin_port));


int name = ntohs(client.sin_port);
printf("Now the client name is %d\n\n", name);
gets (msg);
      
while (msg != "q")
{
      /* Write out message. */
      if (write(sock, msg, sizeof(msg)) < 0)
         pdie("Writing on stream socket");
      
      /* Prepare buffer and read from it. */
      bzero(buf, sizeof(buf));
      if (read(sock, buf, BUFFER_SIZE) < 0)
         pdie("Reading stream message");
      
      printf("User %d says: %s\n", name, buf);

gets(msg);      
}
      close(sock);
   

   exit(0);

}

Recommended Answers

All 6 Replies

Not surprisingly, but your program is doing what you told it to:

// Wait here for the user to enter a message
gets (msg);
      
while (msg != "q")
{
      /* Write out message. */
      if (write(sock, msg, sizeof(msg)) < 0)
         pdie("Writing on stream socket");
      
      /* Prepare buffer and read from it. */
      bzero(buf, sizeof(buf));
      if (read(sock, buf, BUFFER_SIZE) < 0)
         pdie("Reading stream message");
      
      printf("User %d says: %s\n", name, buf);

// Wait here for the user to enter a message
gets(msg);      
}

If you're wanting the client to be both waiting for user input and watching for messages from the server (other client) you will need to implement some form of multi-threading.

One implementation might have a thread that supports the socket and the main display of the chat. (It would add lines to the window as they were received or sent.) Another thread would support the user input, it would wait for the user to enter some data (without blocking the other thread) and then send the message the user entered to the other thread for sending to the server and adding to the chat window.

As an alternative to 'actual' multi-threading, you might be able to do something with a 'main loop' that would check for user input and check for socket data repeatedly. Something like the following:

loop forever
    if there is user input data available:
        if the user input is an 'enter':
            if the input buffer contains 'q':
                break the loop
            send the input buffer
        else if the user input is a 'backspace':
            if the input buffer is not empty:
                remove the last character from the input buffer
        else if the user input is a printable character:
            add the character to the input buffer
    if the socket is closed:
        break the loop
    if the socket has data:
        read the data from the socket
        display the data to the user
commented: Multi-threading idea sounds great! +1

You do not need multi-threading for this task.

Your program proceeds in lockstep because your I/O calls are of the "blocking" type and you are calling them without knowing that they are "ready". If they are not ready, they'll wait until something is ready to return. This includes fgets, which waits until the Enter key is pressed to return (line-buffered input).

You have 4 I/O statements in the loop, a simplified version of which appears below (error handling removed):

for ( ; ; ) {

    gets ( msg );   /* IO_1 */
    if ( strcmp ( msg, "q" ) == 0 ) break;

    write ( ... );  /* IO_2 */

    read ( ... );   /* IO_3 */
    printf ( ... ); /* IO_4 */
  }

Instantly, the gets() blocks reading from the terminal, waiting for an Enter keypress. If the other client had sent us a message, we won't see it until we send our message. To fix that you must read the keyboard in a non-blocking manner and yet avoid active polling.

You avoid active polling with the select() function, which will block until one of a given set of file descriptors are "ready". Blocking is good because it means you're not looping and testing (active polling), which is worse than wasteful in a multitasking environment! select() allows you to block on a set of file descriptors, waiting until any one of them is ready. (I don't know how standardized select() is, but it should perform the same basic function on different systems.)

For keyboard input, just read a character at a time using getchar() and respond to the newline character by calling write. The STDIN_FILENO file descriptor would be part of the set passed to select(), so you will know that a character is there to read.

I was forgetting that you've already written the server, so you must know about select (?). In the client, you can try using select the same way, adding STDIN_FILENO to the fds, but it is system dependent whether that will work. If it doesn't, there may be some other system dependent way to do it (like setting it up so a signal is sent every keystroke, interrupting the select).

Alternatively, the threaded solution is actually very clean. The "keyboard" thread reads keys, sending its buffer to the "main" thread (either when enter is pressed or even char by char) which communicates with the server.

Basically you just say "start a new thread of execution here". It shares the same address space as your main (or original) line of execution, which can be handy. How about something like this:

#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/time.h>
#include <pthread.h>


void*
KeyboardThread (void *unused)
{
    printf ("In keyboard thread\nPress Enter...");
    getchar();
    printf ("Exiting keyboard thread\n:);
    return NULL; /* Or pthread_exit(NULL) */
}


pthread_t
StartKeyboardThread (void)
{
    pthread_t thread;
    if (pthread_create (&thread,        /* Thread id */
                        NULL,           /* Default attributes */
                        KeyboardThread, /* Thread start routine */
                        NULL)           /* Arg to start routine */
            != 0)
    {
        perror ("StartKeyboardThread");
        exit (-1);
    }
    return thread;
}


int
main (void)
{
    /* To start it */
    pthread_t thread = StartKeyboardThread();

    /* And when you want to kill it */
    pthread_cancel (thread);

    return 0;
}
commented: Great idea and easy to understand +1

I finally got the select() syntax right! Thank you all for the wonderful pointers!

After the project is complete, I'd like to go back and try to redo it with the multi-thread idea. That sounds like a lot of fun to learn.

- Jim

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.