CODEWITHSUNDEEP
c++visual-c++
CXO2
CXO2

Reputation: 648

C++ Parsing data bytes into struct

I am new in here and I have a question I have a struct, let's say overall size is 8 bytes, here the struct:

struct Header 
{
    int ID;             // 4 bytes
    char Title [4];     // 4 bytes too
}; // so it 8 bytes right?

and I have a file with 8 bytes too... I just want to ask, how to parse data on that file into the struct of that

I have tried this one:

    Header* ParseHeader(char* filename)
    {
        char* buffer = new char[8];
        fstream fs(filename);

        if (fs.is_open() != true)
            throw new exception("Couldn't Open file for Parsing Header.");

        fs.read(buffer, 8);

        if (!fs)
        {
            delete[] buffer;
            throw new exception("Couldn't Read header OJN file.\nHeader data was corrupted");
        }

        Header* header = (Header*)((void*)buffer);
        delete[] buffer;
        fs.close();

        return header;
    }

but it fail, and return invalid data than what I was expect (I can make you sure, this is not file fault, the file structured correctly)

Can someone help me? Thanks

Upvotes: 0

Views: 2021

Answers (4)

Ajay
Ajay

Reputation: 18411

Just define your structure in 1-byte boundary as:

#pragma pack(1)
struct Header 
{
    int ID;             // 4 bytes
    char Title [4];     // 4 bytes too
};
#pragma pack()

First pack statement instructs the compiler to use one-byte padding for members in structure. This way size of Header will be 8 bytes. Second pack statement instructs to go back to default setting. You may need to use push and pop instructions (See enter link description here) - but this isn't required for you.

Secondly, and more importantly, you should not use hard-code values like 8. Always use sizeof to read or write a structure. Also, this statement is absolutely not needed:

char* buffer = new char[8];
...

Just declare Header variable itself, and read on it:

Header header;
...
fs.read(&header, sizeof(Header));

Upvotes: 0

syam
syam

Reputation: 15069

The fact that your 8 bytes file correctly maps to your struct Header is mere luck as far as C++ is involved. The structure could have internal padding that make it bigger than 8 bytes, and the data endianness could be different between your file and your CPU.

I realize your code works with your particular compiler version, on your operating system and on your CPU but you should not get into the habit of coding like that, otherwise you'll probably get into big trouble as soon as you change any of those parameters (or maybe even just some compiler flags). In other words, what you are doing is extremely bad practice. In C++ you don't even have the guarantee that an int is actually 4 bytes.

The Right Way™ to load such binary data from a file is to load each field individually and ensure proper endianness conversion depending on the CPU you're using (eg. through hton* / ntoh* or similar functions). Using a fixed-size type like int32_t also helps.

Upvotes: 0

Kyurem
Kyurem

Reputation: 1869

You are deleting the data that is being returned. Therefore header is no longer accessible.

I think you meant the line to be:

Header header = *(Header*)((void*)buffer);

This will actually copy the header.

Upvotes: 0

Alon
Alon

Reputation: 1804

Seems like you do everything fine until this point:

Header* header = (Header*)((void*)buffer);
delete[] buffer;
fs.close();

notice you delete the buffer after the casting, meaning that header points to a deleted location -> junk, you need to either not delete or copy the data if you like to still use it.

Also, to be quite honest, I don't understand how your code compiles, your function states it returns a Header, while you return a Header*..

Upvotes: 1

Related Questions