Segmentation fault for all elements except the first

Question

so I've got a struct called 'library' that stores objects of the struct 'books', and is initialized by a list of 3 books, but when I try to print the object's attributes I get a "Segmentation fault (core dumped)" error. I understand that it means I'm trying to access some memory I don't have access to, but in this case I can access the first element correctly, so it makes me believe I initialized something incorrectly.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define MAXBOOKS 10

typedef struct books
{
    char* title;
    char* author;
    int id;
}book;

typedef struct library
{
    int number_of_books;
    book* booklist [MAXBOOKS];
}lib;

void storeBook(lib library,book CurrentBook)
{
    library.booklist[0] = &CurrentBook;
}

void printLibrary(lib library)
{
    for(int i = 0; i < library.number_of_books; i++)
    {
        printf("Author:%s\n",library.booklist[i]->title);
    }
}


int main()
{

    book b_1 = {"The trial","Kafka",101};
    book b_2 = {"The lurking fear","Lovecraft",102};
    book b_3 = {"Dora's storytime collection","Valdes",103};

    book* list = (book*)malloc(3*sizeof(book));
    list[0] = b_1; list[1] = b_2; list[2] = b_3;

    lib CurrentLibrary = {3,{list}};
    printLibrary(CurrentLibrary);
    return 0;
}

Answer 1

booklist1[i] = *(booklist1+i),thenbooklist2[i][j] = *(*(booklist2+i)+j), if j=0, then *(*(booklist2+i)+j) = *(*(booklist2+i)+0) = *(booklist2[i]) = *booklist2[i]

booklist2[0] points to first row, booklist2[1] points to seconds row, and so,... on.

You are defining an array of book pointers (2D array) : book* booklist [MAXBOOKS]
But list is an array of book (1-D array). After execution this statement, lib CurrentLibrary = {3,{list}}; list array will be stored into booklist[0] row. But all other pointers of booklist[1], booklist[2],..... booklist[9] are not pointing to any element. But, you are accessing booklist[1], booklist[2], and booklist[3] in the printLibrary function. This is the reason for Segmentation fault.

For more insight (for 2-D array), please print the following lines:

printf("Title %s\n", library.booklist[0][0].title); prints--> Title The trial printf("Title %s\n", library.booklist[0][1].title); prints--> Title The lurking fear
printf("Title %s\n", library.booklist[0][2].title);prints-->Title Dora's storytime collection

But trying to access, library.booklist[1][0].title will throw segmentation fault, since second row pointer is not pointing to any element.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define MAXBOOKS 10

typedef struct books
{
    char* title;
    char* author;
    int id;
}book;

typedef struct library
{
    int number_of_books;
    book *booklist;  // It should be 1-D array, since you are passing 1-D array of book
}lib;

void storeBook(lib library,book CurrentBook)
{
    library.booklist[0] = CurrentBook;
}

void printLibrary(lib library)
{
    for(int i = 0; i < library.number_of_books; i++)
    {
        printf("Title:%s\n",library.booklist[i].title);
        printf("Author:%s\n",library.booklist[i].author);
        printf("Book ID:%d\n",library.booklist[i].id);
    }
}


int main()
{

    book b_1 = {"The trial","Kafka",101};
    book b_2 = {"The lurking fear","Lovecraft",102};
    book b_3 = {"Dora's storytime collection","Valdes",103};

    book* list = malloc(3*sizeof(book));
    list[0] = b_1; list[1] = b_2; list[2] = b_3;

    lib CurrentLibrary = {3,list}; // list is 1-D array of book
    printLibrary(CurrentLibrary);
    return 0;
}

The output :

Title:The trial
Author:Kafka
Book ID:101
Title:The lurking fear
Author:Lovecraft
Book ID:102
Title:Dora's storytime collection
Author:Valdes
Book ID:103

Answer 2

Well, the first thing to say is that I have tried to make your code work by making the minimum set of changes, and pointing the places in which you make wrong (or ununderstandable) things. First, you have already booked space on your program for your books, as you have created and initialized three instances of the type struct book in main, so you can assign their references to your lib object without the need to allocate space for them, nor for the array, which is already allocated in the lib structure, so when you create the lib object in main, you can initialize the array also with just the addresses of these instances, in this way:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define MAXBOOKS 10

typedef struct books
{
    char* title;
    char* author;
    int id;
}book;

typedef struct library
{
    int number_of_books;
    book* booklist [MAXBOOKS];
}lib;

void storeBook(lib library,book CurrentBook)
{
    library.booklist[0] = &CurrentBook;
}

void printLibrary(lib library)
{
    for(int i = 0; i < library.number_of_books; i++)
    {
        printf("Author:%s\n",library.booklist[i]->title);
    }
}


int main()
{

    book b_1 = {"The trial","Kafka",101};
    book b_2 = {"The lurking fear","Lovecraft",102};
    book b_3 = {"Dora's storytime collection","Valdes",103};

    /* book* list = (book*)malloc(3*sizeof(book)); // no need to call malloc, you have already reserved memory above (and intialized it) */
    
    /* just use the addresses of the books in the library array of pointers */
    lib CurrentLibrary = { 3, { &b_1, &b_2, &b_3 }};

    /* list[0] = b_1; list[1] = b_2; list[2] = b_3; */

    /* lib CurrentLibrary = {3,{list}}; */
    printLibrary(CurrentLibrary);  /* BEWARE:  you are making a copy of the library structure and passing it by value, it is cheaper to pass a reference */
    return 0;
}

The BEWARE: note makes a point to the fact that you are passing the lib structure by value (copying the lib object to the function, and using the copy in the function) It is more efficient to declare just a reference to the object in the printLibrary() function, by declaring it as:

void printLibrary(lib *library)
{
    ...

and then calling it with:

    ...
    printLibrary(&CurrentLibrary);

as this will make to copy only the reference (which is smaller than the whole thing you are passing, and that must be copied)

If you want to do all of this using dynamically allocated memory, calling malloc, then you have better to use this approach:

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

#define MAXBOOKS 10

typedef struct books {
    char* title;
    char* author;
    int id;
} book;

typedef struct library {
    int number_of_books;
    book* booklist [MAXBOOKS];
} lib;

void panic(char *str)
{
    fprintf(stderr, "PANIC: %s\n", str);
    exit(EXIT_FAILURE);
}

book *createBook(char *title, char *author, int id)
{
    /* don't cast the value returned by malloc() */
    /* first create the structure */
    book *ret_val = malloc(sizeof *ret_val);
    if (ret_val == NULL) {
        panic("couldn't allocate memory for book");
    }
    /* strdup makes a dynamic memory copy of the string
     * you passed as parameter */
    /* allocate memory and copy the string title to it */
    ret_val->title  = strdup(title);
    if (ret_val->title == NULL) {
        panic("couldn't allocate memory for book's title");
    }
    /* allocate memory and copy the string author to it */
    ret_val->author = strdup(author);
    if (ret_val->author == NULL) {
        panic("couldn't allocate memory for book's author");
    }
    ret_val->id = id;
    return ret_val;
}

lib *createLibrary()
{
    lib *ret_val = malloc(sizeof *ret_val);
    if (ret_val == NULL) {
        panic("couldn't allocate memory for library");
    }
    /* initialize the number of books to 0 */
    ret_val->number_of_books = 0;
    return ret_val;
}

void storeBook(lib *library, book *book)
{
    /* check that we can add more books */
    if (library->number_of_books >= MAXBOOKS) {
        panic("No space left on library for another book");
    }
    /* then add it (BEWARE that, as the books are freed as part of the
     * library destruction, you have only to add books created by
     * createBook() */
    library->booklist[library->number_of_books++] = book;
}

void printLibrary(lib *library)
{
    for(int i = 0; i < library->number_of_books; i++)
    {
        /* we are using this reference three times below, so we save it
         * to facilitate things and calculations. */
        book *b = library->booklist[i];

        /* separate the books with an empty line after the first. */
        if (i > 0) printf("\n");
        printf("Id:     %d\n", b->id);
        printf("Title:  %s\n", b->title);
        printf("Author: %s\n", b->author);
    }
}

void freeBook(book *b)
{
    /* first free the references ***INSIDE*** book */
    free(b->title);
    free(b->author);
    /* only after that, we can free() the book instance */
    free(b);
}

void freeLibrary(lib *library)
{
    /* as above, we need to first free() the references to the books,
     * calling freeBook() above, then we are able to free the library
     * reference */
    for(int i = 0; i < library->number_of_books; i++) {
        freeBook(library->booklist[i]);
    }
    free(library);
}

int main()
{

    lib *CurrentLibrary = createLibrary();

    /* create the book and store it in a shot */
    storeBook(CurrentLibrary,
            createBook("The trial", "Kafka", 101));
    storeBook(CurrentLibrary,
            createBook("The lurking fear", "Lovecraft", 102));
    storeBook(CurrentLibrary,
            createBook("Dora's storytime collection", "Valdes", 103));

    printLibrary(CurrentLibrary);

    freeLibrary(CurrentLibrary);

    return EXIT_SUCCESS;
}

Answer 3

Give this a try:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define MAXBOOKS 10

typedef struct book
{
    char *title;
    char *author;
    int id;
} Book;

typedef struct lib
{
    int length;
    Book *list[MAXBOOKS];
} Library;

void freeBook(Book *b)
{
    free(b->title);
    free(b->author);
    free(b);
}

void freeLibrary(Library *l)
{
    for (int i = 0; i < l->length; i++)
    {
        freeBook(l->list[i]);
    }
    free(l);
}

Book* newBook(char* title, char* author, int id){

    // param check 
    if( !title || !author || id<0 ){
        return NULL;
    }

    // allocate memory 
    Book* b = malloc(sizeof(Book));
    b->title = malloc(sizeof(char) * strlen(title) + 1);
    b->author = malloc(sizeof(char) * strlen(author) + 1);

    // copy data into struct
    strcpy(b->title, title);
    strcpy(b->author,author);
    b->id = id;

    // Send back new book
    return b;
}

Library *newLibrary()
{
    // allocate memory
    Library *l = malloc(sizeof(Library));
    l->length = 0;

    // return new Library
    return l;
}

void addBook(Library** library, Book* toAdd){

    // Make sure there is a book to add
    if (toAdd == NULL){
        printf("ERROR: Attempted To Add Invaild Book.\n");
    }

    // check if library is NULL
    else if (*library == NULL){
        printf("ERROR: Library is NULL.\n");
        freeBook(toAdd);
    }
    // Check if library is full
    else if( (*library)->length >= 10 ){
        printf("\nERROR: Library is full. Cannot Store:\n\nTitle: %s\nAuthor: %s\nID: %d\n\n",toAdd->title,toAdd->author,toAdd->id);
        freeBook(toAdd);
    }
    else
    {
        (*library)->list[(*library)->length] = toAdd;
        (*library)->length++;
    }
}

void printLibrary(Library* library)
{
    printf("\n------LIBRARY------\n");
    for (int i = 0; i < library->length; i++)
    {
        printf("\nTitle: %s\nAuthor: %s\nID: %d\n", library->list[i]->title, library->list[i]->author, library->list[i]->id);
    }
    printf("\n\n");
}

int main()
{

    Library *library = newLibrary();

    addBook(&library, newBook("The trial", "Kafka", 101));
    addBook(&library, newBook("The lurking fear", "Lovecraft", 102));
    addBook(&library, newBook("Dora's storytime collection", "Valdes", 103));

    //UNCOMMENT IF WANTING TO SEE MAXED OUT LIBRARY
    /*addBook(&library, newBook("The trial", "Kafka", 101));
    addBook(&library, newBook("The lurking fear", "Lovecraft", 102));
    addBook(&library, newBook("Dora's storytime collection", "Valdes", 103));
    addBook(&library, newBook("The trial", "Kafka", 101));
    addBook(&library, newBook("The lurking fear", "Lovecraft", 102));
    addBook(&library, newBook("Dora's storytime collection", "Valdes", 103));
    addBook(&library, newBook("The trial", "Kafka", 101));*/
    
    // UNCOMMENT TO TEST OVER LIBRARY CAPACITY 
    //addBook(&library, newBook("The trial", "Kafka", 101));

    printLibrary(library);

    freeLibrary(library);

    return 0;
}

EDIT: I just seen this was answered after I posted and refreshed the page. The code is a valid solution though and manages memory properly and handles most errors that would arise handling data with the struct.