while loop hangs on read() while receiving data from remote server. C socket

Question

I am very new to socket programming and am trying to write simple program which executed a command in remote machine and returns back the output. Using popen() to execute the command, assigning the value and sending it to the remote machine. read() is receiving the data but on printing, we see repetition of the data. read() does not hit EoR so the while loop hangs. Looks like the stream is not ending. These are TCP sockets. Need some help here.

Client code

//Opening socket 
    sockfd = socket(AF_INET, SOCK_STREAM, 0);
    if (sockfd < 0)
        error("ERROR opening socket");
    server = gethostbyname(argv[1]);
    if (server == NULL) {
        fprintf(stderr,"ERROR, no such host\n");
        exit(0);
    }
    bzero((char *) &serv_addr, sizeof(serv_addr));
    serv_addr.sin_family = AF_INET;
    bcopy((char *)server->h_addr,
         (char *)&serv_addr.sin_addr.s_addr,
         server->h_length);
    serv_addr.sin_port = htons(portno);
    if (connect(sockfd, (struct sockaddr *) &serv_addr, sizeof(serv_addr)) < 0)
...
...


      bzero(buffer,256);

     n = read(newsockfd,buffer,255);
     if (n < 0) error("ERROR reading from socket");
     
     sprintf(command , buffer) ;
     pf = popen(command,"r");
     
     bzero(data,513);
     
     while (fgets(data, 512, pf) != NULL) {
       
        write(newsockfd, data, strlen(data));
            printf("%s", data);
            
        }

Server Code

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

....
...

    bzero(buffer,513);

        while ((n= read(sockfd, buffer, 512)) > 0 ) {
         printf("%s\n", buffer);
        }
    close(sockfd);
    return 0;

Reason for while loop is for having the output from remote machine in the same format as were executed on command. I am sure there's a hell lot better way of achieving the same. I just couldn't find one.

Answer 1

Unfortunately, there are just so many issues with this code that the only solution is to rewrite it without all the fundamental misunderstandings in it. I'll point out some of the biggest ones:

 n = read(newsockfd,buffer,255);
 if (n < 0) error("ERROR reading from socket");

There is no guarantee that this reads the entire command.

 sprintf(command , buffer) ;

What if the received data includes "%s"? This sprintf will crash. Are you expecting "%" characters to be escaped?

bzero(buffer,513);

    while ((n= read(sockfd, buffer, 512)) > 0 ) {
     printf("%s\n", buffer);
    }

The first time through, this will work because the bzero puts a zero on the end of the string. But the second time through, it won't work. You have n, the number of bytes read. Why aren't you using it?

The core problem is that you need to send a message, a command. But you have no code that sends or receives a message. TCP provides a stream of bytes, not messages. You need to decide on and implement a message protocol to send and receive messages.

Everybody who writes code in a language like C to deal with TCP makes these mistakes at first. This is a phase everyone goes through.

One big piece of advice -- until you are more experienced, formally specify every protocol you implement. Take a look at protocol specifications for things like HTTP or SMTP to learn what they look like. Document it well enough that someone could write a server that worked with your client (or vice-versa) from that specification.

Yes, it's tedious to do. But then when there's a mistake, you can follow this simple three part test:

1. Does the server follow the specification? If not, stop, the server is broken.

2. Does the client follow the specification? If not, stop, the client is broken.

3. The specification is broken. Fix it.

Without a specification, it's impossible to tell if it's the server or the client that's broken when they don't cooperate properly. That makes debugging much more confusion since there's no "one right way" either is supposed to work.