My little C program is iterating through the working directory's contents and there is no problem with that, but I checked it with Valgrind and there are some mysterious leaks that I can't make disappear for days.
So here is the code:
struct record {
char* fullPath;
int wdlen; // working directory length
int relplen; // relative path length
int namlen; // name length
bool folder; // is folder
long modified;
};
void populateDB() {
struct record** records = NULL;
int entryCount = 0;
bool error = false;
listDirRecursive(workingDirectory, &entryCount, &records, &error);
if (error) {
goto cleanup;
}
for (int i = 0; i < entryCount; i++) {
printfColor(C_MAGENTA, "%d. wdlen: %d,\t\trelPathLen: %d,\t\tnamelen: %d,\t\tfolder: %s,\t\tmod: %ld,\t\tpath: %s\n",
i, records[i]->wdlen, records[i]->relplen, records[i]->namlen, records[i]->folder ? "true" : "false", records[i]->modified, records[i]->fullPath);
}
// initDatabase(&entryCount, &records);
cleanup:
for (int i = 0; i < entryCount; i++) {
free(records[i]->fullPath);
free(records[i]);
}
free(records);
}
void listDirRecursive(const char* path, int* count, struct record*** records, bool* error) {
if (*error) return;
struct dirent** fileList = NULL;
int fileCount = scandir(path, &fileList, NULL, alphasort);
if (fileCount < 0) {
printfColor(C_RED, "Couldn't open: %s\n", path);
*error = true;
return;
}
struct record** tmpRecords = malloc(fileCount * sizeof *tmpRecords);
if (!tmpRecords) {
printfColor(C_RED, "Couldn't allocate memory for records\n");
*error = true;
return;
}
struct record* act = NULL;
struct stat filestat;
int processedFiles = 0;
for (int i = 0, wdlen = strlen(workingDirectory), plen = strlen(path); i < fileCount; i++) {
if (strcmp(fileList[i]->d_name, ".") == 0 ||
strcmp(fileList[i]->d_name, "..") == 0) continue;
act = malloc(sizeof *act);
if (!act) {
printfColor(C_RED, "Couldn't allocate memory for actual record\n");
*error = true;
goto cleanup;
}
act->fullPath = malloc(plen + fileList[i]->d_namlen + 2);
if (!act->fullPath) {
printfColor(C_RED, "Couldn't allocate memory for actual record's full path\n");
*error = true;
goto cleanup;
}
strcat(strcat(strcpy(act->fullPath, path), "/"), fileList[i]->d_name);
act->wdlen = wdlen + 1;
act->relplen = strlen(act->fullPath) - ((wdlen + 1) + fileList[i]->d_namlen);
act->namlen = fileList[i]->d_namlen;
act->folder = fileList[i]->d_type == DT_DIR;
stat(act->fullPath, &filestat);
act->modified = filestat.st_mtimespec.tv_nsec;
tmpRecords[processedFiles++] = act;
if (fileList[i]->d_type == DT_DIR) {
char* p = malloc(plen + fileList[i]->d_namlen + 2);
strcat(strcat(strcpy(p, path), "/"), fileList[i]->d_name);
listDirRecursive(p, count, records, error);
free(p);
}
}
if (processedFiles == 0) {
goto cleanup;
}
struct record** shrinkedTmpRecords = realloc(tmpRecords, processedFiles * sizeof *shrinkedTmpRecords);
if (!shrinkedTmpRecords) {
if (verbose) printf("Shrinking tmpRecords failed. Some memory leak will happen\n");
}
else {
tmpRecords = shrinkedTmpRecords;
}
int newCount = *count + processedFiles;
if (*count == 0) {
*records = tmpRecords;
}
else {
struct record** newRecords = realloc(*records, newCount * sizeof *newRecords);
if (!newRecords) {
printfColor(C_RED, "Couldn't allocate memory for further records\n");
*error = true;
goto cleanup;
}
*records = newRecords;
for (int i = 0, j = *count; i < processedFiles; i++, j++) {
(*records)[j] = tmpRecords[i];
}
}
*count = newCount;
cleanup:
for (int i = 0; i < fileCount; i++) {
free(fileList[i]);
}
free(fileList);
}
After running, Valgrind's output is this:
==40861== HEAP SUMMARY:
==40861== in use at exit: 18,084 bytes in 170 blocks
==40861== total heap usage: 423 allocs, 253 frees, 180,357 bytes allocated
==40861==
==40861== 8 bytes in 1 blocks are definitely lost in loss record 1 of 43
==40861== at 0x100111FD6: realloc (in /usr/local/Cellar/valgrind/HEAD-e0af3eb/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==40861== by 0x10000162B: listDirRecursive (filesystem.c:137)
==40861== by 0x1000015DA: listDirRecursive (filesystem.c:127)
==40861== by 0x1000015DA: listDirRecursive (filesystem.c:127)
==40861== by 0x1000015DA: listDirRecursive (filesystem.c:127)
==40861== by 0x1000015DA: listDirRecursive (filesystem.c:127)
==40861== by 0x100001092: populateDB (filesystem.c:44)
==40861== by 0x100000E15: init (mtl.c:111)
==40861== by 0x100000CD2: main (mtl.c:61)
==40861==
==40861== 16 bytes in 2 blocks are definitely lost in loss record 5 of 43
==40861== at 0x100111FD6: realloc (in /usr/local/Cellar/valgrind/HEAD-e0af3eb/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==40861== by 0x10000162B: listDirRecursive (filesystem.c:137)
==40861== by 0x1000015DA: listDirRecursive (filesystem.c:127)
==40861== by 0x1000015DA: listDirRecursive (filesystem.c:127)
==40861== by 0x1000015DA: listDirRecursive (filesystem.c:127)
==40861== by 0x100001092: populateDB (filesystem.c:44)
==40861== by 0x100000E15: init (mtl.c:111)
==40861== by 0x100000CD2: main (mtl.c:61)
==40861==
==40861== 72 bytes in 5 blocks are definitely lost in loss record 22 of 43
==40861== at 0x100111FD6: realloc (in /usr/local/Cellar/valgrind/HEAD-e0af3eb/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==40861== by 0x10000162B: listDirRecursive (filesystem.c:137)
==40861== by 0x1000015DA: listDirRecursive (filesystem.c:127)
==40861== by 0x1000015DA: listDirRecursive (filesystem.c:127)
==40861== by 0x100001092: populateDB (filesystem.c:44)
==40861== by 0x100000E15: init (mtl.c:111)
==40861== by 0x100000CD2: main (mtl.c:61)
==40861==
==40861== 80 bytes in 1 blocks are definitely lost in loss record 23 of 43
==40861== at 0x100111FD6: realloc (in /usr/local/Cellar/valgrind/HEAD-e0af3eb/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==40861== by 0x10000162B: listDirRecursive (filesystem.c:137)
==40861== by 0x100001092: populateDB (filesystem.c:44)
==40861== by 0x100000E15: init (mtl.c:111)
==40861== by 0x100000CD2: main (mtl.c:61)
==40861==
==40861== 112 bytes in 4 blocks are definitely lost in loss record 26 of 43
==40861== at 0x100111FD6: realloc (in /usr/local/Cellar/valgrind/HEAD-e0af3eb/lib/valgrind/vgpreload_memcheck-amd64-darwin.so)
==40861== by 0x10000162B: listDirRecursive (filesystem.c:137)
==40861== by 0x1000015DA: listDirRecursive (filesystem.c:127)
==40861== by 0x100001092: populateDB (filesystem.c:44)
==40861== by 0x100000E15: init (mtl.c:111)
==40861== by 0x100000CD2: main (mtl.c:61)
==40861==
==40861== LEAK SUMMARY:
==40861== definitely lost: 288 bytes in 13 blocks
==40861== indirectly lost: 0 bytes in 0 blocks
==40861== possibly lost: 0 bytes in 0 blocks
==40861== still reachable: 0 bytes in 0 blocks
==40861== suppressed: 17,796 bytes in 157 blocks
The 288 bytes divided by 8 (the size of the pointer) is 36, which is the total records (files and folders) at the end. The 13 blocks count equals the number of folders.
I tried everything that came to my mind, but nothing worked, I'm learning C, so it's possible that I don't know something essential (I read a lot in the topic and tried to avoid the big no-nos).
If somebody doesn't understand what is that printfColor
, here it is:
// header file:
enum Color {
C_RED,
C_BOLD_RED,
C_GREEN,
C_BOLD_GREEN,
C_YELLOW,
C_BOLD_YELLOW,
C_BLUE,
C_BOLD_BLUE,
C_MAGENTA,
C_BOLD_MAGENTA,
C_CYAN,
C_BOLD_CYAN,
C_RESET
};
static const char* colors[] = {
"\033[0;31m", // RED
"\033[1;31m", // BOLD RED
"\033[0;32m", // GREEN
"\033[1;32m", // BOLD GREEN
"\033[0;33m", // YELLOW
"\033[01;33m", // BOLD YELLOW
"\033[0;34m", // BLUE
"\033[1;34m", // BOLD BLUE
"\033[0;35m", // MAGENTA
"\033[1;35m", // BOLD MAGENTA
"\033[0;36m", // CYAN
"\033[1;36m", // BOLD CYAN
"\033[0m" // RESET
};
// c file:
void printfColor(enum Color color, const char* format, ...) {
va_list args;
printf("%s", colors[color]);
va_start(args, format);
vprintf(format, args);
va_end(args);
printf("%s", colors[C_RESET]);
}
Chears
Shawn is correct in the comments. tmpRecords is allocated and used as a set of pointers:
struct record** shrinkedTmpRecords = realloc(tmpRecords, processedFiles * sizeof *shrinkedTmpRecords);
if (!shrinkedTmpRecords) {
if (verbose) printf("Shrinking tmpRecords failed. Some memory leak will happen\n");
}
else {
tmpRecords = shrinkedTmpRecords;
}
Then, if count is 0, you are just assigning records
to tmpRecords
:
if (*count == 0) {
*records = tmpRecords;
}
Later this will be freed up top. Here you free the elements of records
as well as the set of pointers that were allocated to hold them.
for (int i = 0; i < entryCount; i++) {
free(records[i]->fullPath);
free(records[i]);
}
free(records);
If the count is not zero in during an iteration, we allocate more space for records. Then, you loop through tmpRecords
and assign the elements to records
. These elements will be freed when records
is freed, but the set of pointers allocated for tmpRecords
itself are never freed. I've added a line in the code below that will fix this leak.
if (*count == 0) {
*records = tmpRecords;
}
else {
struct record** newRecords = realloc(*records, newCount * sizeof *newRecords);
if (!newRecords) {
printfColor(C_RED, "Couldn't allocate memory for further records\n");
*error = true;
goto cleanup;
}
*records = newRecords;
for (int i = 0, j = *count; i < processedFiles; i++, j++) {
(*records)[j] = tmpRecords[i];
}
free(tmpRecords); /*** THIS LINE ***/
}