Valgrind error and segmentation fault with fread()

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:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
  bool load(const char* 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);
    return true;
}
I know that it should be clear from the code itself, but this is in C, not C++.
Why don't you check to see if the file is opened instead of just assuming it was?

Also, did you mean to use the argument supplied to the function? (Because you aren't using it.)
¿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=='\'')
Last edited on
@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.

Anyway, thanks for your help everyone.
Topic archived. No new replies allowed.