cs50 pset5 dictionary.c : this code is giving segmentation fault

Question

i have written this code for pset5 dictionary.c

i loaded dictionary in trie data structure. it shows segmentation fault.

/**
 * Implements a dictionary's functionality.
 */

    #include <stdbool.h>
    #include<stdio.h>
    #include<cs50.h>
    #include<string.h>
    #include "dictionary.h"
    typedef struct node
    {
        char n;
        struct node *next;
    }node;
    int words = 0;
    node *head , *temp;


/**
 * Returns true if word is in dictionary else false.
 */
    bool check(const char *word)
    {
        temp = head;
        int count = 0;
        int l = strlen(word);
        for(int i=0;i<l;i++)
        {
            if(temp[word[i]-'a'].n == word[i])
            {
                temp = temp[word[i] - 'a'].next;
                count++;
            }
            else
                break;
        }
        if(count == l)
        {
            return true;
        }
        return false;
    }


/**
 * Loads dictionary into memory. Returns true if successful else false.
 */


bool load(const char *dictionary)
    {
        int i = 0;
        head = temp = malloc(26*sizeof(node));
        for(i=0;i<26;i++)
        {
            head[i].n = '0';
            head[i].next = NULL;
        }
        FILE *ptr = fopen(dictionary,"r");
        if (ptr == NULL)
        {
            printf("Could not open dictionary"); 
            return false;
        }
        while(feof(ptr))
        {
            char *d= NULL;
            fscanf(ptr,"%s",d);
            words++;
            int l = strlen(d);
            for(int j=0;j<l;j++)
            {
                if(temp[d[j] - 'a'].next == NULL)
                {
                    temp[d[j] - 'a'].n = d[j];
                    temp[d[j] - 'a'].next = malloc(26*sizeof(node));
                    temp = temp[d[j] - 'a'].next;
                    for(int k=0;k<26;k++)
                    {
                        temp[k].n = '0';
                        temp[k].next = NULL;
                    }
                }
                else
                {
                    temp = temp[d[j] - 'a'].next;
                }
            }
            temp = head;
        }
        return true;
    }
    /**
     * Returns number of words in dictionary if loaded else 0 if not yet loaded.
     */
    unsigned int size(void)
    {
        if(words != 0)
        return words;
        return 0;
    }
    /**
     * Unloads dictionary from memory. Returns true if successful else false.
     */
    bool unload(void)
    {
        free(head);
        free(temp);
        return true;
    }

but it shows segmentation fault. i tried experimenting with my trie code and it worked fine with other codes. output :-

Segmentation fault

I recommend you read [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) by Eric Lippert, and learn how to use a debugger. — Some programmer dude, Aug 04 '17 at 06:07
You should probably also [read about how to ask good questions](http://stackoverflow.com/help/how-to-ask) and learn how to create a [Minimal, ***Complete***, and Verifiable Example](http://stackoverflow.com/help/mcve). Lastly also please read [Why is “while ( !feof (file) )” always wrong?](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) — Some programmer dude, Aug 04 '17 at 06:09
1) `char *d= NULL; fscanf(ptr,"%s",d);` : Memory secured for storing the input string is necessary for `d`. — BLUEPIXY, Aug 04 '17 at 07:30
2) `temp = temp[d[j] - 'a'].next;` and `temp = head;` are occurs memery leak. — BLUEPIXY, Aug 04 '17 at 07:33
when calling any of the heap allocation functions: (malloc, calloc, realloc) always check (!=NULL) the returned value to assure the operation was successful — user3629249, Aug 04 '17 at 07:36
error messages should be displayed on `stderr`, not `stdout`. One easy way to do this, when the error is from a system function, is to use `perror()` as that outputs to `stderr` AND displays the system message as to what the system thinks is the cause of the error — user3629249, Aug 04 '17 at 07:38
never use `while(feof(..))`. It does not do what your code is expecting it to do. — user3629249, Aug 04 '17 at 07:45
when calling an of the `scanf()` family of functions: 1) always check the returned value (not the parameter values) to assure the operation was successful. 2) when using the '%s' input/format specifier, always include a MAX CHARACTERS modifier that is 1 less than the length of the input buffer. This will avoid any buffer overflow events. Such an event is undefined behavior and can lead to a seg fault event. — user3629249, Aug 04 '17 at 07:48
the function: `size()` can be eliminated by just using the global counter `words` — user3629249, Aug 04 '17 at 07:53
for every call to a heap allocation function (malloc, calloc) there needs to be a call to `free()`. As it is, the posted code has some massive memory leaks. — user3629249, Aug 04 '17 at 07:55

qPCR4vir · Answer 1 · 2017-08-04T07:56:10.613

You may begin by searching for places where you may access a wrong memory location. Search for access through pointers: is the memory allocated and the objects correctly initialized? And the indexes are in range? You may have many other errors, but this is one of then:

 char *d= NULL;
 fscanf(ptr,"%s",d);

you need to first allocate sufficient memory in d. And allways make sure your index is <26 (like d[j] - 'a' in temp[d[j] - 'a'])

cs50 pset5 dictionary.c : this code is giving segmentation fault

1 Answers1