Trying to input a strcture, first time works great, second time crashes

Question

Hi I'm inputting a structure of a family with children with those two structures:

typedef struct person {
    int id;
    char* firstName;
    int age;
}person;

typedef struct family {
    char* lastName;
    person father, mother;
    person* children;
    int numChildren;
}family;

EDIT: this is the edited function and it still crashes :

int initializeHouse(family **pdata)
{
    char temp[SIZE];
    int size, i, j;
    printf("enter the number of families\n");
    scanf("%d", &size);
    *pdata = (family*)malloc(sizeof( family)*size);
    for (i = 0; i<size; i++)
    {
        printf("Please enter the last name\n");
        scanf("%s", temp);
        (*pdata)[i].lastName = (char*)malloc(sizeof(char)* (strlen(temp) + 1));
        strcpy(pdata[i]->lastName, temp);
        printf("Enter the fathers first name\n");
        scanf("%s", temp);
        initPerson(temp, &pdata[i]->father.firstName);
        printf("enter the fathers ID\n");
        scanf("%d", &pdata[i]->father.id);
        printf("Enter the fathers age\n");
        scanf("%d", &pdata[i]->father.age);
        printf("Enter the mothers first name\n");
        scanf("%s", temp);
        initPerson(temp, &pdata[i]->mother.firstName);
        printf("enter the mothers ID\n");
        scanf("%d", &pdata[i]->mother.id);
        printf("Enter the mothers age\n");
        scanf("%d", &pdata[i]->mother.age);
        printf("enter the number of children");
        scanf("%d", &pdata[i]->numChildren);
        (*pdata)[i].children= (person*)malloc(sizeof(person)*(pdata[i]->numChildren));
        for (j = 0; j<pdata[i]->numChildren; j++)
        {
            printf("enter the kids name\n");
            scanf("%s", temp);
            initPerson(temp, &pdata[i]->children[j].firstName);
            printf("enter the kids ID\n");
            scanf("%d", &pdata[i]->children[j].id);
            printf("Enter the kids age\n");
            scanf("%d", &pdata[i]->children[j].age);

        }
    }
    return size;
}

void initPerson(char* str, char** fam)
{
    *fam = (char*)malloc(sizeof(char)*(strlen(str) + 1));
    strcpy(*fam, str);
}

EDIT: I changed the code and it still doesn't work, and it requires me to write some description so here it is ..

An aside: [Do not cast the return of `malloc`](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). — lurker, Jan 25 '18 at 19:18
@Lurker I did that *pdata=(family*)malloc(sizeof(family)*size; but why do I need to make each member as an allocated memory? that doesn't make sense. its a one dimensional array of structs — , Jan 25 '18 at 19:24
You need to allocate for an array of structure pointers, not an array of structures. So, `pdata = malloc(sizeof(family *)*size);`. And you still need to initialize each structure pointer as I indicated. `pdata[1]` and above all reference an uninitialized pointer. — lurker, Jan 25 '18 at 19:26
when I allocate a memory of pdata as a one dimensional array I create 0,1,2 many structures as I multiplied by the size, when I allocate again on pdata[1] what do I get from that? memory for what exactly? — , Jan 25 '18 at 19:27
@user 3121023 I edited the main function inside my post, and if im not mistaken those are the same things pdata[i]-> and (*pdata)[i] — , Jan 25 '18 at 19:31
`pdata` is of type `family **`. I believe `pdata[x]` is treating it as an array of `family *` types rather than a pointer to an array of `family` types. — Christian Gibbons, Jan 25 '18 at 19:35
@Christian Gibbons , but isnt family* and family[] is the same thing?.. — , Jan 25 '18 at 19:38
`pdata[i]->children = (person*)malloc(sizeof(person)*(pdata[i]->numChildren));` You can use `calloc` here. — balki, Jan 25 '18 at 19:40
@balki I prefer malloc as it works faster and I kinda have a problem with my fam[i]->lastName .. right now — , Jan 25 '18 at 19:42
Also, always check return value of `scanf`, so you can do something (such as just exit with error message) on parse error. Otherwise, funny things happen on invalid input, and trying to debug a program with funny things will often become an exercise in futility. — hyde, Jan 25 '18 at 19:53
@AlexAlex: And don't edit the question after answers, it can make them meaningless to future visitors — Mark Benningfield, Jan 25 '18 at 20:04
You didn't change all the instances of `pdata[i]->` to `(*pdata)[i].` — yano, Jan 25 '18 at 20:05
@yano thank you, all of you guys thank you, you were right. Taught me a valuable lesson in structs, thank you! — , Jan 25 '18 at 20:08
@AlexAlex They are not the same. An array of pointers will use an index offset to find a pointer and then dereference it to find the object. a pointer to an array will need to be dereferenced to get the array, and then use an index offset to find the object in the array. — Christian Gibbons, Jan 25 '18 at 20:10
@Christian Gibbons, yes I understand it now, once again thank you! — , Jan 25 '18 at 20:12

score 1 · Answer 1 · answered Jan 25 '18 at 19:53

This little example using (*pdata)[i]. doesn't crash.

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

#define SIZE 40

typedef struct person {
    int id;
    char* firstName;
    int age;
}person;

typedef struct family {
    char* lastName;
    person father, mother;
    person* children;
    int numChildren;
}family;

int initializeHouse(family **pdata)
{
    char temp[SIZE];
    int size, i, j;
    printf("enter the number of families\n");
    scanf("%d", &size);
    *pdata = malloc(sizeof(family)*size);
    for (i = 0; i<size; i++)
    {
        printf("Please enter the last name\n");
        scanf("%39s", temp);
        (*pdata)[i].lastName = malloc(sizeof(char)* (strlen(temp) + 1));
        strcpy ( (*pdata)[i].lastName, temp);
    }
    return size;
}

int main ( void) {
    int size;
    family *a;
    size=initializeHouse(&a);
}

Mark Benningfield · Accepted Answer · 2018-01-25T20:14:38.300

1

  int main() {
    int size;
    family *a = NULL;
    size=initializeHouse(&a);
  }

declares a pointer to a family structure. When you pass it's address

size = initializeHouse(&a);

the function gets it as a family**

Okay, we're all on the same page to this point. When you allocate the target of that pointer

*pdata = malloc(sizeof(family) * size);

then *pdata points to an allocated array of structs, not pointers to those structs. Each struct is accessed by (*pdata)[i], which means -> dereference the double pointer pdata to get the address of the first element in the array, then access the array element with a subscript.

So your assignment should be

(*pdata)[i].lastName = malloc(sizeof(char)* (strlen(temp) + 1));

You use the dot . operator to access the members, because the result of the subscript access is a struct, not a pointer to a struct.

edited Jan 25 '18 at 20:14

answered Jan 25 '18 at 19:54

Mark Benningfield

2,800
9
31
31

I edited it just like you said , still crashes... il post the code in the EDIT. – Jan 25 '18 at 20:01
@AlexAlex: Don't change your comments -> it makes it very hard to follow – Mark Benningfield Jan 25 '18 at 20:02
Sorry, I added the edited code to my post, still crashes tho, is there anything wrong there aswell? – Jan 25 '18 at 20:04
@AlexAlex: You have to change all occurrences from `&pdata[i]` to `(*pdata)[i]`. – Mark Benningfield Jan 25 '18 at 20:07

Pablo · Answer 3 · 2018-01-25T20:14:57.070

It's important that you understand the memory layout.

family *fam;
family **pdata = &fam;

*pdata = (family*)malloc(sizeof(family)*size);

You have essentialy this: fam is a uninitialized pointer of type family. pdata is a double pointer initialized with the address of fam. The malloc call allocates space for size family-objects. By doing *pdata = malloc(...) you are initializing fam.

This is the basic memory layout you have. base is the address returned by malloc. slf is the size of struct family object, fpl is the size of a pointer to a struct family object.

base = address returned by malloc
sfl = sizeof(struct family)
fpl = sizeof(struct family*)

base + 0          base + slf         base + 2 * slf
+-----------------+------------------+------------------+
|struct family    | struct family    | struct family    |
+-----------------+------------------+------------------+

base + 0    base + fpl    base + 2*fpl base + 3*fpl base + 4*fpl
+------------+------------+------------+------------+-----------+
| pdata[0]   | pdata[1]   | pdata[2]   | pdata[3]   | pdata[4]  |
+------------+------------+------------+------------+-----------+

The first row shows the memory in terms on struct family objects, the second row shows you the same memory in terms on pointers (to struct family object). This is very important distinction, because pdata[i] returns you a pointer, not the object.

Remember pdata is a double pointer, pdata[i] is the equivalent to pdata + i, that is the ith pointer begining at base.

Because the size of an struct family is defintitely different that the size of a pointer, you see that the block don't align, that means base + slf != base + fpl.

In the first iteration you are lucky, because pdata[0] and (*pdata)[0] are the same. But pdata[1] and *(pdata)[1] are not the same. So doing pdata[1]->lastname (instead of (*pdata)[1].lastname) you are accessing at a wrong location in you allocated memory.

The easiest way to fix you code would be to change the pdata[i] in (*pdata)[i] as shown in user3121023's answer.

edit

I see that user3121023 has retracted his answer. Basically it did:

printf("enter the fathers ID\n");
scanf("%d", &((*pdata)[i].father.id));

in the loop.

balki · Answer 4 · 2018-01-25T21:25:03.937

Fixing the pointers may have solved your problem. But the program has a lot of duplicate code and rewriting as follows would help you test a small portion and debug the program easily.

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

#define SIZE 100

typedef struct person {
    int id;
    char *firstName;
    int age;
} person;

typedef struct family {
    char *lastName;
    person father, mother;
    person *children;
    int numChildren;
} family;

void input_string(const char *prompt, char **where) {
    char temp[SIZE];
    printf("%s\n", prompt);
    scanf("%s", temp);
    *where = malloc(sizeof(char) * (strlen(temp) + 1));
    strcpy(*where, temp);
}

void input_int(const char *prompt, int *where) {
    printf("%s\n", prompt);
    scanf("%d", where);
}

void input_person(const char *name, person *person) {
    char prompt[SIZE];
    sprintf(prompt, "Enter the %s's first name", name);
    input_string(prompt, &person->firstName);
    sprintf(prompt, "Enter the %s's ID", name);
    input_int(prompt, &person->id);
    sprintf(prompt, "Enter the %s's age", name);
    input_int(prompt, &person->age);
}

void input_family(family *fam) {
    input_string("Please enter the last name", &fam->lastName);
    input_person("father", &fam->father);
    input_person("mother", &fam->mother);
    input_int("Please enter the number of children", &fam->numChildren);
    fam->children = malloc(sizeof(person) * (fam->numChildren));
    for (int i = 0; i < fam->numChildren; i++) {
        input_person("kid", &(fam->children)[i]);
    }
}

int initializeHouse(family **families) {
    int size;
    input_int("Please enter the number of families", &size);
    *families = malloc(sizeof(family) * size);
    for (int i = 0; i < size; i++) {
        input_family(&(*families)[i]);
    }
    return size;
}

int main() {
    int size = 0;
    family *a;
    size = initializeHouse(&a);
    printf("Successfully inputted %d families", size);
    return 0;
}

Trying to input a strcture, first time works great, second time crashes

4 Answers4