user2349812
user2349812

Reputation: 1

Access violation error when getting input from a binary file

Okay, so I'm trying to read input from a binary file. I've changed this code a bit, but with this version, I'm getting an access violation error... So it's trying to access something that isn't there. Here's my source code for the problem area:

void HashFile::fileDump (ostream &log)
{
    HashNode *temp = new HashNode;

    fstream bin_file;
    bin_file.open ("storage_file.bin", ios::in | ios::binary);  

    for(int i = 0; i < table_size; i++)
    {
        bin_file.seekg( i * sizeof(HashNode) );

        bin_file.read( (char *)&temp, sizeof(HashNode) );

        printDump(HashNode(temp->title, temp->artist, temp->type, temp->year,
        temp->price), log, i);
    }

    bin_file.close();
}

void HashFile::printDump(HashNode A, ostream &log, int N)
{
    log << "(" << N << ") " << A.title << ", " << A.artist
        << ", " << A.type << ", " << A.year << ", $"
        << setprecision(2) << A.price << endl;
}

I know that I should have some kind of error checking. Right now the error is occurring in the printDump function. Whenever I try to output to the log I get an access violation error. However, I change the log to cout and my code will run fine somewhat. It will read the binary file I've created correctly until it gets to the last element. For what I've been testing with, table_size should be equal to 5. So I get into the for loop and i is incremented until it reaches 5 and then it keeps going. table_size is being changed to some random value even though I haven't physically touched it. Am I somehow writing over table_size's address in memory?

Here is the definition of my Node:

class HashNode
{
    public:
        HashNode();
        ~HashNode();
        HashNode(string title, string artist, string type, int year, float price);
        friend class HashFile;
    private:
        char title [35];
        char artist [25];
        char type [12];
        int year;
        float price;
};

Upvotes: 0

Views: 450

Answers (1)

john
john

Reputation: 87959

This

bin_file.read( (char *)&temp, sizeof(HashNode) );

should be this

bin_file.read( (char *)temp, sizeof(HashNode) );

You are getting confused over pointers.

Whether that code will actually work depends strongly on the definition of Node which you haven't shown.

Also the code leaks memory as temp is never deleted. It would be better not to allocate temp at all, like this

void HashFile::fileDump (ostream &log)
{
    HashNode temp;

    fstream bin_file("storage_file.bin", ios::in | ios::binary);  

    for(int i = 0; i < table_size; i++)
    {
        bin_file.seekg( i * sizeof(HashNode) );

        bin_file.read( (char *)&temp, sizeof(HashNode) );

        printDump(HashNode(temp.title, temp.artist, temp.type, temp.year, temp.price), log, i);
    }
}

Not clear why you feel the need to create a new node from temp, why not just pass temp to printDump? Like this

        printDump(temp, log, i);

But without seeing the definition of Node I can't say for sure.

Also no need to close the file, that happens automatically, also opening the file in the constructor is a little cleaner IMHO.

EDIT

OK having seen the definition of Node this would be my recommendation

void HashFile::fileDump(ostream &log)
{
    fstream bin_file("storage_file.bin", ios::in | ios::binary);  
    for(int i = 0; i < table_size; i++)
    {
        bin_file.seekg(i * sizeof(HashNode));    
        HashNode temp;
        bin_file.read((char *)&temp, sizeof(HashNode));
        printDump(temp, log, i);
    }
}

Also I would change printDump to use a const reference, this avoids copying a Node object (it is quite big).

void HashFile::printDump(const HashNode& A, ostream &log, int N)
{
    log << "(" << N << ") " << A.title << ", " << A.artist
        << ", " << A.type << ", " << A.year << ", $"
        << setprecision(2) << A.price << endl;
}

Upvotes: 0

Related Questions