1
\$\begingroup\$

My code recovers all sent messages (ending with "\r\n") in function read_message. I would like to improve this code, make it more efficient? If it's possible, I am open to all reviews!

Code

static char     *read_message(char *data)
{
    static char     *buffer = NULL;
    char            *message = NULL;
    char            *ptr;
    size_t          size;

    if (buffer)
    {
        size = strlen(buffer) + strlen(data) + 1;
        if (!(buffer = realloc(buffer, size)))
            return (NULL);
        strcat(buffer, data);
    }
    else {
        if (!(buffer = strdup(data)))
            return (NULL);
    }
    if ((ptr = strstr(buffer, "\r\n")))
    {
        size = ((unsigned int)ptr - (unsigned int)buffer) + 1;
        if (!(message = (char*)malloc(sizeof(char) * size)))
            return (NULL);
        strncpy(message, buffer, size);
        ptr += 2;
        if (!(ptr = strdup(ptr)))
            return (NULL);
        free(buffer);
        buffer = ptr;
    }
    return (message);
}

static t_message    *parse_message(char *buffer)
{
    int     id;

    id = atoi(buffer);
    printf("L'id du message est %d\n", id);
    return (NULL);
}

static void     handle_message(void)
{
    printf("OK\n");
}

void            listen_client(t_args *args)
{
    ssize_t     count;
    char        buffer[BUFF_SIZE + 1] = { 0 };
    char        *msg_str;
    t_message   *msg;

    while ((count = recv(args->client->s, buffer, BUFF_SIZE, 0)))
    {
        buffer[count] = '\0';
        if (!(msg_str = read_message(buffer)))
            continue ;
        if (!(msg = parse_message(msg_str)))    
            continue ;
        handle_message();
    }
    printf("Fin de connexion\n");
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Please show the definition of t_message, as well as showing all relevant #includes. \$\endgroup\$ – 200_success Mar 30 at 22:05
2
\$\begingroup\$

Portability

First this program could be made more portable. There are 2 portability issues here

If it was ported to a system that did not define end of line as "\r\n" this program might not work. This problem could be solved by including the proper header files or using

#define EndOfLine "\r\n"

and using

    ptr = strstr(buffer, EndOfLine)

The second portability issue is the use of strdup(). It is not implemented on all systems. This Stackoverflow question discusses the use of strdup.

Consistency in Coding Style

There is one place in the code where the indentation is inconsistent:

    if (buffer)
    {
        size = strlen(buffer) + strlen(data) + 1;
        if (!(buffer = realloc(buffer, size)))
            return (NULL);
        strcat(buffer, data);
    }
    else {
        if (!(buffer = strdup(data)))
            return (NULL);
    }

The use of braces "{" and "}" should be consistent through the entire program. Everywhere except in this code the brace starts on a new line.

It would be very nice if the headers were included for compilation and testing. Without the headers or the calling functions it is difficult to compile or test this code.

Complexity

It might be better if the function read_message(char *data) was separated into multiple functions to reduce the complexity. All of the functions might be more readable if there was some virtical spacing between blocks of code.

Possible Bug

The function parse_message(char *buffer) always returns NULL, in your testing did the function handle_message() always get called?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.