TCP Server multiple packet sending issue

Question

I have written the server client program using linux tcp sockets.

Client ask server for present directory files list by sending the ls command

server replies all the list of files in server dir.
I was testing it for more files in server working dir.

server response format in the buffer
file/dir [tab] file_name [tab] file_change_time
for each 1000 files to client.

Server Sending Code:

#define BUFSIZE 1400

void lsfun(node_t *pclient)
{
    DIR *directory;
    int status;
    int cpylen = 0;
    int msglen = 0;
    unsigned int tt_count = 0;
    unsigned int no_files = 0;
    unsigned int no_sends = 0;
    int clientfd = *(pclient->client_socket);
    char *filectime;
    char *buffer = malloc(BUFSIZE * sizeof(char));
    char *tmp = malloc(BUFSIZE * sizeof(char));
    char ending[] = "#####";
    struct dirent *dir;
    struct stat type;

    pthread_mutex_lock(&lock);
    chdir(pclient->pwd);
    directory = opendir(".");
    pthread_mutex_unlock(&lock);

    if(tmp == NULL || buffer == NULL)
        printf("malloc error for client conn:%d\n", clientfd);

    if(directory)
    {
        while((dir = readdir(directory)) != NULL)
        {
            if(!strcmp(dir->d_name, ".") || !strcmp(dir->d_name, ".."))
                continue;
            status = stat(dir->d_name, &type);

            if(status == 0)
            {
                filectime = ctime(&type.st_ctime);

                if(dir->d_type != DT_REG)
                    cpylen = snprintf(tmp, BUFSIZE, "dir\t%s\t%s", dir->d_name, filectime);
                else
                    cpylen = snprintf(tmp, BUFSIZE, "file\t%s\t%s", dir->d_name, filectime);

                tmp[cpylen] = 0;

                if((cpylen + msglen) < BUFSIZE)
                {
                    strlcpy(buffer + msglen, tmp, cpylen);
                    msglen += cpylen;
                    no_files += 1;
                }
                else
                {
                    tt_count += msglen;
                    printf("%s", buffer);
                    fflush(stdout);
                    send(clientfd, buffer, strlen(buffer), 0);
                    memset(buffer, 0, BUFSIZE + 5);
                    snprintf(buffer, cpylen, "%s", tmp);
                    msglen = cpylen;
                    cpylen = 0;
                    no_files += 1;
                    no_sends += 1;
                }
            }
            else
            {
                cpylen = snprintf(buffer + msglen, BUFSIZE, "%s%s\n", "file stat error:", dir->d_name);
                msglen += cpylen;
            }
            memset(tmp, 0, BUFSIZE);
        }
    }

    cpylen = strlen(buffer);
    if(msglen == cpylen)
        send(clientfd, buffer, strlen(buffer), 0);

    send(clientfd, ending, strlen(ending), 0);      //sending msg ending for client read to close

    printf("\nlssize :%d\tnofile:%d, msglen:%d\tcpylen:%d\tno_sends:%d\n", tt_count + msglen, no_files, msglen, cpylen, no_sends);

    free(tmp);
    free(buffer);
    closedir(directory);
}

Client receiving Code:

#define BUFSIZE 1400
while(true)
{
    msgsize = read(socketfd, buffer, BUFSIZE);
    buffer[msgsize] = 0;
    snprintf(ending, 6, "%s", buffer + (strlen(buffer) - 5));

    if(strcmp(ending, "#####") == 0)
    {
        buffer[strlen(buffer) - 5] = 0;

        if(buffer[strlen(buffer) - 1] == '\n')
            printf("%s", buffer);
        else
            printf("%s\n", buffer);
        fflush(stdout);

        break;
    }
    else
    {
        printf("%s", buffer);
        memset(buffer, 0, BUFSIZE);
    }
}

Server replay debug print:

lssize :19931 nofile:501, msglen:437 cpylen:39 no_sends:14

why am I only receiving two packet instead of 14 packets from
server packets of around 1400 bytes each ?

where is the mistake ?

also welcome any code improvement suggestions.

Answer 1

In addition to the bugs pointed out in the comments, there are more fundamental issues with your code that are too extensive to merely comment on.

This code implies multithreaded use:

pthread_mutex_lock(&lock);
chdir(pclient->pwd);
directory = opendir(".");
pthread_mutex_unlock(&lock);

BUT, this code assumes the current working directory is always the current function's pclient->pwd:

 status = stat(dir->d_name, &type);

It won't be if another thread calls chdir() to another directory while the loop is running, so your results

stat() always checks a relative path from the current working directory of the entire process. Which, in your posted code, can change.

A good rule to follow is to NEVER do things that change any global property of a process if you're writing multithreaded code.

Your use of snprintf() is also bug-prone. For example:

cpylen = snprintf(buffer + msglen, BUFSIZE, "%s%s\n", "file stat error:", dir->d_name);

Per 7.21.6.5 The snprintf function, paragraph 2 of the C11 standard(bolding mine):

The snprintf function returns the number of characters that would have been written had n been sufficiently large, not counting the terminating null character, or a negative value if an encoding error occurred. Thus, the null-terminated output has been completely written if and only if the returned value is nonnegative and less than n.

You are blindly assuming that every one of your calls to snprintf() work. If any one call to snprintf() fails, your message contents are wildly indeterminate and your value for msglen will not accurately reflect the contents of your buffer.

That means this code won't send anything:

cpylen = strlen(buffer);
if(msglen == cpylen)
    send(clientfd, buffer, strlen(buffer), 0);