c - File transfer server/client using socket

Question

I am trying to make a file transfer between server and client, but is working very badly. Basically what needs to happen is:
1) The client send a txt file to the server (I called it "quotidiani.txt")
2) The server saves it in another txt file ("receive.txt")
3) The server runs a script on it that modifies it and saves it with another name ("output.txt")
4) The server send the file back to the client that saves it (on the same socket) with the name (final.txt)

The problem is that the first file (quotidiani.txt) is read just for a little part, and then there are some errors. I'd like someone to help me understand and correct my errors.

Here's my code:

client.c:

#include <stdlib.h>
#include <stdio.h>
#include <errno.h>
#include <string.h>
#include <sys/types.h>
#include <netinet/in.h>
#include <sys/wait.h>
#include <sys/socket.h>
#include <signal.h>
#include <ctype.h>          
#include <arpa/inet.h>
#include <netdb.h>

#define PORT 20000
#define LENGTH 512 


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

int main(int argc, char *argv[])
{
    /* Variable Definition */
    int sockfd; 
    int nsockfd;
    char revbuf[LENGTH]; 
    struct sockaddr_in remote_addr;

    /* Get the Socket file descriptor */
    if ((sockfd = socket(AF_INET, SOCK_STREAM, 0)) == -1)
    {
        fprintf(stderr, "ERROR: Failed to obtain Socket Descriptor! (errno = %d)
",errno);
        exit(1);
    }

    /* Fill the socket address struct */
    remote_addr.sin_family = AF_INET; 
    remote_addr.sin_port = htons(PORT); 
    inet_pton(AF_INET, "127.0.0.1", &remote_addr.sin_addr); 
    bzero(&(remote_addr.sin_zero), 8);

    /* Try to connect the remote */
    if (connect(sockfd, (struct sockaddr *)&remote_addr, sizeof(struct sockaddr)) == -1)
    {
        fprintf(stderr, "ERROR: Failed to connect to the host! (errno = %d)
",errno);
        exit(1);
    }
    else 
        printf("[Client] Connected to server at port %d...ok!
", PORT);

    /* Send File to Server */
    //if(!fork())
    //{
        char* fs_name = "/home/aryan/Desktop/quotidiani.txt";
        char sdbuf[LENGTH]; 
        printf("[Client] Sending %s to the Server... ", fs_name);
        FILE *fs = fopen(fs_name, "r");
        if(fs == NULL)
        {
            printf("ERROR: File %s not found.
", fs_name);
            exit(1);
        }

        bzero(sdbuf, LENGTH); 
        int fs_block_sz; 
        while((fs_block_sz = fread(sdbuf, sizeof(char), LENGTH, fs)) > 0)
        {
            if(send(sockfd, sdbuf, fs_block_sz, 0) < 0)
            {
                fprintf(stderr, "ERROR: Failed to send file %s. (errno = %d)
", fs_name, errno);
                break;
            }
            bzero(sdbuf, LENGTH);
        }
        printf("Ok File %s from Client was Sent!
", fs_name);
    //}

    /* Receive File from Server */
    printf("[Client] Receiveing file from Server and saving it as final.txt...");
    char* fr_name = "/home/aryan/Desktop/progetto/final.txt";
    FILE *fr = fopen(fr_name, "a");
    if(fr == NULL)
        printf("File %s Cannot be opened.
", fr_name);
    else
    {
        bzero(revbuf, LENGTH); 
        int fr_block_sz = 0;
        while((fr_block_sz = recv(sockfd, revbuf, LENGTH, 0)) > 0)
        {
            int write_sz = fwrite(revbuf, sizeof(char), fr_block_sz, fr);
            if(write_sz < fr_block_sz)
            {
                error("File write failed.
");
            }
            bzero(revbuf, LENGTH);
            if (fr_block_sz == 0 || fr_block_sz != 512) 
            {
                break;
            }
        }
        if(fr_block_sz < 0)
        {
            if (errno == EAGAIN)
            {
                printf("recv() timed out.
");
            }
            else
            {
                fprintf(stderr, "recv() failed due to errno = %d
", errno);
            }
        }
        printf("Ok received from server!
");
        fclose(fr);
    }
    close (sockfd);
    printf("[Client] Connection lost.
");
    return (0);
}

server.c

#include <stdlib.h>
#include <stdio.h>
#include <errno.h>
#include <string.h>
#include <sys/types.h>
#include <netinet/in.h>
#include <sys/wait.h>
#include <sys/socket.h>
#include <signal.h>
#include <ctype.h>          
#include <arpa/inet.h>
#include <netdb.h>

#define PORT 20000 
#define BACKLOG 5
#define LENGTH 512 

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

int main ()
{
    /* Defining Variables */
    int sockfd; 
    int nsockfd; 
    int num;
    int sin_size; 
    struct sockaddr_in addr_local; /* client addr */
    struct sockaddr_in addr_remote; /* server addr */
    char revbuf[LENGTH]; // Receiver buffer

    /* Get the Socket file descriptor */
    if((sockfd = socket(AF_INET, SOCK_STREAM, 0)) == -1 )
    {
        fprintf(stderr, "ERROR: Failed to obtain Socket Descriptor. (errno = %d)
", errno);
        exit(1);
    }
    else 
        printf("[Server] Obtaining socket descriptor successfully.
");

    /* Fill the client socket address struct */
    addr_local.sin_family = AF_INET; // Protocol Family
    addr_local.sin_port = htons(PORT); // Port number
    addr_local.sin_addr.s_addr = INADDR_ANY; // AutoFill local address
    bzero(&(addr_local.sin_zero), 8); // Flush the rest of struct

    /* Bind a special Port */
    if( bind(sockfd, (struct sockaddr*)&addr_local, sizeof(struct sockaddr)) == -1 )
    {
        fprintf(stderr, "ERROR: Failed to bind Port. (errno = %d)
", errno);
        exit(1);
    }
    else 
        printf("[Server] Binded tcp port %d in addr 127.0.0.1 sucessfully.
",PORT);

    /* Listen remote connect/calling */
    if(listen(sockfd,BACKLOG) == -1)
    {
        fprintf(stderr, "ERROR: Failed to listen Port. (errno = %d)
", errno);
        exit(1);
    }
    else
        printf ("[Server] Listening the port %d successfully.
", PORT);

    int success = 0;
    while(success == 0)
    {
        sin_size = sizeof(struct sockaddr_in);

        /* Wait a connection, and obtain a new socket file despriptor for single connection */
        if ((nsockfd = accept(sockfd, (struct sockaddr *)&addr_remote, &sin_size)) == -1) 
        {
            fprintf(stderr, "ERROR: Obtaining new Socket Despcritor. (errno = %d)
", errno);
            exit(1);
        }
        else 
            printf("[Server] Server has got connected from %s.
", inet_ntoa(addr_remote.sin_addr));

        /*Receive File from Client */
        char* fr_name = "/home/aryan/Desktop/receive.txt";
        FILE *fr = fopen(fr_name, "a");
        if(fr == NULL)
            printf("File %s Cannot be opened file on server.
", fr_name);
        else
        {
            bzero(revbuf, LENGTH); 
            int fr_block_sz = 0;
            while((fr_block_sz = recv(nsockfd, revbuf, LENGTH, 0)) > 0) 
            {
                int write_sz = fwrite(revbuf, sizeof(char), fr_block_sz, fr);
                if(write_sz < fr_block_sz)
                {
                    error("File write failed on server.
");
                }
                bzero(revbuf, LENGTH);
                if (fr_block_sz == 0 || fr_block_sz != 512) 
                {
                    break;
                }
            }
            if(fr_block_sz < 0)
            {
                if (errno == EAGAIN)
                {
                    printf("recv() timed out.
");
                }
                else
                {
                    fprintf(stderr, "recv() failed due to errno = %d
", errno);
                    exit(1);
                }
            }
            printf("Ok received from client!
");
            fclose(fr); 
        }

        /* Call the Script */
        system("cd ; chmod +x script.sh ; ./script.sh");

        /* Send File to Client */
        //if(!fork())
        //{
            char* fs_name = "/home/aryan/Desktop/output.txt";
            char sdbuf[LENGTH]; // Send buffer
            printf("[Server] Sending %s to the Client...", fs_name);
            FILE *fs = fopen(fs_name, "r");
            if(fs == NULL)
            {
                fprintf(stderr, "ERROR: File %s not found on server. (errno = %d)
", fs_name, errno);
                exit(1);
            }

            bzero(sdbuf, LENGTH); 
            int fs_block_sz; 
            while((fs_block_sz = fread(sdbuf, sizeof(char), LENGTH, fs))>0)
            {
                if(send(nsockfd, sdbuf, fs_block_sz, 0) < 0)
                {
                    fprintf(stderr, "ERROR: Failed to send file %s. (errno = %d)
", fs_name, errno);
                    exit(1);
                }
                bzero(sdbuf, LENGTH);
            }
            printf("Ok sent to client!
");
            success = 1;
            close(nsockfd);
            printf("[Server] Connection with Client closed. Server will wait now...
");
            while(waitpid(-1, NULL, WNOHANG) > 0);
        //}
    }
}

Answer 1

Some comments in no particular order:

• You're passing up the opportunity to know exact errors too often:

  if(listen(sockfd,BACKLOG) == -1)
  {
      printf("ERROR: Failed to listen Port %d.
  ", PORT);
      return (0);
  }
  

  This block should definitely include a perror("listen") or something similar. Always include perror() or strerror() in every error handling block when the error details will be reported via errno. Having exact failure reasons will save you hours when programming and will save you and your users hours when things don't work as expected in the future.

• Your error handling needs some further standardizing:

  if((sockfd = socket(AF_INET, SOCK_STREAM, 0)) == -1 )
  {
      printf("ERROR: Failed to obtain Socket Descriptor.
  ");
      return (0);
  }
  

  This should not return 0 because that will signal to the shell that the program ran to completion without error. You should return 1 (or use EXIT_SUCCESS and EXIT_FAILURE) to signal an abnormal exit.

   else 
      printf("[Server] Server has got connected from %s.
  ", inet_ntoa(addr_remote.sin_addr));
  
   /*Receive File from Client */
  

  In this preceding block you've gotten an error condition but continue executing anyway. That's a quick way to get very undesirable behavior. This should either re-start the main server loop or exit the child process or something similar. (Depends if you keep the multi-process server.)

  if(!fork())
  {
  

  The preceding block forgot to account for fork() failing. fork() can, and does fail -- especially in shared hosting environments common at universities -- so you should be prepared for the full, complicated three possible return values from fork(): failure, child, parent.

• It appears you're using fork() indiscriminately; your client and server are both very simple and the way they are designed to run means they cannot be used to service multiple clients simultaneously. You should probably stick to exactly one process for each, at least until the algorithm is perfectly debugged and you figure out some way to run multiple clients simultaneously. I expect this is the source of the problem you're encountering now.

• You need to use functions to encapsulate details; write a function to connect to the server, a function to send the file, a function to write the file, etc. Write a function to handle the complicated partial writes. (I especially recommend stealing the writen function from the Advanced Programming in the Unix Environment book's source code. File lib/writen.c.) If you write the functions correctly you can re-use them in both the client and server. (Something like placing them in utils.c and compiling the programs like gcc -o server server.c utils.c.)

  Having smaller functions that each do one thing will allow you to focus on smaller amounts of code at a time and write little tests for each that will help you narrow down which sections of code still need improvement.