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.
CodePudding user response:
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 followingopen_log
(and without callingclose_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 <...>
CodePudding user response:
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.