-1

I have hit a weird situation. I am creating a simple echo client-server. The client is able to send message to the server. The accept call succeeds in the server but for some reason, it does not detect the client message the first time. But when the client sends the message the second time, the server is able to detect the message and print it.

Here is the server snippet where I am accepting the client connection.

while(1) {
    // continuously listen for the new client-requests
    int client_fd = accept(server_fd, (struct sockaddr*)&client,&client_socklen);
    if(client_fd > 0) {
     char b[10];
     while(1) {

      ssize_t response = recv(client_fd,b, 10, 0);
      if(response == 0) {printf("reached end of stream"); break;}
      if(response == -1) {printf("error"); break;}

     }

     fprintf("%s", b);

    }
}

When the client send the request the first time, the call to accept succeeds and the control enters the if block but there it does not receive message and just prints reached end of stream. But when the client again sends the message, it receives the message and prints it. Why does this happen? Client is sending a simple echo! message.

Client snippet:

    #include <netdb.h>
#include <netinet/in.h>
#include <stdio.h>
#include <strings.h>
#include <stdlib.h>
#include <unistd.h>

#define SERVER_ADDR "localhost"
#define SERVER_PORT 8889


int main(int argc, char **argv) {

    int socket_fd = 0;
    struct sockaddr_in server_socket_addr;

    // Converts localhost into 0.0.0.0
    struct hostent *he = gethostbyname(SERVER_ADDR);
    unsigned long server_addr_nbo = *(unsigned long *)(he->h_addr_list[0]);

    // Create socket (IPv4, stream-based, protocol likely set to TCP)
    if (0 > (socket_fd = socket(AF_INET, SOCK_STREAM, 0))) {
        fprintf(stderr, "client failed to create socket\n");
        exit(1);
    }

    bzero(&server_socket_addr, sizeof(server_socket_addr));
    server_socket_addr.sin_family = AF_INET;
    server_socket_addr.sin_port = htons(SERVER_PORT);
    server_socket_addr.sin_addr.s_addr = server_addr_nbo;

    // Connect socket to server
    if (0 > connect(socket_fd, (struct sockaddr *)&server_socket_addr, sizeof(server_socket_addr))) {
        fprintf(stderr, "client failed to connect to %s:%d!\n", SERVER_ADDR, SERVER_PORT);
        close(socket_fd);
        //          exit(1);
    } else {
        fprintf(stdout, "client connected to to %s:%d!\n", SERVER_ADDR, SERVER_PORT);
        char *my_message = "hey!!"
        int bytes_sent = send(socket_fd, my_message, strlen(my_message), 0);
    }

    // Close the socket and return
    close(socket_fd);
    return 0;
}
user207421
  • 305,947
  • 44
  • 307
  • 483
Amanda
  • 2,013
  • 3
  • 24
  • 57
  • accept should return after client connected, not send message. – Jiang YD Jan 15 '20 at 08:00
  • 1
    What is your client *really* doing? Please try to show a [mcve] of the client. – Some programmer dude Jan 15 '20 at 08:01
  • May or may not related to your issue, but for security purposes, you need to properly null terminate the `b` buffer before printing it. There's no guarantee that the client sent a null terminated message. Further, TCP is a stream protocol not a message protocol. Even if the client sent a null terminated string, you might only receive partial data with the rest arriving on the next `recv` call. – selbie Jan 15 '20 at 08:01
  • Also, I just noticed. how do you manage to invoke `fprintf` manage to work without passing a FILE to it? Let's assume you really meant to use `printf`. You don't actually print anything until after the client closes the connection. In which case, you get whatever was left in the buffer. – selbie Jan 15 '20 at 08:05
  • @Someprogrammerdude Added my client snippet – Amanda Jan 15 '20 at 08:06
  • @selbie Added my client snippet – Amanda Jan 15 '20 at 08:06
  • 3
    Perhaps the problem is that you read 10 bytes at a time in a loop, until you get an error or the client closes the connection. ***But*** each time you read in the loop, you *overwrite* what you read previously. Try printing the string *inside* the loop (after making sure it's null-terminated of course). – Some programmer dude Jan 15 '20 at 08:07
  • I also suggest you print what `recv` returns when it's not zero or minus one. TCP connection are *streaming*, a single `recv` might not give you all that was sent in a single `send` call. – Some programmer dude Jan 15 '20 at 08:08
  • @Someprogrammerdude It prints `b` when `response == 0`. What would be a good way to read messages? Could you show an example? – Amanda Jan 15 '20 at 08:14
  • 1
    When `response == 0` it means the connection was closed, and the contents of the array `b` will be indeterminate (since nothing was actually read into it). – Some programmer dude Jan 15 '20 at 08:20
  • Related: https://stackoverflow.com/q/1386142/1025391 – moooeeeep Jan 15 '20 at 08:22
  • @Someprogrammerdude So you mean as soon as the client closes the connection the buffer will be become empty? – Amanda Jan 15 '20 at 08:26
  • The buffer will only have contents if `recv` returns with a positive non-zero value. – Some programmer dude Jan 15 '20 at 08:30
  • 1
    The buffer always has contents of some kind. What's in the buffer will only have *meaning* if the immediately prior `recv()` returned a positive value. @Someprogrammerdude – user207421 Jan 15 '20 at 08:41
  • Amanda, your client snippet is part of the question. If you delete it your question will be closed. – user207421 Jan 15 '20 at 09:21
  • @user207421 I thought it was not needed. – Amanda Jan 15 '20 at 10:12

1 Answers1

1

Instead of this:

if(client_fd > 0) {
     char b[10];
     while(1) {

      ssize_t response = recv(client_fd,b, 10, 0);
      if(response == 0) {printf("reached end of stream"); break;}
      if(response == -1) {printf("error"); break;}

     }

     fprintf("%s", b);

This:

if(client_fd > 0) {
     const size_t BUFFER_SIZE = 10;
     char b[BUFFER_SIZE+1];  // +1 for null termination
     while(1) {
      b[0] = '\0';
      ssize_t response = recv(client_fd,b, BUFFER_SIZE, 0);
      if(response == 0) {printf("reached end of stream"); break;}
      if(response == -1) {printf("error"); break;}
      b[response] = '\0';
      printf("%s\n", b);  //  \n so that the message will flush to the console
     }
selbie
  • 100,020
  • 15
  • 103
  • 173
  • Could you please explain the reason we do `b[0] = '\0';` and `b[response] = '\0';`? – Amanda Jan 15 '20 at 08:25
  • For starters, **security**. You say your server is an echo server. If I my code connets to your server and sends you `"hey"` but without a terminating null char, your server will send me a lot more back than just the three characters I sent you, which could include a lot of sensitive data on your program's stack. It also, makes debugging a lot easier - you can easily inspect the string in the IDE without a bunch of garbage characters. – selbie Jan 15 '20 at 08:29
  • I understood `b[response] = '\0';` where you null terminate a string. But in the statement `b[0] = '\0';` what does it mean to add `\0` just at the beginning of the string? – Amanda Jan 15 '20 at 08:46
  • @Amanda It means nothing. You shouldn't be making any use of this buffer unless `recv()` returns positive. IMO you shouldn't be trying to null-terminate it either: you should be using the `recv()` return value as the length to print, copy, convert, etc. – user207421 Jan 15 '20 at 09:02
  • @Amanda - `b[0] = '\0'` is equivalent to `strcpy(b, "")`. It's a trivial initialization with trivial cost such that makes debugging easier. And while it would be a bug to reference `b` if the `recv` call fails, you at least have it that buffer initialized to be an empty string so it is less likely to leak something it case it does. I'm not in full agreement with user207421. At some point, if you got to print the data, you got to null terminate it. But he is correct that you need to be diligent in referencing the length of the buffer for subsequent processing. – selbie Jan 15 '20 at 10:10
  • Not so. To print the data you use `printf("%.*s", count, buffer)`. No null-termination required. And no assuming you got the entire message. – user207421 Jan 16 '20 at 02:12