The issue:
I have functions (add_filename()
and init_filename()
) that I'm currently testing. They have to read lines from a file into a dynamically allocated array of strings. Init starts the array, and Add adds new elements. Both of these functions return a pointer to the beginning of the array. I have taken all the necessary precautions to make sure realloc()
does not lose the pointer. There are also flags that indicate that something is wrong with the memory allocation. And I free everything (that i can think of). Yet, it is still leaking...
Code:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
char **add_filename(char **filenames, char *new_file, int *file_num, int *flag);
char **init_filename(char *new_file, int *flag);
int main() {
FILE *file;
file = fopen("file.txt", "r");
char *buffer = 0;
size_t buf_size = 0;
size_t chars = 0;
int file_num = 0, check = 1;
// char ch;
if (file != NULL) {
char **files;
while ((int)(chars = getline(&buffer, &buf_size, file)) > 0) {
if (!file_num) {
files = init_filename(buffer, &check);
file_num ;
}
files = add_filename(files, buffer, &file_num, &check);
printf("files = %s", files[file_num - 1]);
free(buffer);
buffer = NULL;
if (check == 0) {
printf("we have problems\n");
break;
}
}
free(buffer);
buffer = NULL;
fclose(file);
if (files) {
for (int i = 0; i < file_num; i ) {
free(files[i]);
}
}
}
return 0;
}
char **init_filename(char *new_file, int *flag) {
char **init = malloc((1) * sizeof(char*)); //
if (init) {
init[0] = malloc((strlen(new_file) 1) * sizeof(char));
if (!init[0])
*flag = 0;
} else {
*flag = 0;
}
return init;
}
char **add_filename(char **filenames, char *new_file, int *file_num, int *flag) {
char **temp = realloc(filenames, (*file_num 1) * sizeof(char *));
if (temp) {
filenames = temp;
filenames[*file_num] = malloc((strlen(new_file) 1) * sizeof(char));
if (filenames[*file_num] != NULL) {
strcpy(filenames[*file_num], new_file);
*file_num = *file_num 1;
} else {
*flag = 0;
}
} else {
*flag = 0;
}
return filenames;
}
This is the output of valgrind:
==5881== HEAP SUMMARY:
==5881== in use at exit: 32 bytes in 1 blocks
==5881== total heap usage: 15 allocs, 14 frees, 6,285 bytes allocated
==5881==
==5881== 32 bytes in 1 blocks are definitely lost in loss record 1 of 1
==5881== at 0x484DCD3: realloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==5881== by 0x1094E2: add_filename (test.c:72)
==5881== by 0x109335: main (test.c:23)
==5881==
==5881== LEAK SUMMARY:
==5881== definitely lost: 32 bytes in 1 blocks
==5881== indirectly lost: 0 bytes in 0 blocks
==5881== possibly lost: 0 bytes in 0 blocks
==5881== still reachable: 0 bytes in 0 blocks
==5881== suppressed: 0 bytes in 0 blocks
==5881==
==5881== For lists of detected and suppressed errors, rerun with: -s
==5881== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
And this is the output of ASan:
Direct leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x7f200c4b4c18 in __interceptor_realloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:164
#1 0x5591559a2a8a in add_filename /home/licht/Documents/Knowledge/school21/inProgress/C3_SimpleBashUtils-0/src/test.c:72
#2 0x5591559a2631 in main /home/licht/Documents/Knowledge/school21/inProgress/C3_SimpleBashUtils-0/src/test.c:23
#3 0x7f200c029d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
SUMMARY: AddressSanitizer: 32 byte(s) leaked in 1 allocation(s).
CodePudding user response:
There are multiple problems in the code:
files
must be initialized toNULL
: it will be initialized byinit_filename
if the file is not empty but will remain uninitialized and cause undefined behavior when freed at the end ofmain()
(iffiles
is finally freed as it should be).- you should strip the trailing newline left by
getline()
. - There is no need for
init_filename()
: passing a null pointer torealloc()
is allowed andrealloc
will behave likemalloc
in this case. - the first filename is added twice in the array: once by
init_filename
and another time byadd_filename
- there is no need to free
buffer
inside the loop: it will be reused for the next line and reallocated as needed. - allocating a copy of a string should be done with
strdup()
which safely combines memory allocation and string copy in a single call. This function is part of the C23 C Standard and has been available for decades on POSIX systems. It is easy to implement on legacy systems that do not provide it. There is no doubt it is available on your system since you rely ongetline
. - last but not least: you forget to free
files
, which is probably the leak you observe.
Here is a modified version:
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
char **add_filename(char **filenames, const char *new_file,
int *file_num, int *flag);
int main() {
FILE *file;
char *buffer = NULL;
size_t buf_size = 0;
char **files = NULL;
int file_num = 0, check = 1;
file = fopen("file.txt", "r");
if (file == NULL) {
fprintf(stderr, "cannot open %s: %s\n", "file.txt", strerror(errno));
return 1;
}
while (getline(&buffer, &buf_size, file) > 0) {
// clear the trailing newline if any
buffer[strcspn(buffer, "\n")] = '\0';
files = add_filename(files, buffer, &file_num, &check);
if (check == 0) {
printf("filename could not be inserted: %s\n", buffer);
break;
}
printf("file = %s\n", files[file_num - 1]);
}
free(buffer);
fclose(file);
if (files) {
for (int i = 0; i < file_num; i ) {
free(files[i]);
}
free(files);
}
return 0;
}
char **add_filename(char **filenames, const char *new_file,
int *file_num, int *flag) {
int num = *file_num;
char **temp = realloc(filenames, (num 1) * sizeof(*filenames));
if (temp) {
filenames = temp;
filenames[num] = strdup(new_file);
if (filenames[num] != NULL) {
// update the filename count and set the success flag
*file_num = num 1;
*flag = 1;
} else {
// could not allocate new string: clear flag
*flag = 0;
}
} else {
// realloc failed: clear flag to indicate failure
*flag = 0;
}
return filenames;
}