I am having trouble using fscanf to load data from a file into structs

Question

I am having trouble using fscanf and would appreciate any help. I am trying to read from a file that I am in control of. Since I knew the format this is what I tried to do reading from that file.

typedef struct {
    char *website;
    char *user;
    char *password;
    char *description;
} Account;

typedef struct Database Database;

struct Database { 
    int number_of_acc;
    Account **accounts;
};



int 
load_database(Database *db, FILE *fp) {
    char website[MAX_BUFFER_SIZE], username[MAX_BUFFER_SIZE], password[MAX_BUFFER_SIZE];
    Account acc;

    if (db->number_of_acc == 0)
        db->accounts = malloc(5*sizeof(acc));

    while(fscanf(fp, "%s: %s, %s", website, username, password ) == 3) {

        if(db->number_of_acc > 5) 
            db->accounts = malloc(2*db->number_of_acc*sizeof(acc));

        acc.website = website;
        acc.user = username;
        acc.password = password;
        //acc.description = description;
        db->accounts = malloc(sizeof(acc));
        db->number_of_acc += 1;
    }

    return Success;
}

However, when going through gdb fscanf is not returning 3 so the while loop is ignored. Here is the input file.

Reddit: Username, Password

Any advice and help in terms of my code are greatly appreciated. Thank you for your time.

Answer 1

^{Transferring a comment into an answer.}

Problem with fscanf()

The second %s reads up to a white space character; the next character is, by definition, not a comma. You presumably need to use a (negative) scan set: %[^,] to read up to but not including a comma. You should avoid buffer overflows by specifying sizes for both %s and %[…] — see How to prevent scanf() causing a buffer overflow in C?

Problem with memory management

Note that as ryyker pointed out, you have memory management problems too. You need to check that your memory allocations succeeded, and the second one should be using realloc(), not malloc() (to avoid memory leaks and the loss of already entered data). Error checking still omitted — it is best to assign the result of realloc() to a new variable so that you don't lose the pointer to the previous allocation if the reallocation fails.

if (db->number_of_acc == 0)
{
    db->accounts = malloc(5*sizeof(acc));
    db->number_of_acc = 5;
}

and

if (db->number_of_acc >= db->number_of_acc)
{
    db->accounts = realloc(db->accounts, 2*db->number_of_acc*sizeof(acc));
    db->number_of_acc *= 2;
}

You need to consider whether the relational operator should be >= or >.

Also, the paragraph copying the data that was read is bogus:

acc.website = website;
acc.user = username;
acc.password = password;
//acc.description = description;
db->accounts = malloc(sizeof(acc));
db->number_of_acc += 1;

You don't show the structure definition, but if it contains arrays, you need to use strcpy() and if it contains pointers, you need to use strdup() or an equivalent function. You also need to assign to the right element of the array, and copy the data into the allocated space.

I'm not about to fix this — the question is incomplete on this topic and hence can't be answered definitively. On the face of it, you have an array of structures; you should copy the newly entered data into the next available structure. You need separate counters for the number of allocated structures in the array and the number that are in use (or, equivalently, the index of the next entry to use).

If you still have problems, ask a new question, but read about how to create an MCVE (Minimal, Complete, Verifiable Example — or MRE or whatever name SO now uses) or an SSCCE (Short, Self-Contained, Correct Example) — the same idea by a different name.