Tom Tom - 3 months ago 7
C Question

C returns invalid address in loop

I am looping through my file structure with a C program. It I hit a new folder, i appending it's path to a linked list so I can look through all subdirectories iteratively.

The program consists of:

main function, calling the iterative function (which loops through the files)

When i loop through all the files once everything works fine. However when i have a while loop in my main function to call the iterative function more often, it always fails on the second time due to segmenatation error.

So i was investigating a bit and it seems that one element of the linked list is having an invalid address.

All addresses of my elements have this format and length: 0x2398ff0 or 0x2398ee0

However the illegal pointer has an address from 0x7f3770304c58

Does anyone have any thoughts why this address is so long?

I have checked throught printf("%p", element) every new address that gets added to the linked list, and this address never appear anywhere before in the code. It like magically appears.

I was thinking about a wild pointer maybe, but after i free any pointer i set it to NULL to, which should prevent this right?

Thanks for any tip. I haven't posted the code right now cause it is very long and thought maybe there are obvious things i just dont see.

EDIT: the entire code, including main function

#include <stdio.h>
#include <stdlib.h>
#include <dirent.h>
#include <string.h>
#include <errno.h>
#include <unistd.h>

void iterativefile(FILE *f, char** field, int looper){

DIR *d;
struct dirent *dir;

typedef struct nextpath { // Define element type of linked list
char *thispath;
struct nextpath *next;
}nextpath;

nextpath *startpath = malloc(sizeof(nextpath));
char * beginning = (char *) malloc(2); //create first element in linked list, starting on root node "."
strcpy(beginning, ".");
startpath->thispath = beginning;

int found = 0;

nextpath *currentzeiger = startpath;
nextpath *firstelement = startpath;
char *newdir, *currentfile, *currentpath;

do {
currentpath = currentzeiger->thispath;
d = opendir(currentpath);
if (!d){ //IF the path is invalid or cannot be opened

firstelement = currentzeiger->next;
free(currentzeiger);
currentzeiger = firstelement;
continue;
}
while((dir = readdir(d)) != NULL){
if (dir->d_type != DT_REG){ // current element is a directory -> add it to linked list
if (strcmp(dir->d_name, ".") != 0 && strcmp(dir->d_name, "..") != 0){
newdir = (char *) malloc(2+strlen(currentpath) + strlen(dir->d_name));
strcpy(newdir, currentpath);
strcat(newdir, "/");
strcat(newdir, dir->d_name);
nextpath *new = malloc(sizeof(nextpath)); // add new folder to linked list
new->thispath = NULL;
new->thispath = strdup(newdir);
new->next = currentzeiger->next;
currentzeiger->next = new;
free(newdir);
newdir = NULL;
}
}
else { // current element is a file -> check if already included in list, if not, add it
currentfile = (char *) malloc(2+strlen(currentpath)+strlen(dir->d_name));
strcpy(currentfile, currentpath);
strcat(currentfile, "/");
strcat(currentfile, dir->d_name);
found = 0;
if (field != NULL) {
for (int z = 0; z < looper; z++){
if (field[z] != NULL){
if(strcmp(currentfile,field[z]) == 0){
found = 1;
free(field[z]);
field[z] = NULL;
}
}
}
}
if (found == 0){
char *renamefile = (char *) malloc(strlen(currentpath) + 6);
strcpy(renamefile, currentpath);
strcat(renamefile, ".cbsm");
free(renamefile);
renamefile = NULL;

}
free(currentfile);
currentfile = NULL;
}
}

firstelement = currentzeiger->next;
free(currentzeiger->thispath);
currentzeiger->thispath = NULL;
free(currentzeiger);

currentzeiger = firstelement;
closedir(d);

}while(currentzeiger != NULL);
}



int main()
{
int counterofwhile = 1;
while(1){
printf("Loop number: %d\n", counterofwhile);
counterofwhile++;
FILE *fp = fopen("datasyn.txt", "rw+");
if (fp == NULL) {
printf("FILE ERROR");
FILE *fp = fopen("datasyn.txt", "ab+");
iterativefile(fp, NULL, 0);
}
else {
int lines = 0;
int ch = 0;
int len = 0;
int max_len = 0;
while((ch = fgetc(fp)) != EOF){
++len;
if (ch == '\n'){
if(max_len < len)
max_len = len;
++lines;
len = 0;
}
}

if (len)
++lines;
fprintf(stderr, "%d lines\n", lines);
if (lines > 0){
int numProgs = 0;
char *programs[lines];
char line[max_len + 1];
rewind(fp);
while(fgets(line, sizeof(line), fp)){
int new_line = strlen(line) - 1;
if (line[new_line] == '\n')
line[new_line] = '\0';
programs[numProgs++] = strdup(line);
}

iterativefile(fp, programs, numProgs);
for (int j = 0; j < numProgs; j++){
free(programs[j]);
}
}
else {
iterativefile(fp, NULL, 0);
}
sleep(1);


printf("Done\n");
fclose(fp);
}

}
return 0;
}

Answer

In the function iterativefile(), you don't use calloc() to allocate startpath and you don't set startpath->next to null. The memory returned by malloc() is not necessarily zeroed. When you subsequently use startpath->next, all hell breaks loose.

You also don't use the file pointer passed into iterativefile(). When you remove the parameter from the definition, you change the calls, and you've got a shadowed fp in main() (in the if (fp == NULL) block, you create a new FILE *fp which is really not needed). It really isn't clear what else is meant to happen; you've not given clear instructions on what the program is meant to be doing. I don't have the datasyn.txt file, but it shouldn't matter since the file stream is not used. I got lots of lines like FILE ERRORLoop number: 280 from the code, but no crash where previously I was getting a crash.

Compile with more warning options. I called the file fp17.c and compiled my hacked version using:

gcc -O3 -g -std=c11 -Wall -Wextra -Werror -Wmissing-prototypes -Wstrict-prototypes \
    -Wold-style-definition fp17.c -o fp17

With a few other simple changes (static before the function; int main(void)), the code compiled cleanly (and would have needed -Wshadow to spot the shadowing if it hadn't been for an 'unused variable' warning that pointed me to it).