Why does this C++ code result in a segmentation fault?

Question

I keep getting a segmentation fault when the readAuthor() method is called. Does anybody know why this happens? I am supposed to use dynamic arrays, I know this would be so easy if I was using static array.

#include <iostream>
#include <string>
#include <cstring>
#include <cstdlib>

using namespace std;

/** declare arrays **/
int* isbnArr = new int[25];
char* authorArr = new char[25];
char* publisherArr = new char[25];
char* titleArr = new char[25];
int* editionArr = new int[25];
int* yearArr = new int[25];
int* pagesArr = new int[25];
float* retailPriceArr = new float[25];
float* discountedPriceArr = new float[25];
int* stockArr = new int[25];

/** function prototypes **/
int readIsbn();
char* readAuthor();
char* readPublisher();
char* readTitle();
int readEdition();
int readYear();
int readPages();
float readMsrp();
float readDiscountedPrice();
int readStockAmount();
void readonebook(int* isbn, char* author, char* title, char* publisher, int* edition,
                int* year, int* pages, float* msrp, float* discounted, int* inventory);



int main()
{


    bool stop = false;  //flag when to stop loop
    int ind = 0;        //index for current book

    while( !stop ){

        cout << "Add book: press A: ";
        cout << "another thing here ";
        char choice;
        cin >> choice;

        if( choice == 'a' || choice == 'A' ){
            readonebook(&isbnArr[ind], &authorArr[ind], &titleArr[ind], &publisherArr[ind], &editionArr[ind],
            &yearArr[ind], &pagesArr[ind], &retailPriceArr[ind], &discountedPriceArr[ind], &stockArr[ind]);
            test(&authorArr[ind]);
            ind++;
        }

    }


    return 0;
}

/** define functions **/

int readIsbn(){
    int isbn;
    cout << "ISBN: ";
    cin >> isbn;
    return isbn;
}

char* readAuthor(){
    char* author;
    cout << "Author: ";
    cin >> author;
    return author;
}

char* readPublisher(){
    char* publisher = NULL;
    cout << "Publisher: ";
    cin >> publisher;
    return publisher;
}

char* readTitle(){
    char* title = NULL;
    cout << "Title: ";
    cin >> title;
    return title;
}

int readEdition(){
    int edition;
    cout << "Edition: ";
    cin >> edition;
    return edition;
}

int readYear(){
    int year;
    cout << "Year: ";
    cin >> year;
    return year;
}

int readPages(){
    int pages;
    cout << "Pages: ";
    cin >> pages;
    return pages;
}

float readMsrp(){
    float price;
    cout << "Retail Price: ";
    cin >> price;
    return price;
}

float readDiscountedPrice(){
    float price;
    cout << "Discounted Price: ";
    cin >> price;
    return price;
}

int readStockAmount(){
    int amount;
    cout << "Stock Amount: ";
    cin >> amount;
    return amount;
}

void readonebook(int* isbn, char* author, char* title, char* publisher, int* edition,
                int* year, int* pages, float* msrp, float* discounted, int* inventory){

    *isbn = readIsbn();
    author = readAuthor();
    title = readTitle();
    publisher = readPublisher();
    *edition = readEdition();
    *year = readYear();
    *pages = readPages();
    *msrp = readMsrp();
    *discounted = readDiscountedPrice();
    *inventory = readStockAmount();
}

Answer 1

You have an unitialized pointer (author) that's why you get the GPF.

Answer 2

readAuthor, readPublisher and readTitle all attempt to read data, but don't allocate any space to hold that data -- they just allocate a pointer, and try to write to wherever the pointer happens to point. In one case (readTitle) you've initialized the pointer to NULL, which is probably the one that's producing the segmentation fault. The other two are uninitialized, so they could be writing almost anywhere -- they might produce a segmentation fault, but they might just overwrite other data -- they're much worse.

Answer 3

You are getting segmentation fault because

 char* author; // << It's not initialized
 cout << "Author: ";

The pointer to author is filled with random data. You must allocate enough memory for it before using (using new) it. And don't forget to take care of the memory cleanup. Unlike languages like C# or Java, c++ doesn't support automatic garbage collection. You must manage memory automatically. One more thing, the way you are using dynamically allocated arrays is pretty much the same as with static ones. The main idea of the dynamic memory is providing you with the mechanism of allocating the particular amount of memory, the size of which you know only during the program run (not before).

Answer 4

Everything returning pointer is going to crash. You forgot to use new everywhere. Once you use that, do not forget delete [] also afterwards on all arrays. I am sure this program is nopt going to be very scalable though and you will leave many memry leaks. Consider encapsulating all these variables in some class.

Answer 5

All of your functions that return char * need to allocate memory for the text. You declare a pointer but don't allocate memory.

char* readAuthor(){
    char* author; // This line ALLOCATES a pointer but no memory for the text.
    author = new char [MAXIMUM_AUTHOR_LENGTH];
    cout << "Author: ";
    cin >> author;
    return author;
}

I highly suggest you change all your char * to std::string so you don't have these allocation issues:

std::string readAuthor()
{
    std::string author;
    cout << "Author: ";
    cin >> author;  // You may want to use getline().
    return author;
}

Text is not handled the same as integers, floats, doubles or single characters.

Answer 6

The problem exist because you try to read into unallocated space as everyone said.
You can allocate space with new operator and clean up it with delete operator but it is a messy way. I recommend you to use string class instead of character arrays.
It is as simple as:

string author;
cin >> author;

Answer 7

One common cause is when a process try to access the memory outside its own address space .

for example in the above example of char* readAuthor() . The array is allocated in the stack of the process . So when you return this pointer and return from this method the memory location may be invalid and may point outside valid address.