Thread: record isn't added to file

  1. #1
    Registered User
    Join Date
    Oct 2018
    Posts
    42

    record isn't added to file

    Please look at the code below, it's going to save a simple contact address in a database file, but it's not working. My guess is that the database_add function(at the bottom of the code) is not working properly(especially I'm suspicious about this line: `conn->db->rows[addr->id] = *addr;`, but compiler gives no error. I'd be really grateful if someone takes a look.
    compile, then run it like this:
    1_ ./program dbname.db c
    (c stands for "create")
    2_ ./program dbname.db s 0 "john lee" johnlee@gmail.com
    database file is created but when I look into it I can't find the record.
    thank you.

    Code:
    #include <stdio.h>
    #include <assert.h>
    #include <stdlib.h>
    #include <errno.h>
    #include <string.h>
    
    
    #define MAX_ROWS 250
    #define MAX_DATA 100
    
    
    struct Address{
        int id;
        int set;
        char fullname[MAX_DATA];
        char email[MAX_DATA];
    };
    
    
    struct Database{
        struct Address rows[MAX_ROWS];
    };
    
    
    struct Connection{
        FILE* file;
        struct Database* db;
    };
    
    
    void die(char* msg);
    void database_create(struct Connection*);
    int database_write(struct Connection* conn);
    struct Connection* database_open(char* dbname, char mode);
    void database_close(struct Connection* conn);
    void database_load(struct Connection* conn);
    void database_add(struct Connection* conn, struct Address* addr);
    
    
    
    
    //ex17 mydb.db c
    int main(int argc, char** argv){
        if(argc <= 2) {
            puts("Either no database name(dbname) OR no command was determined.\n\tusage: <program> <dbname> <command_option> <arguments>");
            exit(1);
        }
        char option;
        char* dbname;
        dbname = argv[1];
        option = argv[2][0];
        struct Connection* conn = database_open(dbname, option);
        if(!conn) die("error opening the database");
    
    
        switch(option){
            case 'c':
                database_create(conn);
                database_write(conn);
            break;
            case 's':
                if(argc < 6){
                    die("Error: Provide all fields of the record (id, fullname, email)");
                }
                struct Address addr = {.id=atoi(argv[3]),.fullname=argv[4],.email=argv[5]};
                database_add(conn,&addr);        
                int test_success = database_write(conn);
                if(test_success) puts("Record was added successfully.");
            break;
            default:
                die("Bad usage!\n\tusage: <program> <dbname> <command_option> <arguments>\n\tcommand_option is one of these: c(create) l(list) g(get) s(set) d(delete)");
            break;
        }
    
    
        database_close(conn);
    
    
        return 0;
    }
    
    
    
    
    void die(char* msg){
        if(errno){
            perror(msg);
        }else{
            printf("Error: %s\n",msg);
        }
        exit(1);
    }
    
    
    int database_write(struct Connection* conn){
        rewind(conn->file);
    
    
        int writecount = fwrite(conn->db, sizeof(struct Database), 1, conn->file);
        if(writecount!=1){
            die("database write unsuccessful");
        }
    
    
        //flush
        int rc = fflush(conn->file);
        if (rc == -1)
            die("Cannot flush database.");
    
    
        return 1;
    }
    
    
    void database_create(struct Connection* conn){
    
    
        for(int i=0; i<MAX_ROWS; i++){
            struct Address addr = { .id=i , .set=0 };
            conn->db->rows[i] = addr;
        }
    }
    
    
    struct Connection* database_open(char* dbname, char mode){
        struct Connection* conn;
        conn = malloc(sizeof(struct Connection));
        if(!conn){
            die("memory error\n");
        }
    
    
        conn->db = malloc(sizeof(struct Database));
        if(!conn){
            die("memory error\n");
        }
    
    
        if(mode=='c'){
            database_create(conn);
            conn->file = fopen(dbname,"w");
        }else{
            conn->file = fopen(dbname,"r+");
    
    
            if(conn->file){
                database_load(conn);
            }
        }
    
    
        return conn;
    }
    
    
    void database_load(struct Connection* conn){
        if(!conn) die("a valid connection to database not found\n");
        int readcount = fread(conn->db,sizeof(struct Database),1,conn->file);
        if(readcount!=1) die("error in reading the database");
    }
    
    
    void database_close(struct Connection* conn){
        if(conn){
            if(conn->db){
                free(conn->db);
            }
            if(conn->file){
                fclose(conn->file);
            }
            free(conn);
        }
    }
    
    
    //program s 1 "john lee" johnlee@gmail.com
    void database_add(struct Connection* conn, struct Address* addr){
        if(!conn->db) die("No connection to database");
        if(addr->id > MAX_ROWS) die("id is too big. id must be less than MAX_ROWS");
        if(addr->set) die("This record is already set, delete it first");
        addr->set = 1;
        conn->db->rows[addr->id] = *addr;
    }

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    27,234
    Have you considered using an embedded database engine with a single database file like SQLite instead of programming your own? This way, you focus on what you want to store in the database rather than on the details of the CRUD operations and file format, and still end up with a single database file rather than having to mess with database system installation and complex database storage.

    Quote Originally Posted by narniat
    My guess is that the database_add function(at the bottom of the code) is not working properly(especially I'm suspicious about this line: `conn->db->rows[addr->id] = *addr;`, but compiler gives no error.
    That is okay because your struct Address does not have pointer members, so the shallow copying from assignment works (although it can be "expensive"). Of course, if it did have a pointer member, the compiler would still not complain, but you might find that you have a logic error when you are assigning the same struct to different structs, after modifying its value in between assignments, because the different structs will copy the same pointer.

    Anyway, I compiled your code, and my compiler warned that "initialization makes integer from pointer without a cast" concerning this line:
    Code:
    struct Address addr = {.id=atoi(argv[3]),.fullname=argv[4],.email=argv[5]};
    I think the issue is with trying to initialise an array of char with a pointer to char. Using the traditional approach eliminates this warning:
    Code:
    struct Address addr;
    addr.id = atoi(argv[3]);
    addr.set = 0;
    strcpy(addr.fullname, argv[4]);
    strcpy(addr.email, argv[5]);
    You might find it convenient to provide an Address_init function that wraps these statements together, e.g.,
    Code:
    struct Address addr;
    Address_init(&addr, atoi(argv[3], 0, argv[4], argv[5]);
    As far as I can tell, your program does work, at least for the test case you provided. Note that my fix is vulnerable to buffer overflow because you didn't check that argv[4] and argv[5] are not too large for the respective char arrays. Also, you forgot to check that the id is non-negative.
    Last edited by laserlight; 02-22-2019 at 10:46 PM.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  3. #3
    Registered User
    Join Date
    Oct 2018
    Posts
    42
    Quote Originally Posted by laserlight View Post
    Have you considered using an embedded database engine with a single database file like SQLite instead of programming your own?
    I'm somewhat new to C so I'm gaining experience and this is for practicing. thank you for the suggestion.




    Of course, if it did have a pointer member, the compiler would still not complain, but you might find that you have a logic error when you are assigning the same struct to different structs, after modifying its value in between assignments, because the different structs will copy the same pointer.
    I had already read about shallow vs deep copying but I had never "gotten" it like this, now I can clearly see where it can be so serious and important.


    I think the issue is with trying to initialise an array of char with a pointer to char. Using the traditional approach eliminates this warning:
    Code:
    struct Address addr;
    addr.id = atoi(argv[3]);
    addr.set = 0;
    strcpy(addr.fullname, argv[4]);
    strcpy(addr.email, argv[5]);
    Yes the problem is here, argv is local to main function so when assigning it to "addr" it's dangling reference (I'm not 100% sure if this term refers to the same idea). I should stop assigning pointers to anything! I should add it to my checklist while reviewing my codes for any error!

    Now it's working, laserlight. You're so helpful! Thank you for your time and patience.

  4. #4
    Registered User
    Join Date
    Oct 2018
    Posts
    42
    I was wrong, indeed this wasn't the problem:
    Quote Originally Posted by narniat View Post
    Yes the problem is here, argv is local to main function
    the problem was assigning a pointer string to an array string. (e.g.: char fullname[10] = "hello"; is only allowed if it's an initialization. but char fullname[10]; fullname = "hello"; is wrong, so we need the strcpy function).

    (Just saying for others because the view number of the post has increased and I don't want my previous comment to mislead anyone. argv is fine, since it's on the stack of the main function, so doesn't get destroyed.)

  5. #5
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    27,234
    No, "hello" is an array of 6 char, not a pointer. Your example isn't allowed because assignment to an array itself is not allowed.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 9
    Last Post: 03-15-2016, 07:09 AM
  2. Problem with added header file in code::blocks
    By patishi in forum C Programming
    Replies: 11
    Last Post: 08-27-2013, 07:35 PM
  3. Replies: 4
    Last Post: 08-25-2013, 05:09 AM
  4. Replies: 1
    Last Post: 12-06-2012, 02:46 PM
  5. Update Record & Delete Record in File.
    By unsafe_pilot1 in forum C Programming
    Replies: 13
    Last Post: 05-18-2008, 07:22 AM

Tags for this Thread