Thread: determining the size of a nested struct with allocated variables

This code is from LearnCtheHardWay Chapter 18 Exercise 17: Heap And Stack Memory Allocation. (18 Exercise 17: Heap And Stack Memory Allocation)

In the Extra credit section, it asks you to input Maxdata and Maxrow and not use a static number. Here is the code:

Code:

/**************************
*        Lession 9        *      
***************************/

/*
 * Extra Credit
 * ============
 * 
 */


#include <stdio.h>  /* used for printf, and FILE struct */
#include <assert.h> /* used for debuging */
#include <stdlib.h> /* malloc and free are defined here */
#include <string.h> /* string includes strdup, duplicates a string */
#include <errno.h>  /* error header */

#define MAX_DATA 512
#define MAX_ROWS 100



struct Address {
	int id;
	int set;
	char *name;
	char *email;
};


/*
 * struct rows is actually allocated on the stack.
 */
struct Database {
	long MaxRows;
	long MaxData;
	struct Address *rows;
};


/*
 * FILE object are usually created by fopen or tmpfile which both return a reference to 
 * these objects. both of these functions allocate memory, so use fclose to free the resources
 */
struct Connection {
	FILE *file;
	struct Database *db;
};




long Database_size(struct Connection *conn)
{
	//calculate the size of the file
	long t_dbSize = sizeof(struct Database);
	t_dbSize += sizeof(struct Address) * conn->db->MaxRows;
	t_dbSize += (sizeof(char) * conn->db->MaxData * 2) * conn->db->MaxRows;

	return t_dbSize;
}



void Database_close(struct Connection *conn)
{
	int t_i = 0;

	if(conn) {
		if(conn->file) fclose(conn->file);
		if(conn->db->rows) {
			for(t_i = 0; t_i < conn->db->MaxRows ; t_i++) {
				if(conn->db->rows[t_i].name) free(conn->db->rows[t_i].name);	
				if(conn->db->rows[t_i].email) free(conn->db->rows[t_i].email);	
			}
			free(conn->db->rows);
		}
		if(conn->db) free(conn->db);
		free(conn);	
	}	
}

void die(const char *message, struct Connection *conn)
{

/* 
 * when there is an error returned from a function, it will set an external variable
 * called errno to say exactly what error happened. The errno is just a number, and 
 * the perror function will print the error message of that number.
 */
 
	if(errno) {
		perror(message);
	} else {
		printf("ERROR: %s\n", message);
	}

	/*close the connection*/
	Database_close(conn);
	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)
{
/*
 * problem.
 * the sizeof(struct Database) is not valid here. The sizeof will return the pointer size
 * since the variables inside of the struct Database is *rows then it's dynamically allocated,
 * the sizeof need to be fixed.
 */

	long t_dbSize = Database_size(conn);

	int rc = fread(conn->db, t_dbSize, 1, conn->file);
	if(rc != 1) 
		die ("Failed to load database.", conn);
}

struct Connection* Database_open(const char *filename, char mode, long in_maxData, long in_maxRow)
{

	//allocate memory for a connection struct
	struct Connection *conn = malloc(sizeof(struct Connection));
	if(!conn) die("Memory error", conn);

	//allocate memory for db variable
	conn->db = malloc(sizeof(struct Database));
	if(!conn->db) die("Memory error", conn);

	//set the max row and data numbers
	conn->db->MaxRows = in_maxRow;
	conn->db->MaxData = in_maxData;
	
	/* 
 	* allocate memory for the rows variable
	* rows is an array of size conn->db->MaxRows, and each a size of struct Address type
	* so conn->db->rows[1] is a struct Address type, in which two variables of the type char* need to 
	* have memory allocated for them. for each of these pointers, we need to allocate 
	*/  
	conn->db->rows = malloc(sizeof(struct Address) * in_maxRow);
	if(!conn->db->rows) die("Memory error", conn);

	int t_i = 0;
	for(t_i = 0 ; t_i < conn->db->MaxRows ; t_i++)
	{
		conn->db->rows[t_i].name = malloc(sizeof(char) * conn->db->MaxData);
		if(!conn->db->rows[t_i].name) die("Memory error", conn);

		conn->db->rows[t_i].email = malloc(sizeof(char) * conn->db->MaxData);
		if(!conn->db->rows[t_i].email) die("Memory error", conn);
	}
	

	//if mode is c, then the file is created new
	//if it's not c, then it's open for reading and also writable,
	// r+ is open a file for both reading and writting, the file must exist
	// w creates an empty file for writing, if file exists it's erased, else 
	// a new one is created
	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_write(struct Connection *conn)
{
	//set the position indicator with the stream to the begining of the file
	rewind(conn->file);

	long t_dbSize = Database_size(conn);
	
	//write to the file, the content of the conn->db array
	int rc = fwrite(conn->db, t_dbSize, 1, conn->file);
	if(rc != 1) die("Failed to write database.", conn);
	
	//any unwritten data in the output buffer is written to the file
	//the stream shall be flushed after an output operation before
	//performing an input operation
	rc = fflush(conn->file);
	if(rc == -1) die("Cannot flush database", conn);
}

void Database_create(struct Connection *conn)
{
	int i = 0;
/*
 * for all the rows in the database, create a struct object addr, and assign
 * default values as means of initialization 
 */
	for(i = 0 ; i < conn->db->MaxRows; i++) {
		conn->db->rows[i].id = i;
		conn->db->rows[i].set = 0;
	}
}

void Database_set(struct Connection *conn, int id, const char *name, const char *email)
{
	//get the element at the specified id	
	struct Address *addr = &conn->db->rows[id];
	if(addr->set) die("Already set, delete it first", conn);

	//set the set bit to 1
	addr->set = 1;

	/*copy name to addr->name 
	*char *strncpy(char *restrict s1, const char *restrict s2, size_t n);
	*strncpy shall copy not more than n bytes, (all bytes after a null byte are not copied)
	*from s2, to s1. if size of s2 is less than n, then null bytes are appended to the end
	* of s1 until all n bytes are written.
	*
	* strncpy, doens't crash if name or addr->name is greater than MAX_DATA. 
	* Ask Zed about this
	*/

	memset(addr->name, '\0', conn->db->MaxData);
	memset(addr->email, '\0', conn->db->MaxData);

	char *res = strncpy(addr->name, name, conn->db->MaxData);
	if(!res) die("Name copy failed", conn);

	res = strncpy(addr->email, email, conn->db->MaxData);
	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)
{
	conn->db->rows[id].id = id;
	conn->db->rows[id].set = 0;
}

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

		if(cur->set) {
			Address_print(cur);
		}
	}
}



int main(int argc, char *argv[])
{
	if(argc < 5) die("USAGE: lesson_9 <dbfile> <Max_rows> <Max_data>  <action> [action params]",NULL);	

	char *filename = argv[1]; /* the first argument is the filename */
	char action = argv[4][0]; /* argv[2] is a pointer to a char and argv[2][0] is the first char  */
	long t_max_row = atoi(argv[2]);
	long t_max_data= atoi(argv[3]);

	struct Connection *conn = Database_open(filename, action,t_max_data,t_max_row);
	int id = 0;

	if(argc > 5) id = atoi(argv[5]);
	if(id >= t_max_row) die("There not that many records", conn);

	switch(action) {
		case 'c':
			Database_create(conn);
			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 != 8) die("Need id, name, email to set", conn);

         Database_set(conn, id, argv[6], argv[7]);
         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, only: c=create, g=get, s=set, d=del, l=list", conn); 
	}

	Database_close(conn);

	return 0;
}

I'm having difficulty determining the size of the Database struct to use it when writting or reading from a file:
fwrite(conn->db, t_dbSize, 1, conn->file);

the t_dbSize is calculated using the Database_size function. Does Database_size calculate the correct amount?

O_o

Yog Sothoth!

Okay. The syntax looks fine. The style could use some work.

Beyond that, everything related to serialization (file operations) is horribly broken. The thing you are serializing to and from a file has pointers to an unknown location. The data associated with the structures that contain those pointers may or may not be contiguous in memory so you can not simply write them out to a file or read them back from a file as one large array.

You have to populate the members as is appropriate to the nature of the given type. So, for example, you have to write variables associated with `Address::email' as a "C string", "Pascal string", or other semantic format and read them by allocating an appropriate memory block and writing the file contents to that memory location.

Soma

Thank you Soma for your reply,
I thought I allocate those in Database_open function. Where I have:

Code:

for(t_i = 0 ; t_i < conn->db->MaxRows ; t_i++)
{
      conn->db->rows[t_i].name = malloc(sizeof(char) * conn->db->MaxData);
      if(!conn->db->rows[t_i].name) die("Memory error", conn);

      conn->db->rows[t_i].email = malloc(sizeof(char) * conn->db->MaxData);
      if(!conn->db->rows[t_i].email) die("Memory error", conn);
}

I changed Database_size return type to size_t and now it works but only when running in gdb, but not when I run it from the terminal.

the problem is you are writing pointers to the file, not the data itself. when you write pointers and read them back they probably won't be valid. depends on what happens to the heap in the meantime. for sure if you write the file in one instance of the program. then read it back in a separate instance the pointers won't point to the data. the fact that it works in gdb is because you got lucky that when you wrote the data then read it back the pointers were still pointing to the original data. but there is no guarantee of that. so you have to do something like this to write a record:

Code:

write maxrows
write max data
for each row in Address:
   write id
   write set
   write name
   write email

then when you read it back you need to do similar:

Code:

read maxrows
read maxdata
allocate memoryfor Address
for each row in Address
   read id
   read set
   allocate memory to store 'name'
   read  from file into 'name'
   allocate memory to store 'email'
   read from file in 'email'

I'm working on the same problem, and I tried to follow dmh2000's basic algorithm for reading from and writing to the file.

I'm seeing problems with the Database write method:

Code:

void Database_write(struct Connection *conn)
{
    rewind(conn->file); //set position marker at beginning
    struct Database *db = conn->db;  

    fwrite(&db->max_rows, sizeof(int), 1, conn->file); //write max_rows
    fwrite(&db->max_data, sizeof(int), 1, conn->file); //write max_data
    
    ....

}

I write these two integers to the start of the file, but when I try to read them in the Database read method, I keep getting 0 as the value.

Code:

void Database_load(struct Connection *conn)
{
    int max_rows = 0; int max_data = 0;
    struct Database *db = conn->db;    
    
    fread(&max_rows, sizeof(int), 1, conn->file); //read max rows
    fread(&max_data, sizeof(int), 1, conn->file); //read max data
    ......
}

I've only been learning C for a few days, so I'm not sure if I'm making an obvious mistake here that I can't recognize. But, I would appreciate your advice. Thanks.