SEGMENTATION FAULT in strncpy - load from dictionary

Question

I have this function "load" where I read words from a dictionary and put them in an hashtable of linked lists. When I try to read a line and save it in my new_node->text the compiler returns SEGMENTATION FAULT and I don't know why. The error apperars when I use strncpy.

#define HASHTABLE_SIZE 76801


typedef struct node
{
        char text[LENGTH+1];
        //char* text;
        //link to the next word
        struct node* next_word;
}
node;


    node* hashtable[HASHTABLE_SIZE];

    bool load(const char* dictionary)
    {
        FILE* file = fopen(dictionary,"r");
        unsigned long index = 0;
        char str[LENGTH+1];

        if(file == NULL)
        {
            printf("Error opening file!");
            return false;
        }

        while(! feof(file))
        {
            node * new_node = malloc(sizeof(node)+1000);


            while( fscanf(file,"%s",str) > 0)
            {
                printf("The word is %s",str);
                strncpy(new_node->text,str,LENGTH+1);
                //strcpy(new_node->text,str);

                new_node->next_word = NULL;
                index = hash( (unsigned char*)new_node->text);

                if(hashtable[index] == NULL)
                {
                    hashtable[index] = new_node;
                }
                else
                {
                    new_node->next_word =  hashtable[index];
                    hashtable[index] = new_node;
                }

                n_words++;

            }
            //free(new_node);



        }
        fclose(file);
        loaded = true;

        return true;    
    }

You need to be careful with `strncpy` as it might not terminate the string. And why do you allocate 1000 extra bytes for the node? — Some programmer dude, Feb 28 '13 at 17:58
Also, use a debugger to find out where the crash happen. It will also let you see the call-stack to see how you ended up where the crash is, and let you examine variables to help you figure out why it might have crashed. — Some programmer dude, Feb 28 '13 at 18:00
`node * new_node = malloc(sizeof(node)+1000);` What? Why the extra 1000 bytes? — Nik Bougalis, Feb 28 '13 at 18:23

score 5 · Accepted Answer · edited May 23 '17 at 11:58

Let's look at your code line by line, shall we?

    while(! feof(file))
    {

This is not the right way to use feof - check out the post Why is “while ( !feof (file) )” always wrong? right here on StackOverflow.

        node * new_node = malloc(sizeof(node)+1000);

Hmm, ok. We allocate space for one node and 1000 bytes. That's a bit weird, but hey... RAM is cheap.

        while( fscanf(file,"%s",str) > 0)
        {

Uhm... another loop? OK...

            printf("The word is %s",str);
            strncpy(new_node->text,str,LENGTH+1);
            //strcpy(new_node->text,str);

            new_node->next_word = NULL;
            index = hash( (unsigned char*)new_node->text);

Hey! Wait a second... in this second loop we keep overwriting new_node repeatedly...

            if(hashtable[index] == NULL)
            {
                hashtable[index] = new_node;
            }
            else
            {
                new_node->next_word =  hashtable[index];
                hashtable[index] = new_node;
            }

Assume for a second that both words hash to the same bucket:

OK, so the first time through the loop, hashtable[index] will point to NULL and be set to point to new_node.

The second time through the loop, hashtable[index] isn't NULL so new_node will be made to point to whatever hashtable[index] points to (hint: new_node) and hashtable[index] will be made to point to new_node).

Do you know what an ouroboros is?

Now assume they don't hash to the same bucket:

One of the buckets now contains the wrong information. If you add "hello" in bucket 1 first and "goodbye" in bucket 2 first, when you try to traverse bucket 1 you may (only because the linking code is broken) find "goodbye" which doesn't belong in bucket 1 at all.

You should allocate a new node for every word you are adding. Don't reuse the same node.

Looping using `feof` like that is almost always wrong. This program is one of those cases. — Carl Norum, Feb 28 '13 at 18:42
You are, of course, correct. I totally missed that. Post edited. — Nik Bougalis, Feb 28 '13 at 18:45

SEGMENTATION FAULT in strncpy - load from dictionary

1 Answers1