How can I prevent the current memory leaks in my C program?

Question

I have a program where I load data from a text file into an array of structs. My struct looks like this:

typedef struct A {
    int id;
    char* name;
    char* desc;
} A;

I start loading the file from my main function:

int main(void) {
    A* a;
    int* length = 0;
    arr = betolt("name.txt", &length);
    ...
    free(arr);

    return 0;
}

Then - in a different header file - the real logic happens like this:

A* load(char* file_name, int* length) {
    FILE* fp = fopen(file_name, "r");
    if (fp == NULL) return NULL;
    size_t ar_length = 500;
    size_t line_counter = 0;
    char** lines = malloc(ar_length);
    char line[256];
    while (fgets(line, sizeof(line), fp)) {
        lines[line_counter] = malloc(256);
        strcpy(lines[line_counter], line);
        line_counter++;
        if(line_counter > sizeof(lines)){
            ar_length *= 2;
            lines = realloc(lines, ar_length);
        }
    }
    *length = line_counter;
    fclose(fp);
    return process(lines, line_counter);
}

A* process(char** lines, int length) {
    A* arr = (A*) malloc(length * sizeof(A));
    for (int i = 0; i < length; ++i) {
        char* line = lines[i];
        char* ptr = strtok(line, ";");
        A a;
        a.id = i;
        int j = 0;
        while (ptr != NULL) {
            if (j == 0) {
                a.name = ptr;
            } else if (j == 1) {
                a.desc = ptr;
            }
            j++;
            ptr = strtok(NULL, ";");
        }
        arr[i] = a;
    }
    return arr;
}

My program works just fine, however, I got memory leaks for each loaded line because of the malloc(256), and I also got memory leak after using realloc(lines, ar_length); I'm quite unsure why I get these leaks because isn't it supposed to "automatically free" the memory after a function runs? How can I fix these memory leaks? Thanks in advance.

Use `strdup` rather than allocating a buffer of arbitrary length, hoping it will work. And simply free all of the allocations at the end. Also, `sizeof(lines)` does not do what you think it does and will quickly blow up your memory if `line_counter` gets bigger than your pointer size (probably 8). — Cheatah, Nov 24 '21 at 21:07
Also, the last token of every line is going to contain a newline character. I'm not sure whether that is what you want/need/expect. — Cheatah, Nov 24 '21 at 21:09
This is just plain wrong: `A* arr = (A*) malloc(length * sizeof(length));` — Cheatah, Nov 24 '21 at 21:50
I tried to call free on arr at the end of process function, but then my array will not hold the data anymore correctly — D. Albert, Nov 24 '21 at 22:28
@Cheatah can you please give me solutions to the problems you stated? — D. Albert, Nov 24 '21 at 22:33

score 0 · Answer 1 · answered Nov 25 '21 at 08:57

There are several general problems with your code.

Casting result of malloc is considered a bad practice. See this for details.
Realloc can fail and return NULL without freeing the original pointer, you are not taking it into consideration. A better way would be +- this:

int* x = malloc(200*sizeof(int));
//whatever
int* bigger_x = realloc(x, 400*sizeof(int));
if(bigger_x) x = bigger_x;
else handle_error("realloc");

As for points more specific to your question and problems:

I'm quite unsure why I get these leaks because isn't it supposed to "automatically free" the memory after a function runs? How can I fix these memory leaks?

No, it's not. Generally you should clean up the used memory once you're done with using it. When? Good question ;) Note, that you're using pointers to the dynamically allocated array, e.g. here: a.desc = ptr;. A trivial solution would be to stringduplicate each of those string, i.e. a.desc = strdup(ptr);, then free lines after process() returns. Note, that you strdup actually does allocate, so you need to release the resources allocated by it once you're done with them.

Use valgrind or address sanitizer. Simply compile with debug symbols and -fsanitize=address, that will give you plenty of info of what and where is leaking. Conteporary version of clang and gcc support it. Valgrind is also a way to go, but it's way less convenient imho.

Bonus point: C does not have RAII, but you can utilize some basic "scope guard" techniques anyway. Consider the following code:

char* buffer = malloc(300);
if(buffer) {
//do whatever
}
free(buffer);

Should you forget about free() at the end, you'll have a memleak. What you can do instead is use for loop in slightly creative way to introduce a "scope guard":

for(char* buffer = malloc(300); buffer; free(buffer), buffer=NULL)
{
  //do stuff
}

Similar technique should also works for FILE* and other resources or can be generalized using macros, it's for you to decide whether you like it or not.

How can I prevent the current memory leaks in my C program?

1 Answers1