2

I have the following code to populate an array of strings, but every time i change the value, the entire array changes (instead of a single string the array)

Bonk
  • 1,859
  • 9
  • 28
  • 46

3 Answers3

4

That's because you are the same pointer to char for all positions of the array.

When you do:

words[i] = txt;

You are assigning a pointer. So every single word[i] is the same string (txt). If you really want to read the word into a buffer (like txt) and then put it into the array of strings, you need to copy the contents of the buffer string to the string in the array, like so:

strncpy(words[i], txt, MAX_WORD_LENGTH);

There's also another problem with your code, which is the allocation of the string array. It should be:

words = (char**)malloc(wordcount * sizeof(char*));

That is because a string array is a pointer to a char pointer (char**), and each element of the array is a string (char*). Now you have allocated a array of char pointers, but you have not allocated the memory for each string, which is what we do next:

for (i = 0; i < wordcount; i++) {
    words[i] = (char*)malloc(MAX_WORD_LENGTH * sizeof(char));
}

If you want to not use a buffer and read directly into the string array, your code would be something like this:

words = (char**)malloc(wordcount * sizeof(char*));
input = fopen(filename, "r");
while(!feof(input)) {
    words[i] = (char*)malloc(MAX_WORD_LENGTH * sizeof(char));
    fscanf(input, "%s", words[i]);
}
Mig
  • 768
  • 5
  • 11
  • thanks for point out char** issue, and I tried with strncopy, I get the following error: `0xC0000005: Access violation writing location 0xcdcdcdcd` Looks like the pointer is NULL still – Bonk Mar 25 '12 at 03:04
  • @YonkShi That's probably because you haven't allocated memory from each string in the array yet. Use my last piece of code, make the fscanf read to txt and in the next line do the strncpy. That should work. – Mig Mar 25 '12 at 03:14
1

You need to allocate space for each word and copy the string to this allocated space:

input = fopen(filename, "r");
while(!feof(input)){
    if(fscanf(input,"%s", txt)){
        /* malloc for word here */
        words[i] = malloc(strlen(txt)+1);
        strcpy(words[i], txt); //<---Problem line right here
        ++i;
    }
}
perreal
  • 94,503
  • 21
  • 155
  • 181
  • is that malloc for one word or for all words? the malloc is my code is to allocate space for all the words to be stored in this array – Bonk Mar 25 '12 at 02:57
  • yes, but it stores pointers. you also need to allocate memory for each word (which is the one I added) and assign those to the pointers you allocated – perreal Mar 25 '12 at 02:58
  • fantastic! it worked! Do you have to malloc for every string in the future? what about `char * a = "a string";` why would that work without malloc? – Bonk Mar 25 '12 at 03:09
  • in that case you are pointing to a constant string so there is no need for malloc. but you cannot modify a constant string, it is static. – perreal Mar 25 '12 at 03:38
0

The problem with this piece of code is that you have allocated a single buffer to receive string data from fscanf. Each time a string is read from the file, it is placed in the txt buffer. Each pointer in the words array points to txt. So at the end of the operation, you have many pointers which point to the same buffer, which contains one string.

ShiggityShiggityShwa
  • 194
  • 9