C - Output contains null values and segmentation fault

Question

This program is supposed to create a struct array containing the name and age values from the hard coded name and age arrays at the start of the code. I've been explicity asked to declare the array in the main function and then allocate memory to it within the insert function. The program compiles fine and the output i'm supposed to get is:

Name: Simon

Age: 22

Name: Suzie

Age: 24

Name: Alfred

Age: 106

Name: Chip

Age: 6

etc. etc.

However the output I get is something like this:

Name: Simon

Age: 22

Name: (null)

Age: 33

Name: Suzie

Age: 24

Name: (null)

Age: 33

Name: Suzie

Age: 24

..... segmentation fault.

Some of the names appear twice, some of the names are null, and there is a segmentation fault at the end of the output. Any help would be greatly appreciated. many thanks.

    #include <stdio.h>
    #include <stdlib.h>

    /* these arrays are just used to give the parameters to 'insert',
       to create the 'people' array 
    */

    #define HOW_MANY 7
    char *names[HOW_MANY]= {"Simon", "Suzie", "Alfred", "Chip", "John", "Tim",
                  "Harriet"};
    int ages[HOW_MANY]= {22, 24, 106, 6, 18, 32, 24};

    /* declare your struct for a person here */
    struct person {
      char *name;
      int age;
    };

    static void insert(struct person **arr, char *name, int age) 
    {
      //initialise nextfreeplace
      static int nextfreeplace = 0;
      //allocate memory
      arr[nextfreeplace] = malloc (sizeof(struct person));
      /* put name and age into the next free place in the array parameter here */
      arr[nextfreeplace]->name = name;
      arr[nextfreeplace]->age = age;
      /* modify nextfreeplace here */
      nextfreeplace++;
    }

    int main(int argc, char **argv) 
    {

      /* declare the people array here */
      struct person *people;

      for (int i = 0; i < HOW_MANY; i++) 
      {
        insert (&people, names[i], ages[i]);
      }

      /* print the people array here*/
      for (int i = 0; i < HOW_MANY; i++)
      {
        printf("Name: %s \t Age: %i \n", people[i].name, people[i].age);
      }

      return 0;
    }

Line 24 doesn't need: `sizeof(struct person)` it should be `sizeof(person)`. Other than that please post the code. — Rivasa, Jul 30 '17 at 14:08
Try `struct person *people;` -> `struct person *people[HOW_MANY];` — Marian, Jul 30 '17 at 14:09
@Marian: ... and change `insert(&people, ...` to be `insert(people, ...`. — alk, Jul 30 '17 at 14:10
@alk ...? https://stackoverflow.com/questions/143025/how-do-i-find-the-size-of-a-struct — Rivasa, Jul 30 '17 at 14:13
Sorry for the bad style of question writing. Just made an account and not quite sure how to fix the indentation style. I tried copy/pasting the code from sublime straight to the website but it came out weird :( — Spicy McCree, Jul 30 '17 at 14:14
The output is supposed to print the names and ages (initialised at the start) but from a struct array that hold those values. Thanks again for your help guys — Spicy McCree, Jul 30 '17 at 14:17
@Annabelle: The question you link simply carries the wrong tag, the C tag, which also only had been added later, obviously without closely reading the question. In C a `struct` type ***always*** needs to carry `struct`. This is different in C++. — alk, Jul 30 '17 at 14:18
@alk So basically when calculating sizeof() person we need to include struct in it as well? Huh okay. — Rivasa, Jul 30 '17 at 14:21

score 2 · Answer 1 · answered Jul 30 '17 at 14:31

In your code, you have

#define HOW_MANY 7
/* ... */
struct person {
    /* ... */
};

static void insert(struct person **arr/* ... */) {
    static int nextfreeplace = 0;
    /* ... */
    arr[nextfreeplace] = malloc(sizeof(struct person));
    /* ... */
    nextfreeplace++;
}

int main(int argc, char **argv) {
    /* ... */
    struct person *people;

    for (int i = 0; i < HOW_MANY; i++) {
        insert(&people/* ... */);
    }
    /* ... */
    return 0;
}

The problem is that you define a variable named people as a pointer. Then you pass the address of that pointer to insert. Since local variables are (usually) allocated on the stack, your allocations made in insert overwrite parts of it.

Assume you have

struct person *people;
struct person *otherpeople;

Then, when you have nextfreeplace == 0, you assign to arr[0] == *arr, which is fine. But for nextfreeplace == 1, you assign to arr[1] == *(arr+1) == otherpeople.

This type of bug is called a buffer overflow.

To fix this bug, you would need to use struct person *people[HOW_MANY]; and insert(people/* ... */);.

As a side note: You should also free the memory you allocated and do not need anymore using free.

Hi there. Thanks for taking time out of your day to help. The code is part of my uni coursework and they've said to " Modify insert to call malloc to create a new struct and set the correct array element pointing to it." I think that means that i'm not allowed to just use people[HOW_MANY] in the main function to allocate memory :( — Spicy McCree, Jul 30 '17 at 14:56
From "[...] and set the correct *array element pointing to it*", I would assume that you are actually supposed to use an array of pointers and then allocate memory for each pointer in it. Otherwise, you would be required to allocate memory for `HOW_MANY` structures of type `person` (ie. `struct person *people = malloc(HOW_MANY*sizeof(*people));`) and then fill each of them. But then, you would not set pointers and the allocation would be outside `insert`. — Abrixas2, Jul 30 '17 at 15:07

C - Output contains null values and segmentation fault

1 Answers1