How to handle error in free and realloc in partial recv() in this code

Question

For handling partial recv() in networks, I tried to check the buffer until it receives '\n' by reading the data sent from the client one by one and dynamically allocating the memory and then to print the message, but I encounter an error like realloc(): invalid pointer aborted().

My code for handling partial reads:

char *recieve_msg(int sockfd)
{
    char *newp;
    char *recv_buf;
    int recv_buf_size = 0;
    int ret_recv = 0;

    recv_buf = (char *)malloc(sizeof(char));
    while (1) { 
        ret_recv  = recv(sockfd, recv_buf, sizeof(char), 0);
        if (ret_recv == -1)
            return NULL;
        else if (ret_recv == 0)
            return  NULL;
        else if (recv_buf[recv_buf_size] == '\n')
            break;
        recv_buf_size++;
        newp = realloc(recv_buf, (recv_buf_size + 1) * sizeof(char));
        if (newp == NULL) {
            free(recv_buf);
            return NULL;
        }
        recv_buf = newp;
        recv_buf = recv_buf + ret_recv;
    }
    recv_buf[recv_buf_size] = '\0';

    return recv_buf;
}

Edited Code:

char *recieve_msg(int sockfd)
{
    char *newp;
    char *tmp;
    char *recv_buf;
    int recv_buf_size = 0;
    int ret_recv = 0;

    recv_buf = (char *)malloc(sizeof(char));
    if (recv_buf == NULL)
        return NULL;
    tmp = recv_buf;
    while (1) { 
        ret_recv  = recv(sockfd, tmp, sizeof(char), 0);
        if (ret_recv == -1)
            return NULL;
        else if (ret_recv == 0)
            return  NULL;
        else if (tmp[recv_buf_size] == '\n')
            break;
        recv_buf_size++;
        newp = realloc(tmp, (recv_buf_size + 1) * sizeof(char));
        if (newp == NULL) {
            free(tmp);
            return NULL;
        }
        tmp = newp;
        tmp = tmp + ret_recv;
    }
    tmp[recv_buf_size] = '\0';

    return recv_buf;
}

This too arises the same problem.

You can't `realloc` (or even `free` ) buffer that you have modified the pointer to. (`recv_buf = recv_buf + ret_recv;`). Please note how your question title is irrelevant to your actual problem. — Eugene Sh., Jun 08 '20 at 14:09
@Eugene Sh. Thanks for the suggestion, I have updated the code with the suggestions but this too results in the same output. — Paul, Jun 08 '20 at 14:29
*This too arises the same problem.* - because it has the exact same problem. you `realloc` `tmp` after modifying it. — Eugene Sh., Jun 08 '20 at 14:29
@Eugene Sh. Oh, It would be little helpful if you give an example code with the suggestions. Thanks. — Paul, Jun 08 '20 at 14:31
Unrelated to the problem: [dont cast malloc](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Barmar, Jun 08 '20 at 14:47
Your code also has a few other problems. On errors or when the connection is closed you leak the allocated memory. You also fail to return and data that has already been received when you get an error or receive zero bytes so there's a good chance you're going to lose the last chunk of data you read. — Andrew Henle, Jun 08 '20 at 16:33

m0hithreddy · Accepted Answer · 2020-06-08T16:25:24.643

As other posters pointed out, you should not realloc() the modified pointers. tmp is initialized to recv_buf. But tmp is modified as tmp = tmp + ret_recv, and you are trying to realloc() that modified tmp as newp = realloc(tmp, (recv_buf_size + 1) * sizeof(char)), thus the error. You can have your code as follows:

char *recieve_msg(int sockfd)
{
    char recv_buf[1], *msg_buf = NULL;
    int recv_ret = 0, msg_len = 0;

    while (1) {
        recv_ret  = recv(sockfd, recv_buf, sizeof(recv_buf), 0);

        if (recv_ret == -1)
            return NULL;
        else if (recv_ret == 0)
            return  NULL;
        else if (recv_buf[0] == '\n')
            break;

        msg_len++;
        msg_buf = realloc(msg_buf, (msg_len + 1) * sizeof(char));

        if (msg_buf == NULL) {
            return NULL;
        }

        msg_buf[msg_len - 1] = recv_buf[0];
    }

    if (msg_buf == NULL)
        return NULL;

    msg_buf[msg_len] = '\0';

    return msg_buf;
}

Keeping this problem aside, I strongly condemn realloc()'ing the memory for each recv() call. You can come up with some better approaches. In your case, for example, having a global-buffer and operating on it when \n is received. If the global-buffer is about to fill and your message is still not received, then realloc the buffer to a larger size (maybe 100 bytes more or so).

How to handle error in free and realloc in partial recv() in this code

1 Answers1