0

Goal: I am trying to write a concurrent TCP server that accepts and responds to multiple client connection requests, each requesting a file transfer.

I think the error lies within the file handling section in start_routine(), need to verify where. (Read more below code)

File: server.c

#define CONNECTIONS 5

FILE *input;
pthread_t tids[CONNECTIONS];
char name[100];
char buff[1024]; //buffer size

void *start_routine(void *arg)
{
    //recv file request
    read((int) arg, name, sizeof(name));

    //file handling
    input = fopen(name, "rb");

    printf("hi\n"); //testing

    while (!feof(input))
        write((int) arg, buff, fread(buff, sizeof(char), sizeof(buff), input));
    
    return NULL;
}

int main(int argc, char *argv[])
{
    struct sockaddr_in serv_addr;

    if (argc != 2)
    {
        printf("Usage: ./a.out <port #>\n");
        exit(0);
    }

    //open socket
    int listenfd = socket(AF_INET, SOCK_STREAM, 0);

    //init sockaddr_in struct
    serv_addr.sin_family = AF_INET;
    serv_addr.sin_port = htons(atoi(argv[1]));
    serv_addr.sin_addr.s_addr = htons(INADDR_ANY);

    //binds listenfd to a specific sockaddr_in
    bind(listenfd, (struct sockaddr *)&serv_addr, sizeof(serv_addr));
    
    for(int i = 0; i <= CONNECTIONS; i++){
        //listen for new connections on listenfd
        listen(listenfd, CONNECTIONS);
        //accepts a connection
        int connfd = accept(listenfd, (struct sockaddr *)NULL, NULL);
        pthread_create(tids, NULL, start_routine, &connfd);
    }

    //cleaning
    close(listenfd);
    fclose(input);

    return 0;
}

File: client.c

int main(int argc, char *argv[])
{
    int n;
    char buff[1024]; //buffer size
    struct sockaddr_in serv_addr;

    if (argc != 4)
    {
        printf("Usage: ./a.out <file required> <ip of server> <port #> \n");
        exit(0);
    }

    //open socket
    int sockfd = socket(AF_INET, SOCK_STREAM, 0);

    //init sockaddr_in struct
    serv_addr.sin_family = AF_INET;
    serv_addr.sin_port = htons(atoi(argv[3]));
    serv_addr.sin_addr.s_addr = inet_addr(argv[2]);

    //connect to <server_ip>: <port>
    connect(sockfd, (struct sockaddr *)&serv_addr, sizeof(serv_addr));

    //file handling
    FILE *request;
    request = fopen("dst.txt", "wb");

    //send file request
    write(sockfd, argv[1], strlen(argv[1]) + 1);

    while ((n = read(sockfd, buff, sizeof(buff))) > 0)
        fwrite(buff, sizeof(char), n, request);

    //cleaning
    fclose(request);
    close(sockfd);

    return 0;
}

Other files in folder: src.txt

Compiled Using:

gcc server.c -o server -lpthread
./server 8080
gcc client.c -o client
./client src.txt 127.0.0.1 8080

(5 copies of client and so compiled each [client1, client2 ... client5] 5 times)

Problem: Results in segmentation fault. :(

While testing out, I found out that the segmentation fault lies within the start_routine(), in the while loop.

I have used the same snippet from another program (only the variables updated) -- need to know where am I going wrong!

Also, for multi-threading, I created copies of the client program to simulate 5 connections to the server. Hope it is the right way.

Appreciate the help!

hr1z0n
  • 9
  • 4
  • at least two thins are wrong `read((int) arg,` is that correct as `arg` is `void*` and `while (!feof(input))` – IrAM Feb 01 '21 at 10:52
  • the `read((int) arg,` -- how can I correct it? i know it is `void*`, but don't know what I could possibly do. What should I change the parameter to? I also tried casting, but gives the same error – hr1z0n Feb 01 '21 at 10:54
  • Change it to: `int *fd_pointer = arg; read(*fd_pointer, name, sizeof(name));` – kaylum Feb 01 '21 at 10:56
  • does not give me the seg fault anymore, but the content is not copied to the destination file. why is it so? – hr1z0n Feb 01 '21 at 11:02
  • 1
    Did you change the `write` as well? Anyway, it would help greatly if you do proper error checking. You don't check the return value of many of the calls such as `connect`, `fopen`, `fwrite`, etc. That's the first step to proper error handling and debugging. After that do basic debugging. Use a debugger and/or more debug statements to narrow down to where the problem starts. Does the server get the name correctly? Does the server read each file block correctly? Does the server write each block correctly? Does the client receive each block correctly? Do debugging to answer these questions. – kaylum Feb 01 '21 at 11:10
  • Will do, thank you for your time! – hr1z0n Feb 01 '21 at 11:11
  • listen() call should be outside the accept() loop. – Martin James Feb 01 '21 at 17:31
  • 'pthread_create(tids, NULL, start_routine, &connfd);' - connfd coyld be overwritten by a new client cinnection before the handler tbread can dereference the pointer:( – Martin James Feb 01 '21 at 17:33
  • 'char name[100]; char buff[1024];' these declarations should be local to the client handler thread, else each tbread will be trying to use the same data:(( – Martin James Feb 01 '21 at 17:36
  • 'while (!feof(input))' .... https://stackoverflow.com/q/5431941/758133 – Martin James Feb 01 '21 at 17:39
  • 'read((int) arg, name, sizeof(name));' you must correctly and completely handle the results returned from system calls like read(). – Martin James Feb 01 '21 at 17:41
  • 'read((int) arg, name, sizeof(name));' - there is no guarantee that the full 'name' message will be received with one read() call. You must have a protocol on top of TCP to reliably parse messages longer than one byte. – Martin James Feb 01 '21 at 17:44
  • Your protocol, such as it is, is that you send a,NUL-terminated filename : 'write(sockfd, argv[1], strlen(argv[1]) + 1);', so you need to detect the first NUL to determine where the filename ends. – Martin James Feb 01 '21 at 17:51
  • You could usefully declare a 'connData' struct type that has fields for the name, buf and connfd. When tbe accept() returns, malloc a connData, load in the connfd and pass its pointer as the last parameter to pthread_create(). free the pointer in the handler thread just before it exits. – Martin James Feb 01 '21 at 17:59
  • You must understand, and account for, the octet-stream nature of TCP. For example, your attempt to read the name with 'read((int) arg, name, sizeof(name));' will load something like 'myfile.dat/0[some 0-89 bytes of the file data]' into your 100-byte name buffer. It could conceivably only load one byte 'm'. The result returned by read, (that you currently ignore), is the only way to determine how many bytes have been loaded into the buffer. – Martin James Feb 01 '21 at 18:13
  • Oh! I got my server to recv the name correctly. But now it is not writing to the socket. I think, there' something wrong in `write(*fd_pointer, buff, fread(buff, sizeof(char), sizeof(buff), input));` -- takes forever. – hr1z0n Feb 01 '21 at 18:34
  • There is - it's too 'clever'. Don't nest calls like that, especially system calls. Use more lines, more temp vars and, most importantly, more error-checking!! Get the read result and test it BEFORE trying to write! Only write if the read result>0 – Martin James Feb 01 '21 at 19:55
  • ..and ensure that 'request = fopen("dst.txt", "wb");' actually worked. – Martin James Feb 01 '21 at 20:00
  • Oh! Thank you for letting me know! 'if ((n = fread(buff, sizeof(char), 100, input)) > 0) write(*fd_pointer, buff, n);' – hr1z0n Feb 01 '21 at 20:01

0 Answers0