Small database with a user defined size

Question

This is a program that accepts user input for a database size - number of rows and data size. After creating the database the user should be able to read, add, remove and list entries and the file will be updated accordingly. Please help me improve this code. I think its a bit a mess, is there a better way to organize it? or a better cleaner way to implement the read/write to database part. Thanks.

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


typedef struct address_s {
    int id;
    int set;
    char *name;
    char *email; 
}address_t;

typedef struct database_s {
    int db_max_rows;
    int max_data;
    address_t *rows;
}database_t;

typedef struct connection_s {
    FILE *file;
    database_t *db;
}connection_t;

void database_close(connection_t *conn)
{
    if(conn)
    {
        if(conn->file) fclose(conn->file);

        if(conn->db) 
        {
            for(int i = 0; i < conn->db->db_max_rows; i++)
            {
                free(conn->db->rows[i].name);
                free(conn->db->rows[i].email);
            }

            free(conn->db);
        }

        free(conn);
    }
}


void die(const char *message, connection_t *conn)
{
    if(errno)           
    {
        perror(message);
    }
    else
    {
        printf("ERROR: %s\n", message);
    }

    database_close(conn);
    exit(1);
}

void address_print(address_t *addr)
{
    printf("%d %s %s\n", addr->id, addr->name, addr->email);
}

void database_read_rows(connection_t *conn)
{
    size_t row_size = conn->db->max_data * 2 + sizeof(int) * 2;
    void *row_buf = malloc(row_size);
    int max_rows = conn->db->db_max_rows;

    for(int i = 0; i < max_rows; i++)
    {
        int offset = 0;

        if (fread(row_buf, row_size, 1, conn->file) != 1)
        {
            die("Failed to load database.", conn);
        }

        memcpy(&conn->db->rows[i].id, row_buf + offset, sizeof(conn->db->rows[i].id));
        offset +=  sizeof(conn->db->rows[i].id);

        memcpy(&conn->db->rows[i].set, row_buf + offset, sizeof(conn->db->rows[i].set));
        offset +=  sizeof(conn->db->rows[i].set);

        conn->db->rows[i].name = malloc(conn->db->max_data);    
        if(!conn->db->rows[i].name)
        {
            die("memory error name", conn);
        }

        memcpy(conn->db->rows[i].name, row_buf + offset, conn->db->max_data);
        offset +=  conn->db->max_data;

        conn->db->rows[i].email = malloc(conn->db->max_data);
        if(!conn->db->rows[i].email)
        {
            die("memory error email", conn);
        }

        memcpy(conn->db->rows[i].email, row_buf + offset, conn->db->max_data);
    }
    free(row_buf);
}


void database_write_rows(connection_t *conn)
{
    size_t row_size = conn->db->max_data * 2 + sizeof(int) * 2;
    void *row_buf = malloc(row_size);
    int max_rows = conn->db->db_max_rows;

    for(int i = 0; i < max_rows; i++)
    {
        int offset = 0;

        memcpy(row_buf + offset, &conn->db->rows[i].id, sizeof(conn->db->rows[i].id));
        offset +=  sizeof(conn->db->rows[i].id);

        memcpy(row_buf + offset, &conn->db->rows[i].set, sizeof(conn->db->rows[i].set));
        offset +=  sizeof(conn->db->rows[i].set);

        memcpy(row_buf + offset, conn->db->rows[i].name, conn->db->max_data);
        offset +=  conn->db->max_data;

        memcpy(row_buf + offset, conn->db->rows[i].email, conn->db->max_data);
        if (fwrite(row_buf, row_size, 1, conn->file) != 1)
        {
            die("Failed to load database.", conn);
        }
    }

    free(row_buf);
}

void database_read(connection_t *conn)
{
    conn->db = malloc(sizeof(database_t));  
    if(!conn->db) 
    {
        die("Memory error database_load.", conn);
    }

    if (fread(conn->db, sizeof(database_t), 1, conn->file) != 1)
    {
        die("Failed to load database.", conn);
    }

    conn->db->rows = malloc(sizeof(address_t) * conn->db->db_max_rows);
    if (!conn->db->rows)
    {
        die("memory error rows", conn);
    }

    database_read_rows(conn);
}

void database_write(connection_t *conn)
{
    rewind(conn->file);         

    if (fwrite(conn->db, sizeof(database_t), 1, conn->file) != 1)
    {
        die("Failed to load database.", conn);
    }

    database_write_rows(conn);

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

connection_t *database_open(const char *filename, char mode)
{
    connection_t *conn = malloc(sizeof(connection_t));

    if(!conn) 
    {
        die("Memory error conn.", conn);    
    }

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

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

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

    return conn;
}



void database_create(connection_t *conn, int max_data, int max_rows)
{
    conn->db = malloc(sizeof(database_t));

    conn->db->db_max_rows = max_rows;
    conn->db->max_data = max_data; 
    conn->db->rows = malloc(sizeof(address_t) * conn->db->db_max_rows);

    for(int i = 0; i < conn->db->db_max_rows; i++)
    {
        address_t addr = { .id = i, .set = 0 };
        conn->db->rows[i] = addr;
        conn->db->rows[i].name = malloc(sizeof(max_data));
        if (!conn->db->rows[i].name) 
        {
            die("Memory error name", conn);
        }
        conn->db->rows[i].email = malloc(sizeof(max_data));
        if (!conn->db->rows[i].email)
        {
            die("Memory error email", conn);
        }
    }
}


void database_set(connection_t *conn, int id, const char *name, const char *email)
{
    address_t *addr = &conn->db->rows[id];
    if(addr->set) die("Already set, delete it first", conn);

    addr->set = 1;
    char *res = strncpy(addr->name, name, conn->db->max_data);      
    if(!res) 
    {
        die("Name copy failed", conn);
    }

    addr->name[conn->db->max_data - 1] = '\0';

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

    addr->email[conn->db->max_data - 1] = '\0';
}

void database_get(connection_t *conn, int id)
{
    address_t *addr = &conn->db->rows[id];

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

void database_delete(connection_t *conn, int id)
{
    address_t addr = {.id = id, .set = 0};
    conn->db->rows[id] = addr;
}

void database_list(connection_t *conn)
{
    int i = 0;
    database_t *db = conn->db;

    for(i = 0; i < db->db_max_rows; i++)
    {
        address_t *cur = &db->rows[i];

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



int main(int argc, char *argv[])
{
    if(argc < 3) die("USAGE: db_prog <dbfile> <action: c=create, g=get, s=set, d=del, l=list> [action params]", NULL);

    char *filename = argv[1];
    char action = argv[2][0];
    connection_t *conn = database_open(filename, action);
    int id = 0;

    if(argc > 3) id = atoi(argv[3]); 

    switch(action)
    {
        case 'c':
            if(argc != 5) die("Usage: db_prog <dbfile> create [Max_Rows] [Max_Data]", conn);
            database_create(conn, atoi(argv[3]), atoi(argv[4]));
            database_write(conn);
            break;

        case 'g':
            if(argc != 4) die("Usage: db_prog <dbfile> get [id]", conn);

            database_get(conn, id);
            break;

        case 's':
            if(argc != 6) die("Usage: db_prog <dbfile> set [name] [email]", conn);

            database_set(conn, id, argv[4], argv[5]);
            database_write(conn);
            break;

        case 'd':
            if(argc != 4) die("Usage: db_prog <dbfile> delete [id]", 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;
}

Answer 1

Types

You may want to reconsider use _t endings in your type names. In short, most standard POSIX type names use this, and the general practice is to not do so with user-defined types.

Also, when declaring your type with typedef you can use same name for both parts, this is perfectly valid.

Lets rewrite your address_t type declaration:

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

It is also common to pick some suffix for type names, like in DatabaseAddress or Database_Address.

Since your program compiles to single binary it is fine to keep all your type declarations and code in same file.

Database read/write

Aha, I see. You write memory behind conn->db directly to database file. And in database_read function you read your header back to conn->db memory.

Although it works is has several pitfalls.

Your database struct size equals sizeof(database_t). Now guess what will happen if someday your compiler int becomes 8 bytes long. Or 9 bytes. I bet old database files will stop working. That's not nice.

To guard from that kind of stuff we will use types from stdint.h:

typedef struct Database {
    uint32_t db_max_rows;
    uint32_t max_data;
    Address *rows;
} Database;

Note: I've used unsigned type because negative db_max_rows and max_data doesn't make sense.

Also you can't share same database file between hosts with different endianness. To overcome this we will use ntohl function from arpa/inet.h to convert uint32_t's in database file from network byte order to host.

Let's add some protection from opening random non-database files. Our header will have special magic value just like PNG and other file headers use to do. I will use "database" (without quotes).

So this is what we have for now:

database   - 8 bytes (magic value)
uint32_t   - 4 bytes in network order (db_max_rows)
uint32_t   - 4 bytes in network order (max_data)

Example function to read database looks like this:

void database_read(Connection *conn) {

    conn->db = calloc(1, sizeof(Database));
    if (!conn->db) {
        die("No memory");
    }

    /* 16 bytes header */
    uint32_t header[4];

    if (fread(header, sizeof(header), 1, conn->file) != 1) {
        if (ferror(conn->file)) {
            die("Error reading file");
        } else {
            die("Unexpected end of file");
        }
    }

    /* checking magic value "database" to protect from non-database files */
    if (memcmp("database", header, sizeof("database") - 1)) {
        die("Invalid magic value");
    }

    conn->db->db_max_rows = ntohl(header[2]);
    conn->db->max_data = ntohl(header[3]);

    conn->db->rows = malloc(sizeof(Address) * conn->db->db_max_rows);
    if (!conn->db->rows) {
        die("No memory");
    }

    database_read_rows(conn);
}