t4 Planeur t4 Planeur - 24 days ago 5
C Question

Leak of memory in C

In this function, I have leak of memory

static int read_buffer(int const fd, char **buffer)
{
char buff[BUFF_SIZE + 1];
int ret;

ret = read(fd, buff, BUFF_SIZE);
if (ret > 0)
{
buff[ret] = 0;
if (!(*buffer = ft_strjoin(*buffer, buff)))
return (-1);
}
return (ret);
}


I tried to do this and more solution again.. But I get also leak of memory

static int read_buffer(int const fd, char **buffer)
{
char buff[BUFF_SIZE + 1];
char *tmp;
int ret;

ret = read(fd, buff, BUFF_SIZE);
tmp = *buffer;
if (ret > 0)
{
buff[ret] = 0;
if (!(*buffer = ft_strjoin(*buffer, buff)))
return (-1);
free(tmp);
}
return (ret);
}


Here is ft_strjoin function: (can't modify)

char *ft_strjoin(char const *s1, char const *s2)
{
size_t size_s1;
size_t size_s2;
char *strjoin;

size_s1 = ft_strlen(s1);
size_s2 = ft_strlen(s2);
if (!(strjoin = malloc(size_s1 + size_s2 + 1)))
return (NULL);
ft_strcpy(strjoin, s1);
ft_strcat(strjoin, s2);
return (strjoin);
}


How I can resolve this ?

Thank you !

Answer

Anytime a function returns malloc'd memory, you've got a hot potato. You have to free that memory and cannot pass it on unless you've documented you're doing so. And you mustn't lose hold of (e.g. reuse) the pointer before freeing it. Below is how I interpret how read_buffer() needs to work. Since "buffer length is unknown" I'm assuming it comes from malloc and we can use realloc:

static int read_buffer(int const fd, char **buffer)
{
    char local_buffer[BUFF_SIZE + 1];

    int bytes_read = read(fd, local_buffer, BUFF_SIZE);

    if (bytes_read > 0)
    {
        local_buffer[bytes_read] = '\0';

        char *joined = ft_strjoin(*buffer, local_buffer);

        if (joined != NULL)
        {
            *buffer = realloc(*buffer, strlen(joined) + 1);

            if (*buffer != NULL)
            {
                ft_strcpy(*buffer, joined);
            }
            else
            {
                bytes_read = -1;
            }

            free(joined);
        }
        else {
            bytes_read = -1;
        }
    }

    return bytes_read;
}

Note that there are a lot of potential pitfalls that you need to test for and recover from.

If you're reading from the user directly, and don't want newlines as part of the concatenation of strings, you can replace this line:

     local_buffer[bytes_read] = '\0';

with something like:

    if (local_buffer[bytes_read - 1] == '\n')
    {
        local_buffer[bytes_read - 1] = '\0';
    }
    else
    {
        local_buffer[bytes_read] = '\0';
    }

Assuming the above newline fix, here's a small test program I wrote:

int main() {

    char *buffer = malloc(1);

    *buffer = '\0';

    for (int i = 0; i < 3; i++)
    {
        (void) read_buffer(0, &buffer);

        puts(buffer);
    }

    free(buffer);

    return 0;
}

RESULT

> ./a.out
abcdefg
abcdefg
hijklmn
abcdefghijklmn
opqrstu
abcdefghijklmnopqrstu
>