Home > database >  Memory leak / not properly freeing. How to fix?
Memory leak / not properly freeing. How to fix?

Time:12-29

I am partially reading input into a buffer in a function and then freeing it in main() but it doesn't seem to be working. My code:

char *save_to_buff()
{
    int fd = 0; // set read() to read from STDIN_FILENO, because it's number is 0
    const size_t read_size = 100; // set chunk size
    size_t size = read_size;
    char *buff = malloc(size 1);
    size_t offset = 0;
    size_t res = 0;
    while((res = read(fd, buff   offset, read_size)) > 0) // partial read from stdin and save to buff
    {
        if(res == -1) // check for read errors
        {
            read_error();
            free(buff);
            return NULL;
        }
        
        offset  = res;
        if (offset   read_size > size)
        {
            size *= 2;
            buff = realloc(buff, size 1);
        }
        buff[offset] = '\0';
    }
    return buff;
}

main:

char *buff = save_to_buff();
// do sth
free(buff);

Valgrind result

Edit: just tried it with a 1 byte read and not a partial read and there is no memory leak.

CodePudding user response:

When you read the whole file once, so the read pointer is at EOF, if you read again, as in your code, the read() will return 0, according to this. This will exit your loop without freeing the memory.

Also, depending on what type of file you are reading: Files that do not support seeking - for example, terminals - always read from the current position. The value of a file offset associated with such a file is undefined

CodePudding user response:

You did not post the full code for the main() function, it is possible there is something fishy we don't see.

The function save_to_buff has some issues:

  • if stdin is already at the end of file when it starts, it will return a pointer to an uninitialized block of 101 bytes. It should either return a null pointer or a pointer to an empty string.
  • you do not test for memory allocation failure

The calling sequence in the code fragment seems fine as long as you do not modify buff in the // do sth part.

Here is a modified version:

#include <stdio.h>
#include <unistd.h>

char *save_to_buff() {
    int fd = 0; // set read() to read from STDIN_FILENO
    const size_t read_size = 100; // set chunk size
    size_t size = read_size;
    size_t offset = 0;
    size_t res = 0;
    char *buff = malloc(size   1);

    if (buff == NULL)
        return NULL;
   
    *buff = '\0';
    while ((res = read(fd, buff   offset, read_size)) != 0) {
        // partial read from stdin and save to buff
        if (res == (size_t)-1) { // check for read errors
            read_error();
            free(buff);
            return NULL;
        }
        
        offset  = res;
        buff[offset] = '\0';

        if (offset   read_size > size) {
            char *buff0;

            size *= 2;
            buff = realloc(buff0 = buff, size   1);
            if (buff == NULL) {
                free(buff0);
                return NULL;
            }
        }
    }
    return buff;
}
  • Related