UPDATE
This post has been closed because my program actually had an error even though it worked in GDB so technically it wasn't meant for reviewing. However, after the help of some of friends here, I managed to fix my code and I've posted it below in the UPDATE section. I still left the ORIGINAL message though.
#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 */
/* Structs */
struct Address {
long 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;
};
/* Prototypes */
void Database_close(struct Connection *conn);
void die(const char *message, struct Connection *conn);
void Address_print(struct Address *addr);
void Database_load(struct Connection *conn);
struct Connection* Database_open(const char *filename, char mode);
void Database_write(struct Connection *conn);
void Database_create(struct Connection *conn, long in_maxRow, long in_maxData);
void Database_set(struct Connection *conn, int id, const char *name, const char *email);
void Database_get(struct Connection *conn, int id);
void Database_delete(struct Connection *conn, int id);
void Database_list(struct Connection *conn);
/* implementations */
struct Connection* Database_open(const char *filename, char mode)
{
//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);
//open the file
if(mode == 'c') {
conn->file = fopen(filename, "w");
if(conn->file) {
conn->db->MaxRows = 1;
conn->db->MaxData = 1;
}
} 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_load(struct Connection *conn)
{
//set the position indicator with the stream to the begining of the file
rewind(conn->file);
int rc = fread(&conn->db->MaxRows, sizeof(long), 1, conn->file);
if(rc != 1) die ("Failed to load database.", conn);
rc = fread(&conn->db->MaxData, sizeof(long), 1, conn->file);
if(rc != 1) die ("Failed to load database.", conn);
//allocate memory based on the size extracted from the file
conn->db->rows = malloc(sizeof(struct Address) * conn->db->MaxRows);
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);
}
//write each address to the file
for(t_i = 0 ; t_i < conn->db->MaxRows ; t_i++)
{
rc = fread(&conn->db->rows[t_i].id, sizeof(long), 1, conn->file);
if(rc != 1) die("Failed to write database.", conn);
rc = fread(&conn->db->rows[t_i].set, sizeof(int), 1, conn->file);
if(rc != 1) die("Failed to write database.", conn);
rc = fread(conn->db->rows[t_i].email, sizeof(char) * conn->db->MaxData, 1, conn->file);
if(rc != 1) die("Failed to write database.", conn);
rc = fread(conn->db->rows[t_i].name, sizeof(char) * conn->db->MaxData, 1, conn->file);
if(rc != 1) die("Failed to write database.", conn);
}
}
void Database_create(struct Connection *conn, long in_maxRow, long in_maxData)
{
//allocate memory for rows
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 < in_maxRow ; t_i++)
{
conn->db->rows[t_i].id = t_i;
conn->db->rows[t_i].set = 0;
conn->db->rows[t_i].name = malloc(sizeof(char) * in_maxData);
if(!conn->db->rows[t_i].name) die("Memory error", conn);
conn->db->rows[t_i].email = malloc(sizeof(char) * in_maxData);
if(!conn->db->rows[t_i].email) die("Memory error", conn);
}
conn->db->MaxData = in_maxData;
conn->db->MaxRows = in_maxRow;
}
void Database_write(struct Connection *conn)
{
//set the position indicator with the stream to the begining of the file
rewind(conn->file);
//write the MaxRow size into to the file
int rc = fwrite(&conn->db->MaxRows, sizeof(long), 1 , conn->file);
if(rc != 1) die("Failed to write database.", conn);
//write the MaxData size into to the file
rc = fwrite(&conn->db->MaxData, sizeof(long), 1 , conn->file);
if(rc != 1) die("Failed to write database.", conn);
//write each address to the file
int t_i = 0;
for(t_i = 0 ; t_i < conn->db->MaxRows ; t_i++)
{
rc = fwrite(&conn->db->rows[t_i].id, sizeof(long), 1, conn->file);
if(rc != 1) die("Failed to write database.", conn);
rc = fwrite(&conn->db->rows[t_i].set, sizeof(int), 1, conn->file);
if(rc != 1) die("Failed to write database.", conn);
rc = fwrite(conn->db->rows[t_i].email, sizeof(char) * conn->db->MaxData, 1, conn->file);
if(rc != 1) die("Failed to write database.", conn);
rc = fwrite(conn->db->rows[t_i].name, sizeof(char) * conn->db->MaxData, 1, conn->file);
if(rc != 1) die("Failed to write database.", conn);
}
//write unwritten data in the output buffer to the file
rc = fflush(conn->file);
if(rc == -1) die("Cannot flush database", conn);
}
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)
{
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_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 == 1) die("Already set, delete it first", conn);
//set the set bit to 1
addr->set = 1;
memset(addr->name, '\0', conn->db->MaxData);
memset(addr->email, '\0', conn->db->MaxData);
char *res = strncpy(addr->name, name, strlen(name) + 1);
if(!res) die("Name copy failed", conn);
res = strncpy(addr->email, email, strlen(email) + 1);
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 < 3) die("USAGE: lesson_9 <dbfile> <action> [action params]",NULL);
long _maxRow, _maxData = 0;
char *filename = argv[1]; /* the first argument is the filename */
char action = argv[2][0]; /* argv[2] is a string and argv[2][0] is action */
struct Connection *conn = Database_open(filename, action);
int id = 0;
if(argc >= 6) id = atoi(argv[3]);
if(id >= conn->db->MaxRows) die("There not that many records", conn);
switch(action) {
case 'c':
_maxRow = atoi(argv[3]);
_maxData= atoi(argv[4]);
Database_create(conn, _maxRow, _maxData);
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, only: c=create, g=get, s=set, d=del, l=list", conn);
}
Database_close(conn);
return 0;
}
ORIGINAL MESSAGE
I've been following Zed Shaw's LearnCtheHardWay series and I got stuck on Chapter 18 Exercise 17: Heap And Stack Memory Allocation Extra credit exercise number 2. Here is the link to the page: http://c.learncodethehardway.org/book/learn-c-the-hard-waych18.html
The program basically creates a flat-file database and read and writes to it using a fixed size array. Now basically I have to change the code to allow the user to input Row and Data size, which will dynamically allocated instead of using arrays on the stack. The program runs fine from GDB but I get a segmentation fault when I run it from the terminal. So the program works, and it doesn't. That is why I posted the question here. The segmentation fault happens inside the Database_load function, the line which says:
rc = fread(conn->db->rows, sizeof(struct Address) * conn->db->MaxRows, 1, conn->file);
to run the app and see the seg fault, first create a database:
$ ./fileDb db.dat 3 100 c
then try to set a value
$ ./fileDb db.dat 3 100 s 1 zed [email protected]
and here is the code
/**************************
* fileDb.c *
***************************/
/*
* 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 {
long 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;
};
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)
{
int rc = fread(conn->db, sizeof(struct Database), 1, conn->file);
if(rc != 1) die ("Failed to load database.", conn);
rc = fread(conn->db->rows, sizeof(struct Address) * conn->db->MaxRows, 1, conn->file);
if(rc != 1) die ("Failed to load database.", conn);
//write each address to the file
int t_i = 0;
for(t_i = 0 ; t_i < conn->db->MaxRows ; t_i++)
{
rc = fread(conn->db->rows[t_i].email, sizeof(char) * conn->db->MaxData, 1, conn->file);
if(rc != 1) die("Failed to write database.", conn);
rc = fread(conn->db->rows[t_i].name, sizeof(char) * conn->db->MaxData, 1, conn->file);
if(rc != 1) die("Failed to write 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 == '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);
/*
* cannot write the whole structure in a single write.
* The memory is allocated by traversing down the structure path
* and allocating one pointer at a time. This doens't garentee that
* the whole data is in a single continues block
*/
//int rc = fwrite(conn->db, t_dbSize, 1, conn->file);
//write the database to the file
int rc = fwrite(conn->db, sizeof(struct Database), 1 , conn->file);
if(rc != 1) die("Failed to write database.", conn);
rc = fwrite(conn->db->rows, (sizeof(struct Address) * conn->db->MaxRows), 1 , conn->file);
if(rc != 1) die("Failed to write database.", conn);
//write each address to the file
int t_i = 0;
for(t_i = 0 ; t_i < conn->db->MaxRows ; t_i++)
{
rc = fwrite(conn->db->rows[t_i].email, sizeof(char) * conn->db->MaxData, 1, conn->file);
if(rc != 1) die("Failed to write database.", conn);
rc = fwrite(conn->db->rows[t_i].name, sizeof(char) * conn->db->MaxData, 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)
{
printf("id:%d , name:%s , email:%s \n", id, name, email);
//get the element at the specified id
struct Address *addr = &conn->db->rows[id];
printf("addr address :%p \n", addr);
printf("id: %d ", addr->id);
printf("set: %d ", addr->set);
printf("name: %s ", addr->name);
printf("email: %s \n", addr->email);
if(addr->set == 1) die("Already set, delete it first", conn);
printf("after addr->set is called");
//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
*/
printf("before memset is called \n");
memset(addr->name, '\0', conn->db->MaxData);
memset(addr->email, '\0', conn->db->MaxData);
printf("memset is called \n");
char *res = strncpy(addr->name, name, strlen(name) + 1);
if(!res) die("Name copy failed", conn);
printf("strncpy is called \n");
res = strncpy(addr->email, email, strlen(email) + 1);
if(!res) die("Email copy failed", conn);
printf("strncpy email is called \n");
}
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);
printf("Con->db detailsL MaxRows=%d MaxData=%d \n",
conn->db->MaxRows,
conn->db->MaxData);
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]);
printf("Database_set");
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;
}