1

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.

User_Not_Found
  • 67
  • 7
  • From a [read( ) manual](https://pubs.opengroup.org/onlinepubs/009604599/functions/read.html) you can see there are several reasons for zero to be returned. For example: *If the starting position is at or after the end-of-file, 0 shall be returned.* – mmixLinus Dec 28 '21 at 12:42

2 Answers2

0

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;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • Well in the ... part I do: "char **result = parse_cmdline(buff);" which basically splits the string into an array and then I also free the result, so that shouldn't be a problem "free(buff); free(result);" Isn't that all, what else should I free for this memory leak? – User_Not_Found Dec 28 '21 at 19:30
  • And also I thought it could be a problem, because the whole main is an infinite loop. So valgrind doesn't stop with ctrl+d or ctrl+z only with ctrl+c. Maybe that is the problem, that it frees it but its not shown because it terminates the whole program? (Working on linux terminal) – User_Not_Found Dec 28 '21 at 19:50
  • @User_Not_Found: if you kill the program with ctrl-c, no wonder `buff` is not freed. You probably terminate the program at the `read` system call, after `buff` was just allocated and the program stops there. You should modify the program to detect end of file on standard input and break from the loop after freeing the buffer. – chqrlie Dec 28 '21 at 23:20
0

According to this answer to a post, still reachable does not necessarily indicate a memory leak.

mmixLinus
  • 1,646
  • 2
  • 11
  • 16