EX17 Dynamic Resizing of MAX_DATA Problem

C
#1

The problem is trying to change the Address Struct’s array sizes. I’m not sure where I’m exactly supposed to access them to change.

I’ve tried to change MAX_DATA so that it’s not a constant. However, this throws a compiler error. Trying to change MAX_DATA within the running code doesn’t work either. I’m right now passing the value from commandline arguments, however I’m not sure how to proceed.

My questions are:
How do you dynamically resize the address struct array?
Where would you access this code from?

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

#define MAX_DATA 512 //This is the Max Data size. However, this is used as
#define  MAX_ROWS 100
#define  OUTPUT_ON_NOTHING  1 //
struct Address { //Defines an address  as a fixed size. This makes it less efficent but easier to store and read.
	int id;
	int set;
	char name[MAX_DATA];
	char email[MAX_DATA];
};

struct Database { //Database also  a fixed size structure. Allows you to write entire thing to disk in one move later on.
	struct Address rows[MAX_ROWS];
};

struct Connection {
	FILE *file; //Introduced to new fucntions like fopen, fread, fclose, rewind .  Defined by standard C library
	struct Database *db;
};

//Forward Declarations, prevent chicken and egg problem. Where you're using a function before its defined.  Allows keeping code more organized
void Database_close(struct Connection *conn);

void die(const char *message, struct Connection *conn) //In small programs can make a single fucntion that kills the program with an error if there's anything wrong. This case called the die function.
{
	if(errno){ //Where you have an error return from a fuction, it will usually set an external variable called erno to say exactly what happened. These are just numbers.
		perror(message);  //Use perror to print the error message
  } else {
    printf("Closin connection to database...\n");
    Database_close(conn);
    printf("ERROR: %s\n",message);

  }
  exit(1);
}
void Address_print(struct Address *addr)
{
	printf("%d %s %s\n", addr->id, addr->name, addr->email);
}

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.",conn);
}

struct Connection *Database_open(const char *filename, char mode)
{
	struct Connection *conn = malloc(sizeof(struct Connection)); //Requests  from the OS the required amount of memory.
	if (!conn) //Null is O. So this is the same as asking if (ptr == NULL) die("fail!");
		die("Memory error",conn);
	conn->db = malloc(sizeof(struct Database));
	if (!conn)
		die("Memory error",conn);

	if (mode == 'c') {
		conn->file = fopen(filename,"w");
	} else {
		conn->file = fopen(filename,"r+");

		if(conn->file) {
			Database_load(conn);
		}
	}

	if(!conn->file)
		die("Failed to open the file",conn);

	return conn;
}

void Database_close(struct Connection *conn)
{
	if(conn){
		if(conn->file)
			fclose(conn->file);
		if(conn->db)
			free(conn->db);
	}
}

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 to database",conn);
	rc = fflush(conn->file);
	if (rc == -1)
		die("Cannot flush database.",conn);
}

/**
Creates a database with name and address of set length. If null is passed for length or less than 0 is passed. Then 512 is used.
*/
void Database_create(struct Connection *conn, int length)
{
  int recievedLength = length;
  if(length <= 0){
    recievedLength = MAX_DATA;
  }
	int i = 0;
  printf("%d",recievedLength);
	for (i = 0; i < MAX_ROWS; i++)
	{
		//make a prototype to initialize it
		struct Address addr = {.id = i,.set=0};
		// then just assign it
		conn->db->rows[i] = addr;
	}
}

void Database_set(struct Connection *conn, int id, const char *name, const char *email)
{
	struct Address *addr = &conn->db->rows[id]; //Nested struct pointers. This code reads "get the i element of rows, which is in db, which in  conn, then get the address of it"
	if(addr->set)
		die("Already set, delete it first",conn);

	addr->set = 1;
	// WARNING: bug, read the "How to Break It" and fix this
	char *res = strncpy(addr->name,name, MAX_DATA);
	// demonstrate the strncpy bug
	if(!res)
		die("Name copy failed",conn);

	res = strncpy(addr->email, email, MAX_DATA);
	if(!res)
		die("Email copy failed",conn);
}

void Database_get(struct Connection *conn, int id)
{
	struct Address *addr = &conn->db->rows[id];

	if(addr->set) {
		Address_print(addr);
	} else {
		die("ID is not set",conn);
	}
}

void Database_delete(struct Connection *conn, int id)
{
	struct Address addr = {.id = id, .set = 0};//Creating a temporarly local address, initalizng its id and set fields and then copying it into the rows array by assinging it tothe element I want.
	//This trick maes sure that all the fields except set and id are initialized to  zeros and it's actually easier to write.
	//WARNING: Don't use memcpy to do these kinds of struct copying operatins. Modern C allows you to simply assing one struct to another and it'll handle the copying for you.
	conn->db->rows[id] = addr;
}

void Database_list(struct Connection *conn)
{
	int i = 0;
  int numTimesOutputted = 0;
	struct Database *db  = conn->db;
	for(i = 0; i < MAX_ROWS; i++) {
		struct Address *cur = &db->rows[i];

		if (cur->set) {
			Address_print(cur);
      numTimesOutputted++; //WARNING: Possible Stack overflow
		}
	}
  if(numTimesOutputted == 0){
    if(OUTPUT_ON_NOTHING == 1){
      printf("<Database Empty>\n");
    }
  }
}

int main(int argc, char *argv[])
{
	//HEre I proccess complex argument. This isn't the BEST way to do it.
  printf("%d",MAX_DATA);
	if(argc < 3){
	    printf("Invalid action: c=create, g=get, s=set, d=del, l=list\n");
		die("USAGE: ex17 <dbfile> <action> [action parms]",NULL);}
	char *filename = argv[1];
	char action = argv[2][0];
	struct Connection *conn = Database_open(filename, action);
	int id = 0;

	if(argc > 3) id = atoi(argv[3]); //Converting a String to an int
  if(id >= MAX_ROWS) die("There's not that many records.",NULL);
	switch(action) {
		case 'c':
      if(argc > 3){
        Database_create(conn,id );
      }
			Database_create(conn,-1);//Negative number or zero, default to MAX_DATA constant defined in the header of this file. //Allocating large memeory using Malloc. Malloc creates data on the heap.  Whenever MALLOC is used. You're using heap memory. When you're creating variables. That's using stack memory.
			Database_write(conn);
			break;
		case 'g':
			if (argc != 4)
				die("Need an id to get",conn);
			Database_get(conn, id);
			break;
		case 's':
			if (argc != 6)
				die("Need id, name, email to set",conn);
			Database_set(conn,id,argv[4],argv[5]);
			Database_write(conn);
			break;
		case 'd':
			if (argc != 4)
				die("Need id to delete",conn);
			Database_delete(conn, id);
			Database_write(conn);
			break;
		case 'l':
			Database_list(conn);
			break;
		default:
			die("Invalid action: c=create, g=get, s=set, d=del, l=list",conn);
	}

	Database_close(conn);
	return 0;
}
#3

Ok and you’re referring to this right?

“Change the code to accept parameters for MAX_DATA and MAX_ROWS, store them in the Database struct, and write that to the file, thus creating a database that can be arbitrarily sized.”

So the first clue is that you have to store max_data and max_rows in the Database struct rather than as a #define at the top. A first step is to NOT allow this to change on the command line, but instead you make a default Database that has the same size set for these two. So something like this:

struct Database {
        size_t max_data;
        size_t max_rows;
	struct Address *rows;
};

Now, once you have that in there, and you’ve changed all your code to use this new form of Database where you’re forcing it to a fixed size, then you can take in a parameter from the user fairly easily. Remember that they only give these parameters when you create the database, and then they’re fixed.

The next thing to realize is that I’ve changed the database rows to a pointer to Address structs. So, your saving procedure is something like this:

  1. Write the database struct, but assume that rows is garbage since it points at ram in the computer.
  2. Write out the rows pointer (treat it like an array) but use the max_rows count to do it the correct number.
  3. However, each Address has the same setup, since you also have max_data as dynamic. That means you write the address struct, then write name and email of max_data length to the file.

Then when you load the database it’s the reverse:

  1. Load the database struct, but ignore rows. It’s garbage.
  2. Read the max_rows number of Address structs.
  3. After loading the Address struct, assume name and email (which are now char * right?) is garbage, then read database->max_data from the file twice. First time assign that to name. Second assign that to email.

Now, this is a lot to do, so I think a good strategy is this:

  1. Get only max_rows to work dynamically but with you setting a fixed count in your code.
  2. Once you’ve moved to that, get max_data working with the dynamic addresses. What’s nice is you can set max_rows to 10 or so to make this go quicker.
  3. Once you have max_data and max_rows dynamic, you can then allow people to set it on the command line.

Let me know if that helps, and also if this is a lot to get then you can probably skip this and come back later.

Ex17 Seg fault when setting new entries
#5

Definitely a confusing extra credit. I changed the Database struct declaration to match yours. Then I edited Database_create to generate a fixed sized db:

void Database_create(struct Connection *conn)
{
    int i = 0;
    struct Address rows[10];
    for (i = 0; i < 10; i++) {
        rows[i] = (struct Address){.id = i};
    }
    conn->db->rows = rows;
    for (i = 0; i < 10; i++) {
        // make a prototype to initialize it
        struct Address addr = {.id = i, .set = 0 };
        // then just assign it
        conn->db->rows[i] = addr;
    }
}

Something gets written to file, but trying to set an entry doesn’t work. I’m not quite understanding what you mean by writing the database struct, then writing out the rows pointer, then writing out each Address’s name & email. It’s this all happening during the the file writing stream in Database_write?

#6

So, you have a Database, and an Address struct right? The Database struct contains the number of rows, then a pointer to all of them. You can also get the sizeof(struct Database) and sizeof(struct Address). If you write out your database file like this (this is pseudo code):

size_t db_len = sizeof(struct Database);
size_t row_len = sizeof(struct Address) * db->max_rows;

fwrite(conn->db, db_len, 1, outfile);
fwrite(conn->db->rows, row_len, 1, outfile);

Then you’d be able to first read back the DB header, put that into the struct, then use that struct to read back all the rows. Now if the data is also randomly sized then you’d need to instead change that last line to a for-loop that went through each row and wrote that row to the file using the max_data size plus the address header.

The idea is that, you can write a struct to the file as raw bytes, then read those raw bytes back into the struct and it’ll come back. Now all you have to do is use the Database struct to save the max_data, max_row, information, then write out all the rows. Getting them back is just the reverse process.

2 Likes
#7

I’m actually glad that I was on the right track by thinking that there was something about the way this stuff was written out to file. I use gdb all the time now, plus I can load the binary in a separate pane in my editor to watch things update. I could see that the default db wasn’t quite writing out the way it was supposed to. I just had a brain fart as to which side of things needed fixing to resolve the issue, the process of getting the information to file properly, constructing the database struct properly, or reading it from the file correctly. Thanks to your information I can see it might be a combination of the 3. Clearly I still have a lot of thinking to do, but I have all the time in the world and I really want to make sure I’m understanding things and doing the extra credit before I proceed with the course. So I’m going to ‘stubborn-bastard’ this until I’ve got it sorted out :laughing:.

1 Like