(C) Getting segmentation fault on pointer strcpy

Question

I am new to C and I have been stuck on this code for the whole moring.
It compiles without a problem, but fails when executed.
If you have any idea that would help me solve this, please leave me a comment. Any comment would be greatly appreciated.

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

typedef struct phonebook {
    char name[20];
    char phoneNum[20];
} Phonebook;

int bookSize=1;

void load(Phonebook **book);
void insert(Phonebook **book);
void delete(Phonebook **book);
void search(Phonebook *book);
void print(Phonebook *book);
void save(Phonebook *book);

int main(void) {
    Phonebook *book = (Phonebook *)calloc(sizeof(Phonebook), bookSize);
    load(&book);
    int menuInput=0;
    while(menuInput != 5) {
        puts("***** MENU *****");
        puts("1. Insert");
        puts("2. Delete");
        puts("3. Search");
        puts("4. Print All");
        puts("5. Exit");
        printf(">> ");
        scanf("%d", &menuInput);

        switch(menuInput) {
            case 1 : insert(&book); break;
            case 2 : delete(&book); break;
            case 3 : search(book); break;
            case 4 : print(book); break;
            case 5 : break;
            default : puts("enter correct command"); break;
        }
    }
    save(book);
    free(book);
    puts("\nexit\n");
    return 0;
}

void load(Phonebook **book) {
    FILE *fp = fopen("phonebook.txt", "rt");
    if(fp == NULL) {
        FILE *fp = fopen("phonebook.txt", "wt");
        fclose(fp);
        puts("Welcome! It looks like you don't have an existing phonebook.");
        puts("A new phonebook has been created.\n");
        return;
    }
    else {
        char temp[20];
        int i=0;
        while(fscanf(fp, "%s", temp) != EOF) {
            strcpy(book[i]->name, temp);
            fscanf(fp, "%s", temp);
            strcpy(book[i]->phoneNum, temp);
            i++;
            bookSize++;
            *book = (Phonebook *)realloc(*book, sizeof(Phonebook) * (bookSize));
        }
        fclose(fp);
        printf("Loaded %d contacts\n", bookSize-1);
    }
}

void insert(Phonebook **book) {
    puts("\nCreate a new contact");
    getchar();
    char temp[20];
    printf("Name : ");
    fgets(temp, 20, stdin);
    //temp[strlen(temp)-1]=0;
    strcpy(book[bookSize-1]->name, temp);
    //fgets(book[bookSize-2]->name, 20, stdin);
    //book[bookSize-2]->name[strlen(book[bookSize-2]->name)-1]=0;
    printf("Phone : ");
    fgets(temp, 20, stdin);
    //temp[strlen(temp)-1]=0;
    strcpy(book[bookSize-1]->phoneNum, temp);
    //fgets(book[bookSize-2]->phoneNum, 20, stdin);
    //book[bookSize-2]->phoneNum[strlen(book[bookSize-2]->phoneNum)-1]=0;
    puts("Done!\n");
    bookSize++;
    *book = (Phonebook *)realloc(*book, sizeof(Phonebook) * bookSize);
}

void delete(Phonebook **book) {}

void search(Phonebook *book) {}

void print(Phonebook *book) {
    if(bookSize == 1) {
        puts("\nempty\n");
        return;
    }
    puts("");
    for(int i=0; i<bookSize-1; i++) {
        printf("Name : %-10s  Phone : %s\n", book[i].name, book[i].phoneNum);
    }
    puts("");
}

void save(Phonebook *book) {
    FILE *fp = fopen("phonebook.txt", "wt");
    for(int i=0; i<bookSize-1; i++) {
        fprintf(fp, "%s\n%s\n", book[i].name, book[i].phoneNum);
    }
    fclose(fp);
    printf("\nSaved %d contacts", bookSize-1);
}

Segmentation fault (core dumped)

** sorry for removing parts of the code I thought was 'irrelevant'!
I have added the whole code to the post. Thanks!

Answer 1

tl;dr: insert(&book) should just be insert(book), and define this to be the address you get from allocating memory in the heap for storing the address you get from calloc.

You define your argument for insert() as **book, and when you get a *book from your calloc() call, you reasonably "add on another *" with the address operator &. The catch is that the address of *book that you got from your calloc call is a location on the call stack of your main() function. So, when the argument of strcpy() goes to dereference this address with the array index notation, it attempts to get the value located at the pointer that's on your call stack + bookSize - 1. This is already in undefined behavior territory, since the stack isn't supposed to store memory dynamically, but you're getting the segfault because the stack is at the top of the memory layout (high address area), so adding a large enough value to the dereferenced value of book puts you in an illegal memory access zone.

Answer 2

As your other answer indicates, you are tripping over the details of the double indirection.

You are maintaining your phone book as an array of structures. In main, variable book is a pointer to the first structure in that array. The second will immediately follow it in memory, and the third will immediately follow that, etc.. That's all perfectly fine.

Both insert() and load() accept as a parameter a pointer to the pointer to the first book. This also is right and proper, because these methods reallocate the memory for the array. Reallocation is not necessarily done in place -- the new space may be in a different location than the old. The original pointer passed into realloc must be considered invalid after the call, and the return value used in its place (supposing the call succeeds). You handle this correctly, too, updating main's pointer through the pointer argument:

           *book = (Phonebook *)realloc(*book, sizeof(Phonebook) * (bookSize));

But your attempts to write phone book entries into the allocated space is incorrect. For example, in load(), this:

           strcpy(book[i]->name, temp);

tries to access the i^th Phonebook * in the array of pointers to which book points, and to write to the name member of the Phonebook to which it points. But there is only ever one Phonebook *, not an array of them. You are allocating and reallocating space for the Phonebooks to which it points.

Here's a crude diagram:

---

Actual layout:

[Phonebook **]  ----> [Phonebook *]  ----> [ Phonebook, Phonebook, Phonebook ... ]

Being accessed as if it were:

[Phonebook **]  ----> [Phonebook *, Phonebook *, Phonebook *, ...]
                           |            |            |
                           V            |            |
                      [Phonebook]       V            |
                                   [Phonebook]       V
                                                 [Phonebook]

---

Solution:

Just as you assign the allocated pointer to *book, not to book, it is *book to which you should be applying the indexing operator:

            strcpy((*book)[i].name, temp);

And since it's an array of Phonebooks, not an array of pointers to them, you use the direct member access operator (.), as shown, not the indirect access operator.

Beware, however, that you use the same name, book, in different functions to designate pointers with different degrees of indirection. Thus, whereas the above would be correct in load() and insert(), it would be wrong in main() and some of the other functions.