0

I having trouble with load function. I am trying to load dictionary in simple linked list, but in middle of dictionary I get segmentation fault. Please help.

    bool load(const char *dictionary)
{

    FILE *dict = fopen(dictionary, "r");

    //check if file opens
    if(dict == NULL)
    {
        printf("Could not load dictionary");
        return false;
    }

     char buffer[LENGTH+2];

    //gets first string from dictionary
    fgets(buffer, LENGTH, dict);

    //initializing values with first node
    node *begining = newNode(buffer);
    node *end = begining;

    //iterating thought dictionary and adding nodes one after another
    while( fgets(buffer, LENGTH, dict)!=NULL )
    {

        //updates old pointer so at each iteration it points at last node
        end = nodeAdd(end, buffer);
    }


    return true;
}

functions which are called

node *newNode(char *val)
{
    //allocates space in memory for node 
    node *result = malloc( sizeof(node) );

    //allocates spece for string
    char *temp = malloc( sizeof(char*) );

    //assigns pointer of allocated space to node string pointer
    result->word = temp;

    //copies word from buffer
    strncpy(result->word, val, strlen(val));

    //sets last node pointer to NULL
    result->next = NULL;

    return result;
}

node *nodeAdd(node *old, char *val)
{
    node *new = newNode(val);
    old->next = new;

    return new;

}
1

Assuming result->word should hold the word from the dictionary (cannot see struct definition), it is not allocated. Here result->word = temp; you are stuffing in 8 bytes (sizeof(char*)). And here strncpy(result->word, val, strlen(val)); you are stuffing in strlen(val) bytes (which replace/overwrite the previously stuffed bytes). I don't think you need the temp pointer at all. If you allocate result->word properly, you can simply copy val into it. And don't forget a byte for the null-terminator!

1

I am slightly confused by your code. I just finished this, so I could be wrong. I believe most of what I'm going to say is in the video. So I'd watch that again if I were you. Here goes:

1) To make things efficient I think you need 26 linked lists, one for every letter of the alphabet. Essentially an array of linked-lists. Otherwise your checking will be really slow.

2) Your functions have return values, which doesn't make sense to me because linked-lists rely on pointers, you don't need to return values. I found this helpful, just make sure to cite it if you use some of the concepts: http://www.learn-c.org/en/Linked_lists

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .