Search code examples
cpointersstructstrcpy

(C) Getting segmentation fault on pointer strcpy


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!


Solution

  • 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 ith 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.