CS50 Pset5 Speller:Unload函数Valgrind内存检测失败求助
Let's break down the issues causing your Valgrind errors and fix them step by step.
1. Root Cause 1: Hash Function Returning Out-of-Bounds Index
Valgrind's hint about "using a variable that might not have a value" points directly to your while(root != NULL) line in unload. This usually happens if your hash function is returning an index outside the valid range of your buckets (0 to 5489, since you defined 5490 buckets).
When you access table[index] with an invalid index, you're writing to memory outside the hash table array. This corrupts nearby memory, turning some table entries into wild pointers—hence Valgrind's warning when you try to check root != NULL later.
Fix:
Ensure your hash function returns a value between 0 and buckets - 1. The simplest way to enforce this is to take your hash calculation result and modulo it by buckets:
unsigned int hash(const char *word) { // Your existing hash logic here unsigned int hash_value = ...; return hash_value % buckets; // Guarantees index stays within bounds }
2. Root Cause 2: Unreleased Memory on Load Failure
In your load function, if malloc fails for a new node, you immediately return false—but you don't free any of the nodes you already allocated before the failure. This leaves orphaned nodes in memory, causing Valgrind to flag memory leaks.
Fix:
Add cleanup logic before returning false when malloc fails. Iterate through all buckets and free any existing nodes:
bool load(const char *dictionary) { FILE *file = fopen(dictionary, "r"); if (file == NULL) { fprintf(stderr, "Could not open %s.\n", dictionary); return false; } char dictionaryWord[LENGTH + 1]; while (fscanf(file, "%s", dictionaryWord) != EOF) { node *newNode = malloc(sizeof(node)); if (newNode == NULL) { fprintf(stderr, "Not enough memory available to load %s.\n", dictionary); // Clean up already allocated nodes for (int i = 0; i < buckets; i++) { node *root = table[i]; while (root != NULL) { node *tmp = root; root = root->next; free(tmp); } table[i] = NULL; // Reset bucket to avoid dangling pointers } wordCount = 0; // Reset word count fclose(file); // Don't forget to close the file! return false; } strcpy(newNode->word, dictionaryWord); int index = hash(dictionaryWord); newNode->next = table[index]; table[index] = newNode; wordCount++; } fclose(file); return true; }
Note: I simplified your linked list insertion logic—you don't need a separate if (table[index] == NULL) check. Pointing newNode->next to the current table head, then updating the head to newNode, works for both empty and non-empty buckets.
3. Polish Your Unload Function
Your current unload logic is correct, but adding a reset to NULL after freeing each bucket prevents accidental access to freed memory later:
bool unload(void) { for (int i = 0; i < buckets; i++) { node *root = table[i]; while(root != NULL) { node *tmp = root; root = root->next; free(tmp); } table[i] = NULL; // Reset bucket to NULL } wordCount = 0; // Reset word count return true; }
After making these changes, re-run Valgrind with valgrind --leak-check=full ./speller dictionary.txt—your memory errors should be resolved.
内容的提问来源于stack exchange,提问作者Qunoot K

