1

I am new in C. I play with a dynamic array of strings. I want to load all words from textfile into an array of strings. I dynamically add items into array. It almost works, but when I want to print them out I got words repeated. I think the problem is in dictionary = (char **)realloc(dictionary, sizeof(char *) * (idx)); The array should be increased by one with original data copied.

int main() {

    FILE *fr;
    int i = 0;
    int c;
    const char *path = "..../sample.txt";
    char *word;
    char **dictionary;
    word = (char *)malloc(sizeof(char));
    dictionary = (char **)malloc(sizeof(char *));

    int idx = 0;    // index of word
    int widx = 0;   // index of char in word

    fr = fopen(path, "r");
    while((c = getc(fr)) != EOF){
        if( c == ' '){
            widx++;
            word = (char *)realloc(word, sizeof(char) * (widx));
            *(word + widx-1) = '\0';
            idx++;
            dictionary = (char **)realloc(dictionary, sizeof(char *) * (idx));
            *(dictionary+idx-1) = word;
            widx = 0;
            word = (char *)realloc(word, 0);
        }
        else if( c == '.' || c == ',' || c == '?' || c == '!' ){
            // skip
        }
        else{
            widx++;
            word = (char *)realloc(word, sizeof(char) * (widx));
            *(word + widx-1) = (char)c;
        }
    }
    fclose(fr);

    // print words
    int n = idx;
    for(i = 0; i < n; i++){
        printf("%d - %s \n", i, *(dictionary+i));
    }
    return 0;
}

Output:

0 - shellbly 
1 -  
2 - shellbly 
3 - Bourne-derived 
4 - shellbly 
5 - Bourne-derived 
6 - shellbly 
7 - Bourne-derived 

Expected:

1 - The 
2 - original 
3 - Bourne 
4 - shell 
5 - distributed 
6 - with 
7 - V7 
8 - Unix 

I must do something wrong. Thanks for any feedback.

aschepler
  • 70,891
  • 9
  • 107
  • 161
Joe Bobson
  • 1,216
  • 15
  • 19
  • 1
    If your target language is C, then don't add the C++ tag. C and C++ are two *very* different languages. – Some programmer dude Nov 05 '17 at 23:46
  • `const char *path = "..../sample.txt";` - what is that supposed to do? –  Nov 05 '17 at 23:46
  • As for *one* very obvious problem: `int widx = 0` followed by `widx++` followed by `realloc(word, sizeof(char) * (widx))` followed by `*(word + widx-1) = '\0'`. When you do that last operation, what is the value of `widx - 1`? – Some programmer dude Nov 05 '17 at 23:48
  • 1
    Use a debugger to step through your program, one line at a time, and examine which pointers get placed in the dictionary array, and keep track of those pointers until you understand why your code ends up corrupting memory. Your code saves each word in the dicitonary, then immediately proceeds to reuse the same buffer for the next word. Hillarity ensues. With sufficiently large input, you will probably get complete garbage in your output, due to attempts to dereference dangling pointers. – Sam Varshavchik Nov 05 '17 at 23:49
  • You might also be interested in [this discussion about casting the result of `malloc` (and friends)](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). Also, don't assign back to the pointer you pass to `realloc`, if `realloc` fails you lose the original pointer. – Some programmer dude Nov 05 '17 at 23:49
  • @SamVarshavchik Thanks. Now I see it. – Joe Bobson Nov 06 '17 at 08:16

1 Answers1

1

realloc(word, 0) is equivalent to free(word). But that pointer word is the same pointer you just stored as an element of dictionary, which means it's no longer valid to access that pointer element of dictionary.

Instead of word = realloc(word, 0);, you could do just word = NULL;. This will cause the next realloc to allocate fresh storage, and leave the pointer in dictionary alone.

If you're concerned about properly cleaning up the memory you allocated, you would then later free all the valid pointers in dictionary.

aschepler
  • 70,891
  • 9
  • 107
  • 161
  • Thanks. Now I see it, I was reusing the same buffer for the next word. One more question: When I want to clean up the dictionary, I need to call ```free(word)``` for every word and only after that I can call ```free(dictionary)```. Right? – Joe Bobson Nov 06 '17 at 08:29