strcpy assertion error c++

Question

I am a student Just learning c++ so I am sure there are much more efficient ways of doing this; with that said I would really appreciate some help figuring out why my program crashes. I have narrowed it down the a strcpy function that crashes everything and breaks it, which I have commented out and labeled. I have obviously used the strcpy function multiple times in the program with similar parameters, so I don't understand why that specific one crashes. I have tried everything I can think of and really appreciate the help. As of now I have a lot commented out so it should run with the right text file named "bookdb" my text file currently has this in it

Active Learning Approach,Randal Albert,9780763757236,1,650,1,<br>
Technical Communications,John Lannon,9780321899972,2,724,0,

to see the error you will have to un-comment strcpy(bookArray[num_books].author_name, temp_authorName);

#define _CRT_SECURE_NO_WARNINGS
#include <iostream>
#include <iomanip>
#include <fstream>
#include <cstring>

enum Genres {HORROR=1, SCIFI, COMEDY, DRAMA, ACTION};


struct Book
{
    char title[100];
    char author_name[50];
    char isbn[14];
    Genres genre;
    int num_pages;
    bool paperback;
};

//Function Declarations
unsigned short ReadBooks(Book * bookArray, unsigned short & num_books);
void DisplayBooks(Book * bookArray, unsigned short num_books);
void ResizeArrays(Book * bookArray, unsigned short num_books);

int main ()
{
    unsigned short num_books = 0;
    Book * bookArray = new Book();

    num_books = ReadBooks(bookArray, num_books);

}//End Main

unsigned short ReadBooks(Book * bookArray, unsigned short & num_books)
{
    ifstream readBooks("bookdb.txt");
    char temp_title[100] = "0";
    char temp_authorName[100] = "0";
    char temp_isbn[14] = "0";

    char temp_genre[50] = "0";
    char temp_numPages[50] = "0";
    char temp_paperback[50] = "0";

    int genreNumber = 0,
        numPages = 0,
        paperback = 0;

    if (readBooks.is_open() )
    {
        cout << "The file was successfully opened\n" << endl;

        readBooks.getline(temp_title, 100, ',');//Reads into temp cstring
        strcpy(bookArray[num_books].title, temp_title); //copies to dynamic cstring
        //cout << bookArray[num_books].title << endl; //displays part of structure to make sure it worked!!

        readBooks.getline(temp_authorName, 100, ',');
        strcpy(bookArray[num_books].author_name, temp_authorName); 
        //cout << bookArray[num_books].author_name << endl; 

        readBooks.getline(temp_isbn, 14, ',');
        strcpy(bookArray[num_books].isbn, temp_isbn); 
        //cout << bookArray[num_books].isbn << endl;

        readBooks.getline(temp_genre, 50, ',');//Get the genre as a char
        genreNumber = atoi(temp_genre);//converts char to an int
        bookArray[num_books].genre = static_cast <Genres> (genreNumber);//converts int to ENUM
        //cout << bookArray[num_books].genre << endl;//Displays ENUM to make sure it worked!!

        readBooks.getline(temp_numPages, 50, ',');
        numPages = atoi(temp_numPages); //converts char to an int
        bookArray[num_books].num_pages = numPages; //assigns int to structure
        //cout << bookArray[num_books].num_pages << endl; //Displays part of structure to make sure to works!!

        readBooks.getline(temp_paperback, 50, ',');
        paperback = atoi(temp_paperback); //converts char to an int
        bookArray[num_books].paperback = static_cast <bool> (paperback);
        //cout << bookArray[num_books].paperback << endl;
        num_books++;

        //DisplayBooks(bookArray, num_books);
        ResizeArrays(bookArray, num_books);
        cout << "The number of books is: " << num_books << endl;

        //while (!readBooks.eof() )
        //{

        readBooks.getline(temp_title, 100, ',');//Reads into temp cstring
        strcpy(bookArray[num_books].title, temp_title); //copies to dynamic cstring
        cout << bookArray[num_books].title << endl; //displays part of structure to make sure it worked!!

        readBooks.getline(temp_authorName, 100, ',');
        cout << temp_authorName << endl; 
        //strcpy(bookArray[num_books].author_name, "0");
        ///THIS BREAKS MY CODE////strcpy(bookArray[num_books].author_name, temp_authorName); 
        //cout << bookArray[num_books].author_name << endl; 

        readBooks.getline(temp_isbn, 14, ',');
        //strcpy(bookArray[num_books].isbn, temp_isbn); 
        //cout << bookArray[num_books].isbn << endl;

        readBooks.getline(temp_genre, 50, ',');//Get the genre as a char
        //genreNumber = atoi(temp_genre);//converts char to an int
        //bookArray[num_books].genre = static_cast <Genres> (genreNumber);//converts int to ENUM
        //cout << bookArray[num_books].genre << endl;//Displays ENUM to make sure it worked!!

        readBooks.getline(temp_numPages, 1000, ',');
        //numPages = atoi(temp_numPages); //converts char to an int
        //bookArray[num_books].num_pages = numPages; //assigns int to structure
        //cout << bookArray[num_books].num_pages << endl; //Displays part of structure to make sure to works!!

        readBooks.getline(temp_paperback, 50, ',');
        //paperback = atoi(temp_paperback); //converts char to an int
        //bookArray[num_books].paperback = static_cast <bool> (paperback);
        //cout << bookArray[num_books].paperback << endl;*/

        num_books++;

        //ResizeArrays(bookArray, num_books);
        //}//End while

        readBooks.close();
    }//End if
    else
    {
        cout << "There was not an existing book file, so one will be created"  << endl;
    }//End else

    return 0;
}//End ReadBooks

void DisplayBooks(Book * bookArray, unsigned short num_books)
{
    for (unsigned short i = 0; i < num_books; i++)
    {
        cout << setw(30) << left << bookArray[i].title << left << setw(20) << bookArray[i].author_name << left << setw(15) << bookArray[i].isbn
             << left << setw(3) << bookArray[i].genre  << left<< setw(6) << bookArray[i].num_pages << left << setw(4) << bookArray[i].paperback << endl;
    }//End For
}//ENd Display Function

void ResizeArrays(Book * bookArray, unsigned short num_books)
{
    Book * temp_bookArray = new Book[num_books + 1];


    for (int i = 0; i < num_books; i++)
    {
        strcpy(temp_bookArray[i].title, bookArray[i].title);
        //cout << temp_bookArray[i].title << endl; //For Debugging

        strcpy(temp_bookArray[i].author_name, bookArray[i].author_name);
        //cout << temp_bookArray[i].author_name << endl; //for Debugging

        strcpy(temp_bookArray[i].isbn, bookArray[i].isbn);
        //cout << temp_bookArray[i].isbn << endl;//for debugging

        temp_bookArray[i].genre = bookArray[i].genre;
        //cout << temp_bookArray[i].genre << endl;//for debugging

        temp_bookArray[i].num_pages = bookArray[i].num_pages;
        //cout << temp_bookArray[i].num_pages << endl;// for debugging

        temp_bookArray[i].paperback = bookArray[i].paperback; 
        //cout << temp_bookArray[i].paperback << endl; //for debugging


    }//End for

    delete [] bookArray;

    bookArray = temp_bookArray; 

    DisplayBooks(bookArray, num_books); //debugging to make sure bookArray is reassigned


}//End Resize Function

Answer 1

strlen gets all elements without '\0',so if you have str = "1234"; the str length is 5,(1234 + '\0) but return 4.Strcpy take all 5 elements + '\0' .

Answer 2

There are problems with the ResizeArrays:

• It allocates new memory for the array, but doesn't return that new array back to the calling function, so that it can adjust its pointers to the new array location.
• Resize probably needs to sizes, the old size, and the new size, and you will only be able to copy the minimum of those numbers of elements.

Just to fix this to the point where it will work, you can do something like:

Book* ResizeArrays(Book * bookArray, unsigned short num_books)
{ 
    ....
    return bookArray;
}

Then use the function in this way

bookArray = ResizeArrays(bookArray, num_books);

However, as you use an array delete, for your first allocate, you need to use an array allocation in main() as well,

Book * bookArray = new Book[1];

Even though its only allocating one book, it should be done as an array, so it is compatible with the delete [] used in ResizeArrays.

I assume this is a learning exercise. In normal use of C++ you wouldn't use naked new and delete operators. Either you would simply use std::vector or similar, or to create an object that will manage the memory, with the new and delete in the constructor and destructor respectively.

Answer 3

Problems that I see:

Problem 1

There isn't enough space to hold the ISBN properly. The length of ISBN in your input file is 14 characters long. To hold them in a null terminated string, you need to have an array that can hold 15 or more characters.

As a result, the line

    readBooks.getline(temp_isbn, 14, ',');

will stop reading after 13 characters since it has to use the 14-th character to store '\0'.

Problem 2

This line doesn't look right.

    readBooks.getline(temp_numPages, 1000, ',');

I suspect this was a typo and you meant

    readBooks.getline(temp_numPages, 50, ',');

Problem 3

There are problems with handling of memory in ResizeArray.

void ResizeArrays(Book * bookArray, unsigned short num_books)
{
    Book * temp_bookArray = new Book[num_books + 1];

    // Here, you are deleting the memory that was allocated in `main`.
    // bookArray in the calling function now points to memory that has been
    // deleted here.
    delete [] bookArray;

    // Here, you are changing the value of bookArray but only
    // locally in this function. This assignment doesn't
    // change the value of bookArray in the calling function.
    bookArray = temp_bookArray; 

    DisplayBooks(bookArray, num_books); //debugging to make sure bookArray is reassigned


}//End Resize Function

If you change ResizeArray to:

void ResizeArrays(Book*& bookArray, unsigned short num_books)

things will be better.

Problem 4

I see that you have commented out the while loop in ReadBooks. If you decide to bring that loop back to life, you'll have to resize bookArray in each run of the loop.

Suggestion It will be easier to use an std::vector<Book*> to hold the books instead of allocating memory for arrays of Books.