Opening and closing a log file leads to memory leaks in C

Question

Debugging a legacy code. I have a program written in C. it's actually much longer but I have created a small reproducible program to show the issues (please ignore the fact that this program does not make a lot of sense). The program:

#include <sys/types.h>
#include <sys/stat.h>
#include <string.h>
#include <malloc.h>
#include <ctype.h>
#include <glib.h>
#include <stdlib.h>
#include <unistd.h>
#include <dirent.h>
#include <stdio.h>


#define FILE_SEP "/"
#define LENGTH 10

FILE *log_file;
char TMP[LENGTH];
char RESULT_DIRECTORY[LENGTH + 12];

int open_log() {
    GString *path = g_string_new(RESULT_DIRECTORY);
    g_string_append(path, FILE_SEP);
    g_string_append(path, TMP);
    g_string_append(path,".usr");
    log_file = fopen(path->str,"w");
    if (!log_file)
        return 1;
    
    return 0;
}

void close_log() {
    fclose(log_file);
}


int main(int argc, char *argv[]) {
    strcpy(TMP, "tmp");
    strcpy(RESULT_DIRECTORY, "/home/");
    if (open_log()) {
        return 1;
    }
    close_log();
    return 0;
}

Running valgrind I get:

> valgrind --tool=memcheck --track-origins=yes --leak-check=full --show-reachable=yes program
==84011== Memcheck, a memory error detector
==84011== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==84011== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==84011== Command: c_status_reader
==84011== 
==84011== 
==84011== HEAP SUMMARY:
==84011==     in use at exit: 5,292 bytes in 9 blocks
==84011==   total heap usage: 10 allocs, 1 frees, 5,860 bytes allocated
==84011== 
==84011== 16 bytes in 1 blocks are possibly lost in loss record 1 of 7
==84011==    at 0x4C29104: malloc (vg_replace_malloc.c:299)
==84011==    by 0x4C29278: realloc (vg_replace_malloc.c:785)
==84011==    by 0x5078ABD: g_realloc (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x5092B13: ??? (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x5093C99: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x5093D14: g_string_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x108BE0: open_log (program.c:21)
==84011==    by 0x108A6E: main (program.c:40)
==84011== 
==84011== 252 bytes in 1 blocks are still reachable in loss record 2 of 7
==84011==    at 0x4C27393: calloc (vg_replace_malloc.c:711)
==84011==    by 0x5078B39: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x508CCE2: ??? (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x508EB5C: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x5093C6A: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x5093D14: g_string_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x108BE0: open_log (program.c:21)
==84011==    by 0x108A6E: main (program.c:40)
==84011== 
==84011== 496 bytes in 1 blocks are possibly lost in loss record 3 of 7
==84011==    at 0x4C2710E: memalign (vg_replace_malloc.c:858)
==84011==    by 0x4C271A9: posix_memalign (vg_replace_malloc.c:1021)
==84011==    by 0x508D4B0: ??? (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x508ECE2: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x5093C6A: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x5093D14: g_string_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x108BE0: open_log (program.c:21)
==84011==    by 0x108A6E: main (program.c:40)
==84011== 
==84011== 504 bytes in 1 blocks are still reachable in loss record 4 of 7
==84011==    at 0x4C27393: calloc (vg_replace_malloc.c:711)
==84011==    by 0x5078B39: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x508CCC3: ??? (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x508EB5C: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x5093C6A: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x5093D14: g_string_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x108BE0: open_log (program.c:21)
==84011==    by 0x108A6E: main (program.c:40)
==84011== 
==84011== 504 bytes in 1 blocks are still reachable in loss record 5 of 7
==84011==    at 0x4C27393: calloc (vg_replace_malloc.c:711)
==84011==    by 0x5078B39: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x508CD2A: ??? (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x508EB5C: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x5093C6A: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x5093D14: g_string_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x108BE0: open_log (program.c:21)
==84011==    by 0x108A6E: main (program.c:40)
==84011== 
==84011== 1,488 bytes in 3 blocks are possibly lost in loss record 6 of 7
==84011==    at 0x4C2710E: memalign (vg_replace_malloc.c:858)
==84011==    by 0x4C271A9: posix_memalign (vg_replace_malloc.c:1021)
==84011==    by 0x508D4B0: ??? (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x508ED22: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x5093C6A: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x5093D14: g_string_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x108BE0: open_log (program.c:21)
==84011==    by 0x108A6E: main (program.c:40)
==84011== 
==84011== 2,032 bytes in 1 blocks are still reachable in loss record 7 of 7
==84011==    at 0x4C27393: calloc (vg_replace_malloc.c:711)
==84011==    by 0x5078B39: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x508EB12: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x5093C6A: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x5093D14: g_string_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==84011==    by 0x108BE0: open_log (program.c:21)
==84011==    by 0x108A6E: main (program.c:40)
==84011== 
==84011== LEAK SUMMARY:
==84011==    definitely lost: 0 bytes in 0 blocks
==84011==    indirectly lost: 0 bytes in 0 blocks
==84011==      possibly lost: 2,000 bytes in 5 blocks
==84011==    still reachable: 3,292 bytes in 4 blocks
==84011==         suppressed: 0 bytes in 0 blocks
==84011== 
==84011== For counts of detected and suppressed errors, rerun with: -v
==84011== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 6 from 5)

I think that I understand the problem - it's missing the g_string_free call somewhere. But, when I actually use it, it says: still reachable: 5,276 bytes in 8 blocks. For example I have only the following open_log (and without calling close_log):

int open_log() {
    GString *path = g_string_new(RESULT_DIRECTORY);
    g_string_append(path, FILE_SEP);
    g_string_free(path,TRUE);
    //log_file = fopen(path->str,"w");
    //if (!log_file)
    //    return 1;
    return 0;
}

I get:

==88254==
==88254== LEAK SUMMARY:
==88254==    definitely lost: 0 bytes in 0 blocks
==88254==    indirectly lost: 0 bytes in 0 blocks
==88254==      possibly lost: 0 bytes in 0 blocks
==88254==    still reachable: 5,276 bytes in 8 blocks
==88254==         suppressed: 0 bytes in 0 blocks

But, if I remove g_string_append(path, FILE_SEP); then it actually works! So I'm guessing, when I do g_string_free, it only removes the initial string path and not the append part. Is it correct? How can I solve the issues?

EDIT:

Actually the first think I tried was to add g_string_free(path, TRUE); after the fopen, but I still see memory leaks. The code:

#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <dirent.h>
#include <stdio.h>
#include<string.h>
#include<malloc.h>
#include<ctype.h>
#include<glib.h>
#include<stdlib.h>


#define FILE_SEP "/"
#define LENGTH 10

FILE *log_file;
char TMP[LENGTH];
char RESULT_DIRECTORY[LENGTH + 12];

int open_log(void) {
    GString *path = g_string_new(RESULT_DIRECTORY);
    g_string_append(path, FILE_SEP);
    g_string_append(path, TMP);
    g_string_append(path, ".usr");
    log_file = fopen(path->str, "w");
    g_string_free(path, TRUE);
    
    if (!log_file)
        return 1;
    
    return 0;
}

void close_log() {
    if (log_file) {
        fclose(log_file);
        log_file = NULL;
    }
}


int main(int argc, char *argv[]) {
    strcpy(TMP, "tmp");
    strcpy(RESULT_DIRECTORY, "/home/");
    if (open_log()) {
        return 1;
    }
    close_log();
    return 0;
}

The valgrind output:

==23525== Memcheck, a memory error detector
==23525== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==23525== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==23525== Command: c_status_reader
==23525== 
==23525== 
==23525== HEAP SUMMARY:
==23525==     in use at exit: 5,276 bytes in 8 blocks
==23525==   total heap usage: 10 allocs, 2 frees, 5,860 bytes allocated
==23525== 
==23525== 252 bytes in 1 blocks are still reachable in loss record 1 of 6
==23525==    at 0x4C27393: calloc (vg_replace_malloc.c:711)
==23525==    by 0x5078B39: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x508CCE2: ??? (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x508EB5C: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x5093C6A: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x5093D14: g_string_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x108C31: open_log (program.c:21)
==23525==    by 0x108ABC: main (program.c:45)
==23525== 
==23525== 496 bytes in 1 blocks are still reachable in loss record 2 of 6
==23525==    at 0x4C2710E: memalign (vg_replace_malloc.c:858)
==23525==    by 0x4C271A9: posix_memalign (vg_replace_malloc.c:1021)
==23525==    by 0x508D4B0: ??? (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x508ECE2: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x5093C6A: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x5093D14: g_string_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x108C31: open_log (program.c:21)
==23525==    by 0x108ABC: main (program.c:45)
==23525== 
==23525== 504 bytes in 1 blocks are still reachable in loss record 3 of 6
==23525==    at 0x4C27393: calloc (vg_replace_malloc.c:711)
==23525==    by 0x5078B39: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x508CCC3: ??? (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x508EB5C: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x5093C6A: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x5093D14: g_string_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x108C31: open_log (program.c:21)
==23525==    by 0x108ABC: main (program.c:45)
==23525== 
==23525== 504 bytes in 1 blocks are still reachable in loss record 4 of 6
==23525==    at 0x4C27393: calloc (vg_replace_malloc.c:711)
==23525==    by 0x5078B39: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x508CD2A: ??? (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x508EB5C: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x5093C6A: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x5093D14: g_string_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x108C31: open_log (program.c:21)
==23525==    by 0x108ABC: main (program.c:45)
==23525== 
==23525== 1,488 bytes in 3 blocks are still reachable in loss record 5 of 6
==23525==    at 0x4C2710E: memalign (vg_replace_malloc.c:858)
==23525==    by 0x4C271A9: posix_memalign (vg_replace_malloc.c:1021)
==23525==    by 0x508D4B0: ??? (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x508ED22: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x5093C6A: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x5093D14: g_string_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x108C31: open_log (program.c:21)
==23525==    by 0x108ABC: main (program.c:45)
==23525== 
==23525== 2,032 bytes in 1 blocks are still reachable in loss record 6 of 6
==23525==    at 0x4C27393: calloc (vg_replace_malloc.c:711)
==23525==    by 0x5078B39: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x508EB12: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x5093C6A: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x5093D14: g_string_new (in /usr/lib64/libglib-2.0.so.0.2200.5)
==23525==    by 0x108C31: open_log (program.c:21)
==23525==    by 0x108ABC: main (program.c:45)
==23525== 
==23525== LEAK SUMMARY:
==23525==    definitely lost: 0 bytes in 0 blocks
==23525==    indirectly lost: 0 bytes in 0 blocks
==23525==      possibly lost: 0 bytes in 0 blocks
==23525==    still reachable: 5,276 bytes in 8 blocks
==23525==         suppressed: 0 bytes in 0 blocks
==23525== 
==23525== For counts of detected and suppressed errors, rerun with: -v
==23525== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 6 from 5)

Why it didn't solve it? I even tries to set path as global var and call the g_string_free in the close_log function. But it didn't work.

Answer 1

Your first assumption is right, you do not call g_string_free neither in open_log nor in close_log, thus you get a memory leak.

As for

But, when I actually use it, it says: still reachable: 5,276 bytes in 8 blocks.. For example I have only the following open_log (and without calling close_log):

The last sentence in the quote is the answer. You do not call close_log, thus the FILE *log_file leaks now.

if I remove g_string_append(path, FILE_SEP); then it actually works!

If you remove g_string_append then log_file = fopen(path->str,"w"); is unable to open a file because it is a directory, fopen returns NULL and nothing leaks.

If you keep getting memory leaks after all fixes, then you see valgrind's false positive reports regarding glib internal stuff. The glib developers were so kind that they provided the special file for suppressing such false positive reports in valgrinds:

/usr/share/glib-2.0/valgrind/glib.supp

This path may vary, look for glib.supp, use the command locate valgrind/glib.supp. Use the file with valgrind:

valgrind --suppressions=/usr/share/glib-2.0/valgrind/glib.supp <...>

Answer 2

path is allocated with g_string_new in open_log(), but it is not freed nor stored elsewhere before the function returns, hence causing a memory leak.

Free this string with g_string_free before returning:

int open_log(void) {
    GString *path = g_string_new(RESULT_DIRECTORY);
    g_string_append(path, FILE_SEP);
    g_string_append(path, TMP);
    g_string_append(path, ".usr");
    log_file = fopen(path->str, "w");
    g_string_free(path, TRUE);

    if (!log_file)
        return 1;
    
    return 0;
}

For cleanliness I would recommend setting log_file to NULL after it is closed:

void close_log(void) {
    if (log_file) {
        fclose(log_file);
        log_file = NULL;
    }
}

This should fix the block no longer accessible problem, but there may be some blocks allocated by the Glib for its own bookkeeping or by a successful call to fopen still lingering for Valgrind to report.

EDIT: after fixing the memory leak, there are still 8 blocks allocated and reachable when the program exits: these blocks are all allocated by g_string_new probably for ancillary data in the GLib string package. This package is known to have some issues with valgrind, there might be ways to reduce the noise, but you can ignore these blocks for you purpose.

Using the GLib for just this purpose is not necessary. To avoid its side effects, here is an alternative:

int open_log(void) {
    char path[1024];
    snprintf(path, sizeof(path), "%s%s%s.usr",
             RESULT_DIRECTORY, FILE_SEP, TMP);
    log_file = fopen(path, "w");
    if (!log_file)
        return 1;
    
    return 0;
}