CODEWITHSUNDEEP
cpointers
pogo
pogo

Reputation: 2307

Dynamically sized arrays in C

I'm attempting to learn C using http://c.learncodethehardway.org/ but I'm stuck on one of the extra credit questions in chapter 18 (http://c.learncodethehardway.org/book/learn-c-the-hard-waych18.html) and I'm hoping someone can help me.

The specific problem I'm having is there are a couple of structs defined like this:

#define MAX_ROWS = 500;
#define MAX_DATA = 512;

struct Address {
    int id;
    int set;
    char name[MAX_DATA];
    char email[MAX_DATA];
};

struct Database {
    struct Address rows[MAX_ROWS];       
};     

struct Connection {
    FILE *file;
    struct Database *db;
};

The challenge is to rework that so that rows can have a variable size that doesn't rely on that constant.

So in my Database_create method I'm attempting to initialize rows with the following:

conn->db->rows = (struct Address*) malloc(max_rows * sizeof(struct Address));

where conn->db points to an instance of Database and max_rows is an int that gets passed to the function. I've also changed the Database struct to

struct Database{
    struct Address* rows;
}

That bit of code seems to run ok but if I try and access any member of rows I get a segmentation fault which I believe means I'm attempting to access bits of memory that aren't in use.

I've spent a good few hours on this and I'm sure I can't be too far off but I'd really appreciate any guidance to get me on the right track.

EDIT: Just wanted to add some more detail to this after running it with Valgrind, that throws up the error:

==11972== Invalid read of size 4
==11972==    at 0x100001578: Database_set (ex18.c:107)
==11972==    by 0x100001A2F: main (ex18.c:175)
==11972==  Address 0x7febac00140c is not stack'd, malloc'd or (recently) free'd

The line of code it's point to is:

struct Address *addr = &conn->db->rows[id];
if(addr->set) die("Already set, delete it first");

Line 107 is the if(addr->set) which I presume means it's trying to read something it can't

Upvotes: 3

Views: 244

Answers (3)

Daniel Fischer
Daniel Fischer

Reputation: 183873

When you're loading

void Database_load(struct Connection *conn)
{
        int rc = fread(conn->db, sizeof(struct Database), 1, conn->file);
        if(rc != 1) die("Failed to load database");
}

or writing the database,

void Database_write(struct Connection *conn)
{
        rewind(conn->file);

        int rc = fwrite(conn->db, sizeof(struct Database), 1, conn->file);
        if(rc != 1) die("Failed to write database");

        rc = fflush(conn->file);
        if(rc == -1) die("Cannot flush database.");
}

you're not reading or writing the contents (the struct Addresses), but only the pointer to the memory location. When reading a previously written database, that pointer doesn't point at anything specific, it's a wild pointer. Then of course trying to dereference it is very likely to cause a segmentation fault, if perchance it doesn't, you'll get nonsense pseudo-data.

If you change your struct Database so that rows is a struct Address*, you need to keep a count of items and change your reading and writing code to also handle the data pointed to by rows. First write how many items you have, then write the rest (max_data, max_rows and the items); and when reading, read how many items you have, allocate space for them, read max_data and max_rows and the items.

Upvotes: 1

Eric Finn
Eric Finn

Reputation: 8985

EDIT: Well, it looks like (with your latest edit), you actually are doing this part properly.

You are not actually allocating enough size for the Address structures. What you need is something like this:

struct Database{
    struct Address** rows;
}
//create array of pointers to Address structures
// (each element has size of the pointer)
conn->db->rows = (struct Address**) malloc(max_rows * sizeof(struct Address*));
for(int i=0; i < max_rows; i++) {
    conn->db->rows[i] = (struct Address*) malloc(sizeof(struct Address));
}

or this:

struct Database{
    struct Address* rows;
}
//create array of Address structures
// (each element has size of the structure)
conn->db->rows = (struct Address*) malloc(max_rows * sizeof(struct Address));

Upvotes: 1

Dan F
Dan F

Reputation: 17732

You want sizeof(struct Address) not sizeof(struct Address*)

sizeof(struct Address*) is likely returning a size of 4 (completely dependent on the target platform, though) whereas the actual size of the Address struct is closer to 1040 (assuming 1 byte per char and 4 per int)

Upvotes: 2

Related Questions