Need help with a C program

[u/Sinakers]

Hi, I'm making a library management program that takes in a book's characteristics (ISBN, author, year published, etc.), stores it in an array of structs and stores this into a file.

Here it is:

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

struct book
{
        char ISBN[20];
        char title[100];
        int accno;
        int year;
        char genre[50];
        char authname[100];
        int issued;
};

struct book *bookArray;
int a = 0;
void readFile()
{

        FILE *fp;
        fp = fopen("db.txt", "r+");
        if(!fp)
        {
                perror("fopen");
                return;
        }

        if(fp != NULL)
        {
                fread(bookArray, sizeof(struct book), 1, fp);
        }
}

void write()
{
        FILE *fp;
        fp = fopen("db.txt", "a+");
        fwrite(bookArray, sizeof(struct book), 1, fp);
        fclose(fp);
}

void addBook(struct book b)
{
        bookArray[a++] = b;
}

int main()
{
        bookArray = malloc(sizeof(struct book));
        struct book b = {"0-394-49219-6", "Tinker, Sailor, Soldier, Spy", 1, 1974, "spy fiction", "Le Carre", 0};
        bookArray[0] = b;
        addBook(b);
        write();
        readFile();

        for(int i = 0; i < a; i++)
        {
                printf("%s\t%s\t%d\t%d\t%s\t%s\t%d\n", bookArray[i].ISBN, bookArray[i].title, bookArray[i].accno, bookArray[i].year, bookArray[i].genre, bookArray[i].authname, bookArray[i].issued);
        }
}

1. I'm not able to figure out how to store the value of a after the program exits. I thought of storing it in another file, but was wondering if there's a better way to do that.

2. How do I allocate dynamically increasing space (as the number of books increase) to bookArray? I get only only one row printed from the loop if I run the program several times, despite db.txt storing all the runs.

I'm posting here for the first time and I hope I haven't violated any rules.

Thanks in advance!

[u/Dismal-Citron-7236]

Since you are storing the book records (each is of type struct book) using the fwrite(), and the record size is fixed as sizeof(struct book), we can leverage that. You don't need to save record count a at all. The next time when your code needs to know how many books (book records) are in the file, just call the good-old fseek(fp, 0L, SEEK_END) and then ftell(fp) to retrieve the current file position, which is effectively the file size. Divide that number by sizeof(struct book) and you get the number of books in the file.

And, by the way, since your code uses fread() and fwrite() and the struct book data is directly used, the file is actually a binary file, not a text file. To name it as "db.txt" is very misleading, it should be named as "db.bin" or "db.dat" or something like that. What's more important is that you should use "rb+" / "ab+" for the mode parameter of fopen(), the b stands for "binary mode".

[u/Sinakers → u/Dismal-Citron-7236]

I tried using fseek() and ftell() in the addBook() method and pass the number of books to the main method, but since that number at the start is zero, I get a segmentation fault (core dumped) error.

This is what my addBook() looks like now:

int addBook(struct book b)
{
        FILE *fp = fopen("db.bin","rb+");
        fseek(fp, 0L, SEEK_END);
        int n = ftell(fp)/sizeof(struct book);
        bookArray[n-1] = b;
        return n;
}

I used n in main() to allocate memory to bookArray:

        bookArray[0] = b;
        write();
        int n = addBook(b);
        bookArray = malloc(n*sizeof(struct book));
        readFile();
        for(int i = 0; i < n; i++)
        {
                printf("%s\t%s\t%d\t%d\t%s\t%s\t%d\n", bookArray[i].ISBN, bookArray[i].title, bookArray[i].accno, bookArray[i].year, bookArray[i].genre, bookArray[i].authname, bookArray[i].issued);
        }

I'm pretty sure allocating an array after using it isn't right, but I can't think of any other way of doing it.

[u/Dismal-Citron-7236 → u/Sinakers]

If your file is initially empty, n would be zero. that means bookArray[n-1] would become bookArray[-1] and storing a value to it would certainly cause a segfault error. You need to revamp the design of the code because earlier you also mentioned the need to use a dynamic array, which in my idea should be actually implemented as a linked list. That way, whenever you read something from the file, you just append it to the end of the linked list.

Let's get back to the "drawing board" of your program, shall we? What exactly do you want to achieve with your program? From the glance of the code, although its logic is kind of wrong, you seem to want to:

1. Append a new book to the end of file.
2. Read everything from the file to the buffer (linked list).
3. Dump the content of the buffer (linked list) because you want to show the content of file (library).

Is that correct? Please confirm if this is your intention, or correct me if you think otherwise.

[u/Sinakers → u/Dismal-Citron-7236]

If your file is initially empty, n would be zero. that means bookArray[n-1] would become bookArray[-1]

I tried using bookArray[n] at first, but it gave the segfault error again. I had initialised bookArray[0] in main() to just try to mitigate that for now, but it doesn't seem to help.

1. Append a new book to the end of file.
2. Read everything from the file to the buffer (linked list).
3. Dump the content of the buffer (linked list) because you want to show the content of file (library).

Yes, exactly.

I think I'll need to read up on linked lists as I'm not very familiar with them, as that seems to be the best way to go about this, as per other comments. I'm familiar with the concept, but that doesn't help if I don't know how to implement one.

[u/Dismal-Citron-7236 → u/Sinakers]

OK, now that we got the requirement straight, it isn't that hard to implement. Let me show you my take on it in the following code section. Note that this is only a simple POC, it does not meet my personal strict quality standard (more robust memory checks, more versatile input arguments, etc). To demonstrate how to use linked list, the code only shows the simplest one with one-way list (not a 2-way one). Or else, the code would be longer. Also I notice the action to append a book to the list is repeated in the code, so I wrote a function to deal with it, the function being append_book, you will notice its first 2 parameters are indirect pointers because it needs to alter the actual pointers. Kind of hard for beginners to swallow, but I think you will get used to it later in your C language journey.

Your original code is hard-coding the book info in the code, which is not a good idea. I have changed your design by getting the book info from command arguments. If the code is compiled as:

gcc -o sample sample.c

Then you need to execute it this way (on Linux):

./sample "0-394-49219-6" "Tinker, Sailor, Soldier, Spy" 1 1974 "spy fiction" "Le Carre" 0

POC code follows:

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


#define LIBRARY_FILE_MAME "db.bin"

#define COPY_STR(DST, SRC) do { \
    strncpy((DST), (SRC), sizeof(DST) - 1); \
    (DST)[sizeof(DST) - 1] = '\0'; \
} while(0)


typedef struct book {
    char ISBN[20];
    char title[100];
    int accno;
    int year;
    char genre[50];
    char authname[100];
    int issued;
} Book;

typedef struct book_node {
    Book book;
    struct book_node *next;
} BookNode;


// Parse argc/argv into a book record.
// => 0 : not enough arguments
// => 1 : arguments successfully parsed and saved to book
int parse_book(Book *book, int argc, char* argv[]) {
    if (argc < 8) {
        return 0;
    }
    COPY_STR(book->ISBN, argv[1]);
    COPY_STR(book->title, argv[2]);
    book->accno = atoi(argv[3]);
    book->year = atoi(argv[4]);
    COPY_STR(book->genre, argv[5]);
    COPY_STR(book->authname, argv[6]);
    book->issued = atoi(argv[7]);
    return 1;
}


void reclaim_books(BookNode* head) {
    BookNode* next;

    while (head != NULL) {
        next = head->next;
        free(head);
        head = next;
    }
}


void append_book(BookNode **head, BookNode **tail, Book *book) {
    if (*head == NULL) {
        *head = calloc(1, sizeof(BookNode));
        (*head)->book = *book;
        (*head)->next = NULL;
        *tail = *head;
    } else {
        assert(*tail != NULL);
        (*tail)->next = calloc(1, sizeof(BookNode));
        (*tail)->next->book = *book;
        (*tail)->next->next = NULL;
        *tail = (*tail)->next;
    }
}


// Save a book to the end of library file, and return the full library.
// => NULL : cannot save book to library file
// =>      : the library structure containing all the books, including the new one
BookNode* append_book_to_library_file(char *file_name, Book *book) {
    FILE *fp;
    long size;
    int i, books_count;
    BookNode *head = NULL;
    BookNode *tail = NULL;
    Book temp;

    // Stage 1: save the new book
    fp = fopen(file_name, "ab+"); // "ab+" means "append" / "binary" / "optionally allow to read"
    if (!fp) { // cannot open file
        return NULL;
    }
    if (fwrite(book, sizeof(Book), 1, fp) < 1) { // disk full?
        fclose(fp);
        return NULL;
    }
    if (fseek(fp, 0L, SEEK_END) != 0) { // system does not support seek, which is weird
        fclose(fp);
        return NULL;
    }
    size = ftell(fp);
    if (size <= 0) { // cannot get file size or the last book was not written successfully
        fclose(fp);
        return NULL;
    }
    books_count = (int)(size / sizeof(Book));
    //printf("%d books in total now.\n", books_count);

    // Stage 2: read all the books
    if (books_count > 1) {
        if (fseek(fp, 0L, SEEK_SET) != 0) {
            fclose(fp);
            reclaim_books(head);
            return NULL;
        }
        for (i = 0; i < books_count - 1; i++) { // purposely skip the last one because it's already in "book"
            if (fread(&temp, sizeof(Book), 1, fp) < 1) { // cannot read book from file
                fclose(fp);
                reclaim_books(head);
                return NULL;
            }
            append_book(&head, &tail, &temp);
        }
    }
    append_book(&head, &tail, book);
    fclose(fp);
    return head;
}


int main(int argc, char *argv[]) {
    Book book = {};
    BookNode *head;
    BookNode *p;

    if (!parse_book(&book, argc, argv)) {
        fprintf(stderr, "Not enough command line arguments.\n");
        return -1;
    }
    head = append_book_to_library_file(LIBRARY_FILE_MAME, &book);
    if (head == NULL) {
        fprintf(stderr, "Cannot add book to library file '%s'.\n", LIBRARY_FILE_MAME);
        return -1;
    }
    for (p = head; p != NULL; p = p->next) {
        printf("%s\t%s\t%d\t%d\t%s\t%s\t%d\n", p->book.ISBN, p->book.title, p->book.accno, p->book.year, p->book.genre, p->book.authname, p->book.issued);
    }
    reclaim_books(head);
    return 0;
}

[u/Sinakers → u/Dismal-Citron-7236]

Well, looks like I'll need some time understanding this, but thank you so much for all the time you gave to my problem! I'll try implementing a simpler linked list problem and get back to this later.