CODEWITHSUNDEEP
c++
Panagiotis
Panagiotis

Reputation: 27

Passing pointer array to function

i have the following situation. the following program although it compiles just fine when i run it, it stops working. can anyone please help me to find the problem? i think i'm using the wrong pointer into the function but i don't know how to fix it and make it work

#include <fstream>
//some other includes
using namespace std;

struct Book{
    string id;
    string title;
    string authorName;
    string authorSurname;
    };

int load(Book* booksInfo)
{
int count = 0;
ifstream fin;
fin.open("myfile.txt");

if (!fin.is_open())
{
    cout << "Unable to open myfile.txt file\n";
    exit(1);
}

while (fin.good())
{   
    getline(fin, booksInfo[count].id, '#'); 
    getline(fin, booksInfo[count].title, '#'); 
    getline(fin, booksInfo[count].authorName, '#'); 
    getline(fin, booksInfo[count].authorSurname, '#'); 

    count++;
} //end while

fin.close(); 

return 0;
} //end load()

//some other functions here

int main()
{
Book * bookInfo;
bookInfo = (Book*) malloc(sizeof(Book)*100);

//some code here

load(bookInfo);

    //some code here

return 0;
} //end main

Upvotes: 0

Views: 143

Answers (4)

alexrider
alexrider

Reputation: 4463

It is UB to use malloc to allocate non POD types, in your case book instances will contain some garbage in strings, because there was no std::string constructor called. And it won't be just garbage strings, it will be most likely garbage pointer pointing to some random locations.
You should use std::vector or at least new if you really need to allocate memory manually, to create new Book instances in a heap.
And if you really, really must use malloc, you can use placement new to create valid std::strings in raw memory you have allocated somehow (by malloc in your case).

Upvotes: 3

gled
gled

Reputation: 121

What about:

Book bookInfo[100];

This avoids heap allocation altogether and should serve your purposes.

Upvotes: 0

trojanfoe
trojanfoe

Reputation: 122381

Use std::vector to store your list of books:

#include <fstream>
#include <vector>
//some other includes
using namespace std;

struct Book{
    string id;
    string title;
    string authorName;
    string authorSurname;
    };

vector<Book> load()
{
    ifstream fin;
    Book book;
    vector<Book> books;
    fin.open("myfile.txt");

    if (!fin.is_open())
    {
        cout << "Unable to open myfile.txt file\n";
        return books;
    }

    while (fin.good())
    {   
        getline(fin, book.id, '#'); 
        getline(fin, book.title, '#'); 
        getline(fin, book.authorName, '#'); 
        getline(fin, book.authorSurname, '#'); 
        books.push_back(book);
    } //end while

    fin.close(); 

    return books;
} //end load()

//some other functions here

int main()
{
    vector<Book> books = load();
    return 0;
} //end main

Upvotes: 3

Bathsheba
Bathsheba

Reputation: 234635

You need to use 

Book* bookInfo = new Book[100];

instead. This is because, in C++, a struct is an object (just like a class), and calling malloc on anything other than plain old data is undefined behaviour.

Remember to free your memory using delete[] bookInfo; (note carefully the square brackets). If you use delete on it's own, that's a little more undefined behaviour.

Also make sure that you don't read more than 100 lines; or you'll overflow the array: yet more undefined behaviour.

Finally, consider using a standard template library container like std::vector.

Upvotes: 1

Related Questions