I am making my first attempt at utilizing linked lists and dynamic memory allocation, and I have a function that is supposed to load a dictionary into memory so that it can be used for spellchecking (this is from pset 5 in cs50). The problem is that I have a segmentation fault and a valgrind error. The valgrind error (invalid memory read of size 4) is pinpointed at the fread() function in the while loop, but I don't really know how that could be the case, since the code seems so straightforward to me. I am obviously missing something both crucial and basic, but I don't even know how to go about finding the solution to this particular problem. Here is the function, as I have defined it:
bool load(constchar* dictionary)
{
FILE* dicfile=fopen("dictionary","r");
//allocates one node of memory, then defines a pointer to that first node of the tree
//neglected error check for sake of speed
dicstart=calloc(1,sizeof(node));
memhead=malloc(sizeof(listnode));
memcrawler=memhead;
memcrawler->memory=dicstart;
memcrawler->next=NULL;
//creates a pointer that will travel along the tree paths to input data
node* crawler=dicstart;
int8_t letter;
//reads in the dictionary, one character at a time
while(fread(&letter,sizeof(char),1,dicfile)==1)
{
//lowercase letters and apostrophe
if ((letter>96 && letter<123) || letter==39)
{
if (letter==39) //apostrophe maps to 0
letter=0;
else
letter=(letter%32); //maps ascii values of lowercase to a=1 b=2 ... etc
//if pointer at letter location does not exist, make one. Otherwise travell along pointer
if (crawler->alphabet[letter]==NULL)
{
crawler->alphabet[letter]=calloc(1,sizeof(node));
crawler=crawler->alphabet[letter];
//index the newly allocated memory with a linked memory pointer list
memcrawler->next=malloc(sizeof(listnode));
memcrawler=memcrawler->next;
memcrawler->memory=crawler;
memcrawler->next=NULL;
}
else
{
crawler=crawler->alphabet[letter];
}
}
else
{
if(crawler!=dicstart)
{
crawler->isword=true;
crawler=dicstart;
words++;
printf("%i\n",words);
}
}
}
fclose(dicfile);
returntrue;
}
¿what for? If trying to read from a failed stream, the operation would simply fail.
Edit: oh, right, C. don't know what happen if trying to read from NULL
@OP: provide a testcase that reproduce your issue.
¿what are `dicstart', `memhead' and `memcrawler'?
¿why are they global?
¿is this a trie?
Also
1 2 3
//lowercase letters and apostrophe
// if ((letter>96 && letter<123) || letter==39)
if (islower(letter) || letter=='\'')
@cire: Yeah, that was a really stupid mistake on my part. I just had to remove the quotes from the fread input and everything worked fine. Thanks for pointing that out.
@ne55: yes, this is a trie and dicstart, memhead, and memcrawler are all pointers to parts of the try. The reason they are global is because I needed to access them in multiple functions, but I am not allowed to change the declaration of any of the functions in order to accept different arguments, so I just declared the things that I needed in the associated .h file and then defined them at the top of the .c file. If there is a better way/more conventional way to go about it, I would like to know. I'm teaching myself programming, so I don't get a lot of input on things like conventional practices and style tips.