Trouble populating an array of structs

Question

Relevent code block:

typedef struct
{
  char* phrase;
  char* hint;
} phrasenhint;

void populate_list(phrasenhint* parr[])
{
  FILE* fin = fopen("phrases.txt", "r");
  int MSIZE = 1024;
  int i = 0;
  char str[MSIZE];

  fgets( str, MSIZE, fin );
  while ( !feof(fin) )
  {
    phrasenhint* tmp = (phrasenhint*)malloc(sizeof(phrasenhint));
    char* ph = strtok(str, "~");
    char* hi = strtok(NULL, "\n\r");
    tmp->phrase = strdup(ph);
    tmp->hint = strdup(hi);
    printf("%s  |  %s\n", tmp->phrase, tmp->hint);
    parr[i] = tmp;
    free(tmp);
    i++;
    fgets( str, MSIZE, fin );
  }
  fclose(fin);
}
...
//IN MAIN
phrasenhint* parr[12];
for(int i = 0; i < 12; i++) parr[i] = NULL;
populate_list(parr);
...

I am having trouble populating an array of structs. I am not having trouble tokenizing the phrase and hint strings. When I loop through the array in main after calling populate_list() it is filled with just the last stored struct in all indexes of the array.

Contents of phrases.txt

Mother~A birth giver.
Cat~Our feline friends.
C++~Object oriented version of C.
Coffee~Made from a bean. Drunk in the morning.
DOOM~Popular video game where the protagonist slays demons on mars.
Link~People often confuse me for Zelda.
Nintendo~Every single gaming console (X-Box, Playstation, Genesis, etc.) is this to a mother.
Minecraft~Popular video game made by Markus Persson.
MacBook~Every developer has or will have one of these.
Pointers~Enough to make a grown man cry.
Python~People who program in this language claim it to be the best.
PHP~Not a programming language.

Output

Mother  |  A birth giver.
Cat  |  Our feline friends.
C++  |  Object oriented version of C.
Coffee  |  Made from a bean. Drunk in the morning.
DOOM  |  Popular video game where the protagonist slays demons on mars.
Link  |  People often confuse me for Zelda.
Nintendo  |  Every single gaming console (X-Box, Playstation, Genesis, etc.) is this to a mother.
Minecraft  |  Popular video game made by Markus Persson.
MacBook  |  Every developer has or will have one of these.
Pointers  |  Enough to make a grown man cry.
Python  |  People who program in this language claim it to be the best.
PHP  |  Not a programming language.
PHP, Not a programming language., 0
PHP, Not a programming language., 1
PHP, Not a programming language., 2
PHP, Not a programming language., 3
PHP, Not a programming language., 4
PHP, Not a programming language., 5
PHP, Not a programming language., 6
PHP, Not a programming language., 7
PHP, Not a programming language., 8
PHP, Not a programming language., 9
PHP, Not a programming language., 10
PHP, Not a programming language., 11

Expected output:

Mother  |  A birth giver.
Cat  |  Our feline friends.
C++  |  Object oriented version of C.
Coffee  |  Made from a bean. Drunk in the morning.
DOOM  |  Popular video game where the protagonist slays demons on mars.
Link  |  People often confuse me for Zelda.
Nintendo  |  Every single gaming console (X-Box, Playstation, Genesis, etc.) is this to a mother.
Minecraft  |  Popular video game made by Markus Persson.
MacBook  |  Every developer has or will have one of these.
Pointers  |  Enough to make a grown man cry.
Python  |  People who program in this language claim it to be the best.
PHP  |  Not a programming language.
Mother, A birth giver, 0
Cat, Out feline friends., 1
...//didnt feel like typing indexes 2-9 but follows the same output scheme
Python, People who program in this language claim it to be the best., 10
PHP, Not a programming language., 11

I have filled arrays of structs like this before and am doing it exactly as I have in the past but for some reason the array is not filling properly.

`parr[i] = tmp; free(tmp);` - why are you freeing the thing you just carefully constructed? — Mat, Dec 14 '20 at 17:16
What Mat wants to say: You are not allowed to access the memory at that address after you free it. Copying that address in another variable does not affect this. You mustn't access the memory via `parr[i]` afterwards. If you still do it you invoke undefined behaviour. — Gerhardh, Dec 14 '20 at 17:23
You might also find these interesting: https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong and https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858#605858 — Gerhardh, Dec 14 '20 at 17:24

score 0 · Answer 1 · answered Dec 14 '20 at 20:25

The problem is that you call free(tmp);, as already mentioned in the comments of Mat and Gerhardh.

The reason why you should not do this is the following: In parr[i] = tmp;, tmp is only a reference (a pointer) to a memory location. This reference gets copied to parr[i]. However, the memory itself is not copied. After the assignment parr[i] = tmp;, parr[i] and tmp both point to the same memory location.

After you called free(tmp);, you are not allowed to access this memory any more, no matter if you try to access it via tmp or via parr[i].

The solution to your problem is relatively simple: Just remove parr[i] = tmp;.

score 0 · Answer 2 · answered Dec 21 '20 at 20:46

phrasenhint* parr[12]; is not an array of structs, it is an array of pointer to struct.

phrasenhint parr[12]; is an array of structs and you can zero them out using phrasenhint parr[12] = {};, no for loop is needed.

In case of the pointers phrasenhint* parr[12] = {}; will fill the array with NULL pointers.

And if you use an array of structs you can assign directly to parr[i].phrase or parr[i].hint. So you do not need to malloc them.

Trouble populating an array of structs

2 Answers2