CODEWITHSUNDEEP
cpointerslinked-list
Ayush
Ayush

Reputation: 42450

Am I overwriting my linked list?

Disclaimer: This is a homework problem. As should be evident, I am trying to solve it myself. I seem to have hit a problem I am unable to figure out, so some help would be appreciated.

I am required to hash a set of words, and insert it into an array of linked lists. So, if 3 different words have the hash 38, at array[38], I need to have a linked list with the 3 words.

I am using this struct

struct Word {
    char* word;
    struct Word* next;
};

After I hash, I insert it into the array like this:

struct Word* table[1000];   // array of linked-lists

char word[128];
while(fgets(word, sizeof(word), dict) != NULL) {
    hashValue = hashWord(word, strlen(word));        

    struct Word *newWord = malloc(sizeof(struct Word));
    newWord->word = word;           

    if (table[hashValue] == NULL) {
        table[hashValue] = newWord;             
    } else {   // at index hashValue, table already contains at least one element
        // insert new word as first element of linked-list
        newWord->next = table[hashValue];
        table[hashValue] = newWord;         
    }

}

I know there are about 5 words that have a hash of 38, but when I print them, I get the same word 5 times:

struct Word* foo = table[38];

while (foo->next != NULL) {
    printf("%s", foo->word); // prints the same word every time
    foo = foo->next;
}

It seems I am overwriting my linked list at some point, but I cannot figure out where.

Upvotes: 0

Views: 1594

Answers (2)

Talya
Talya

Reputation: 19367

while(fgets(word, sizeof(word), dict) != NULL) {
    ...
    newWord->word = word;

The problem is that you're replacing the contents of word, which is the same pointer stored in every newWord.

You're allocating a new Word structure on the heap each time with this line:

struct Word *newWord = malloc(sizeof(struct Word));

But this only allocates memory for the Word structure itself; the Word structure includes a char *word—i.e. a pointer to a character (or NUL-terminated string, in this case), but it doesn't actually include any space for the string itself.

You need to explicitly allocate memory for the string, now. Try this:

newWord->word = strdup(word);

This will put a copy of word into each Word structure. This copy won't be overwritten when fgets() next gets called in your while loop. Remember that the stack-allocated character array:

char word[128];

is only valid while this function is executing; you must allocate something on the heap (with malloc(), or something that uses it, like strdup()) if you want it to live beyond the function call.

When you're finished, don't forget to free() the char *words before freeing each Word*.

Upvotes: 5

Antimony
Antimony

Reputation: 39471

You're overwriting the word array. You are only storing pointers to the array, but the array gets overwritten with each new word. You need separate memory to hold each word.

Upvotes: 1

Related Questions