Array of pointers to struct

Question

I have a struct like so

typedef struct person {
 int id;
 char name[20];
} Person;

Then, outside of the function, I have a pointer array of pointers to these structs, like so

Person **people;

Then in the function I am adding people to the array like so (in a loop)

Person person;

for (i = 0; i < 50; i++)
{
  person.id = i;
  person.name = nameArray[i];
  people[i] = &person;
}

person is being added to the people array but when (in VS2010) I go to the Watch screen and type people, 50 I just see the same person in every slot as if when adding the next person, it changes all previous as well. What am I doing wrong here?

Also, to retrieve a certain person's name, is this the right syntax?

people[0] -> name; Or is it people[0][0].name?

Thanks!

Sidenote about a deleted question: I was just writing an answer to your question stackoverflow.com/questions/9485491/… when you deleted it. — user811773
– user811773, Commented Feb 28, 2012 at 18:49
Sorry about that. Felt it may not have been a very productive questions for the site. — Jake Sankey
– Jake Sankey, Commented Feb 29, 2012 at 4:55
Nevermind. The idea was: Set the whole char[20] to all '\0' before calling fscanf. After fscanf returns, do for(int i=19;i>=0; i--) {if(s[i]=='\0') s[i]=' '; else break;}. — user811773
– user811773, Commented Feb 29, 2012 at 7:32

Seth Carnegie · Accepted Answer · 2012-02-27 19:11:31Z

What do you expect? You are making all the pointers point to the same Person. And when person goes out of scope, all the pointers in your array (which are all the same) will be invalid and point to a deallocated block of memory. You have to use malloc in each iteration of the loop to allocate dynamic storage and create a Person that won't go away till you free it:

for (i = 0; i < 50; i++)
{
  Person *person = malloc(sizeof(Person));
  person->id = i;
  person->name = nameArray[i];
  people[i] = person;

  /* or:
  people[i] = malloc(sizeof(Person));
  people[i]->id = i;
  people[i]->name = nameArray[i];

  it does the same thing without the extra temporary variable
  */
}

// then when you are done using all the Person's you created...
for (i = 0; i < 50; ++i)
    free(people[i]);

Alternatively, you could have an array of Persons instead of Person*s and what you are doing would work:

Person people[50];

Person person;

for (i = 0; i < 50; i++)
{
  person.id = i;
  person.name = nameArray[i];
  people[i] = person; // make a copy
}

And with that way you don't have to free anything.

Ed Swangren · Accepted Answer · 2012-02-27 19:06:23Z

2

I just see the same 'person' in every slot...

Right, because you use the same Person each time. You are reassigning its members, so the last assignment sticks and your pointers all point to the same chunk of memory.

Also realize that you're storing the address of a variable with automatic storage duration (i.e., stack allocated). That memory will (probably) be cleaned up when the function exits and dereferencing any of those pointers at a later time will result in undefined behavior.

You should be using dynamic allocation if you need to initialize the array in the function and keep it valid when the function exits.

answered Feb 27, 2012 at 19:06

Ed Swangren

125k24 gold badges189 silver badges271 bronze badges

Comments

Zeta · Accepted Answer · 2012-02-27 19:06:31Z

1

You have to allocate memory for each person:

for (i = 0; i < 50; i++)
{
  people[i] = (person*)malloc(sizeof(person)); // dynamic memory allocation
  people[i]->id = i;
  people[i]->name = nameArray[i];
}

The address &person of person does not change in your original version, so you have a bunch of pointers pointing on the same data, which will change on every iteration.

answered Feb 27, 2012 at 19:06

Zeta

106k13 gold badges205 silver badges249 bronze badges

Comments

citronaut · Accepted Answer · 2012-02-27 19:12:59Z

1

Because of the way you have written this, you have initialized each pointer to the same instance of Person, examine:

Person person; // this allocates a single person

for (i = 0; i < 50; i++)
{
  person.id = i;
  person.name = nameArray[i];
  people[i] = &person;
}

What you need to do is allocate dynamically each person, as so:

for (i = 0; i < 50; i++)
{
  Person* person = (Person*)malloc(sizeof(Person));
  person->id = i;
  person->name = nameArray[i];
  people[i] = person;
}

Don't forget to free all the memory after you're done, so at the end of your code:

int i;
for (i=0; i<50; i++)
  free(people[i]);
free(people);

answered Feb 27, 2012 at 19:12

citronaut

111 bronze badge

Comments

trutheality · Accepted Answer · 2012-02-27 19:07:18Z

0

You need to make sure to allocate space for each person. Your code as it is right now only has one person struct that you are reusing, and all of those pointers are pointing to the same address.

answered Feb 27, 2012 at 19:07

trutheality

23.5k6 gold badges58 silver badges68 bronze badges

Comments

sparc_spread · Accepted Answer · 2012-02-28 02:24:58Z

0

The other answers are all correct. The only thing I can add is, besides not creating a new Person, you are also allocating it on the stack when you should be using the heap. Make sure you understand why the following "quick change" to your existing code is bad:

for (i = 0; i < 50; i++)
{
  Person person;   /* <--- BAD. Make sure you understand why. */

  person.id = i;
  person.name = nameArray[i];
  people[i] = &person;
}

This makes a new Person each time, using your old way of creating person. Again, please be sure you understand why this is wrong and the other answers (using malloc) are right.

edited Feb 28, 2012 at 2:24

answered Feb 27, 2012 at 19:15

sparc_spread

10.9k12 gold badges50 silver badges65 bronze badges

Collectives™ on Stack Overflow

Array of pointers to struct

6 Answers 6

Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related