Trying to create an array of Struct and load data from file

Question

I have this struct i created called User which stores all kind of data, now i'm trying to create an array of it (User*) and get data from a file, this is what i do in the start of my main program:

int amount = 0;
User* users = NULL;

// read from file functions
loadUsers(&users,&amount);

And the function (the amount of users is the first line of my txt file):

void loadUsers(User** users,int* amount2){
    int amount, i = 0, j;
    char temp[STRING_SIZE], temp2[STRING_SIZE];
    FILE* f;

    f = fopen("input.txt", "r");

    if (!f){
        return;
    }

    if (fileEmpty(f)){
        return;
    }

    fscanf(f, "%d", &amount);
    *amount2 = amount;

    *users = malloc(sizeof(User)*amount);
    /**users = (User*)malloc(sizeof(User)*amount);*/

    while (!feof(f)){
        fscanf(f, "%s", temp);
        users[i]->ID = (char*)malloc(sizeof(char)*(strlen(temp) + 1));
        strcpy(users[i]->ID, temp);
        fscanf(f, "%s", temp);
        users[i]->f_name = (char*)malloc(sizeof(char)*(strlen(temp) + 1));
        strcpy(users[i]->f_name, temp);
        fscanf(f, "%s", temp);
        users[i]->l_name = (char*)malloc(sizeof(char)*(strlen(temp) + 1));
        strcpy(users[i]->l_name, temp);

        i++;
    }

For some reason i get an error and while debugging i see the allocation is wrong since i only have users[0] and not users[1], like an array of users should have, even when the amount is higher than 1.

My target is to have an array, which each cell of it is a User.

What could be the reason?

Edit: User struct:

struct User{
    char* ID;
    char* f_name;
    char* l_name;
    int age;
    char gender;
    char* username;
    char* password;
    char* description;
    char** hobbies;
}typedef User;

why don't you try this char id[100]; f_name[100]; l_name[100]; while ((i < amount) && (fscanf(f, "%99s%99s%99s", id, f_name, l_name) == 3)) { /* copy the strings to users[i] here */ };. — Iharob Al Asimi
– Iharob Al Asimi, Commented Jan 27, 2015 at 17:16
If you are only getting a single User record (which is what I think you mean by 'i only have users[0]', then check your fscanf of amount . Trying printing the value of amount after you read it to see what value you get. — user590028
– user590028, Commented Jan 27, 2015 at 17:30

Iharob Al Asimi · Accepted Answer · 2015-01-27 17:44:16Z

2

You might be invoking undefined behavior because you don't check for i < amount in the while loop, you also don't check for fscanf() to see if it did succesfully read the data, if it fails, tha contents of the temp array would be uninitialized and trying to copy them to the malloced poitners is Undefined Behavior too.

So your problem is basically that your program is blindly assuming that everything is working as expected and potentially invoking Undefined Behavior, that mistake is very commong among new programmers.

You are allocating space for amount structs of User and yet you try to initialize amount pointers of User in the for loop, you should dereference the double pointer for it to work properly.

This is your own code with some checks that will prevent undefined behavior

void loadUsers(User** userlist, int* amount2)
{
    int  amount, i = 0;
    char id[100];
    char fname[100];
    char lname[100];
    FILE *file;
    User *users;

    file = fopen("input.txt", "r");

    if (!file) {
        return;
    }

    if (fileEmpty(file)) {
        fclose(file);
        return;
    }
    if (fscanf(file, "%d", &amount) != 1)
    {
        fclose(file);
        return;
    }

    *amount2  = amount;
    *userlist = malloc(sizeof(User) * amount);
    if (*userlist == NULL)
        return; /* malloc can fail, check that */

    /* point to the destination pointer to prevent the need of (*users)[i] */
    users = *userlist;
    /* if fscanf() returns 3 it means that all the arguments where read */
    while ((i < amount) && (fscanf(file, "%99s%99s%99s", id, fname, lname) == 3)) {
        size_t length;

        length      = strlen(id);
        users[i].ID = malloc(length + 1); /* sizeof(char) == 1 */
        if (users[i].ID != NULL)
            strcpy(users[i].ID, id);

        length          = strlen(fname);
        users[i].f_name = malloc(length + 1); /* sizeof(char) == 1 */
        if (users[i].f_name != NULL)
            strcpy(users[i].f_name, fname);

        length          = strlen(lname);
        users[i].l_name = malloc(length + 1); /* sizeof(char) == 1 */
        if (users[i].l_name != NULL)
            strcpy(users[i].l_name, lname);

        i++;
    }
    fclose(file);
}

edited Jan 27, 2015 at 17:44

answered Jan 27, 2015 at 17:38

Iharob Al Asimi

53.1k6 gold badges66 silver badges100 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Paul Ogilvie Over a year ago

I see you also correct "*users = malloc(sizeof(User)*amount);" to be "users = malloc(sizeof(User)*amount);" (implicitly).

Weather Vane Over a year ago

@PaulOgilvie iharob correctly wrote *userlist = malloc(sizeof(User) * amount); unlike your answer. Take a closer look. He then went users = *userlist; so that the assignments are more straight forward.

Iharob Al Asimi Over a year ago

@PaulOgilvie what do you mean? I don't understand, Wheather Vane's answer is not wrong.

Paul Ogilvie Over a year ago

@iharob: Yes, Whether Vane's answer is correct, but in the posted code he also fixed another error, namely that you derferenced Users in your original code when assigning the malloc result.

Paul Ogilvie Over a year ago

@iharob: I didn't see Users was passed aas a double pointer. Sorry for the confusion.

Weather Vane · Accepted Answer · 2015-01-27 18:04:37Z

0

Syntax error over the double pointer. Instead of this:

users[i]->ID = (char*)malloc(sizeof(char)*(strlen(temp) + 1));
strcpy(users[i]->ID, temp);

Try this:

(*users)[i].ID = malloc(strlen(temp) + 1);
strcpy ((*users)[i].ID, temp);

And so on for the other fields.

edited Jan 27, 2015 at 18:04

answered Jan 27, 2015 at 17:38

Weather Vane

34.8k7 gold badges43 silver badges63 bronze badges

3 Comments

Paul Ogilvie Over a year ago

The expressions are equivalent. In "users[i]", the compiler knows that 'users' is a pointer. It takes the value stored in users and adds i * sizeof(struct User) to it. We now have a pointer to a struct User, which you can dereference with the -> operator. In "(*users)[i]" the compiler dereferences users, which yields a struct User, and add i * sizeof(struct User) to that. We now have an expression of type struct User and can use the dot-operator to access the members. Hence the two are equivalent.

Weather Vane Over a year ago

@PaulOgilvie you are wrong. Using a single-indirection pointer gives the wrong results as OP found. I compiled and ran a simple program testing both methods, and using users[i]->ID works only for the first element, as OP found. Did you down vote me? Have you tested you own answer?

Paul Ogilvie Over a year ago

You are righ; I didn't see Users was passed as a double pointer. So indeed there must be a double indirection.

Collectives™ on Stack Overflow

Trying to create an array of Struct and load data from file

2 Answers 2

5 Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

5 Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related