Sign in to follow this  

Strange Problem with file IO....

This topic is 4355 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I'm working on a FileIo library, and am currently testing it. Every file that my engine uses has a certain kind of header, and I have automated their loading:
struct StandardFileHeader
{
    char* fileType;
    int formatVersion;
            
    std::string name;
    std::string author;
    std::string description;
    std::string creationProgram;
};

(note, the formatting is kind of screwed up)

StandardFileHeader FileIoHandle::LoadStandardHeader()
        {
            StandardFileHeader header;
            int nameSize, authorSize, descSize, creatorSize;
            char* name, *author, *desc, *creator;
            
            ToBeginning();
            
            header.fileType = GetChunk<char>(8);
            header.formatVersion = GetChunk<int>();
            
            nameSize = GetChunk<int>();
            name = GetChunk<char>(nameSize);
            authorSize = GetChunk<int>();
            author = GetChunk<char>(authorSize);
            descSize = GetChunk<int>();
            desc = GetChunk<char>(descSize);
            creatorSize = GetChunk<int>();
            creator = GetChunk<char>(creatorSize);
            
            header.name = name;
            header.author = author;
            header.description = desc;
            header.creationProgram = creator;
            
            return header;
        }

void FileIoHandle::PutStandardHeader
            (const std::string& fileType, int formatVersion, const std::string& name, 
             const std::string& author, const std::string& description, 
                                            const std::string& creationProgram)
        {
            ToBeginning();
            
            PutString(fileType.c_str(), 8);
            PutChunk<int>(formatVersion);
            
            
            PutChunk<int>(name.size());
            PutString(name.c_str(), name.size());
            PutChunk<int>(author.size());
            PutString(author.c_str(), author.size());
            PutChunk<int>(description.size());
            PutString(description.c_str(), description.size());
            PutChunk<int>(creationProgram.size());
            PutString(creationProgram.c_str(), creationProgram.size());
        }

Here is the method I use to test is:
bool FileIoTestWrite()
        {
            using namespace stratus;
            using namespace base;
            
            FileIoHandle* File = new FileIoHandle();
            
            File->Open("testFile.txt");
                
            File->PutStandardHeader("testfil", 27, "Test File", 
"Grant Williams",
"This is a file for testing", 
"FileIo library");
            
            File->Close();
            
            delete File;
        }
        
        bool FileIoTestRead()
        {
            using namespace stratus;
            using namespace base;
            
            using namespace std;
            
            FileIoHandle* File = new FileIoHandle();
            
            if(!File->Open("testFile.txt"))
                return false;
                
            StandardFileHeader header;
            header = File->LoadStandardHeader();
            
            cout << File->GetFileName() << endl
                 << File->GetFileSize() << endl << endl;
            
            cout << header.fileType << endl
                 << header.formatVersion << endl
                 << header.name << endl
                 << header.author << endl
                 << header.description << endl
                 << header.creationProgram << endl;
                 
            File->Close();
            
            delete File;
        }

The output should be:
testFile.txt
127

testfil
27
Test File
Grant Williams
This is a file for testing
FileIo library
but instead I get:
testFile.txt
127

testfil
27
Test File
Grant Williams
I can't for the life of me figure out why the description and creation method don't show up. Can anyone help? I hope it's not a stupid mistake =/

Share this post


Link to post
Share on other sites
Ah, sorry about that:


template<typename T>
T FileIoHandle::GetChunk()
{
if(!SafeForIo())
throw FileIoException("Error-Fatal: File stream, unsafe for io.");

if(m_FileSize - (m_CurrentPosition + sizeof(T)) < 0)
throw FileIoException("Error-Fatal: Overflow on file read.");

T data;
m_File.read((char*)&data, sizeof(T));

m_CurrentPosition += sizeof(T);

return data;
}

template<typename T>
T* FileIoHandle::GetChunk(int size)
{
if(!SafeForIo())
throw FileIoException
("Error-Fatal: File stream, unsafe for io.");

T* data = new T[size];
m_File.read((char*)data, size);

m_CurrentPosition += size;

return data;
}

Share this post


Link to post
Share on other sites
So, you allocate a char, read 1 byte of data into it, and return it. It will be assigned to the std::string using its operator=(char) member function. Hmmm, I don't think that's really what you want. I suspect.

I hate to say it, since there may be a reason why you're writing your own file I/O, but have you considered using the standard library instead? You're already using std::string, why not just use std::fstream?

You could provide standard insertion and extraction operators.

std::ostream& operator<<(std::ostream&, const StandardFileHeader&)
std::ostream& operator>>(std::istream& StandardFileHeader&).

Implementation would be trivial.

You can also overload templated functions and use lookup to determine what to call.


template<typename T> void GetChunk(T& ival) { ...generic code... }
void GetChunk(int size, std::string& sval) { ...special-case std::string ... }

...
int len;
std::string s;

GetChunk(len);
GetChunk(len, s);


That ends up with cleaner-looking code and allows special handling for std::string. Note that if you use std::getline() you don't even need to pass in the length of the string.

Share this post


Link to post
Share on other sites
No, I'm not just loading one char into the buffer. There are two GetChunk() functions, one takes a size parameter (for arrays) and one doesn't.

The reason I don't derive from std::fstream is because, well, I just like it this way better :) (I'll probably end up doing it your way.) My IoHandle does contain fstream however:


class FileIoHandle
{
std::fstream m_File;
std::string m_FileName;

int m_FileSize;
int m_CurrentPosition;

bool safeForIo; //true if the stream is good.

public:
bool SafeForIo();

template<typename T>
T GetChunk();
//vv used when you want an array
template<typename T>
T* GetChunk(int size);

template<typename T>
void PutChunk(const T& data);

void PutString(char* data, int size);
void PutString(const char* data, int size);

bool SetCurrentPosition(int pos);
bool ToBeginning();
bool ToEnd();

StandardFileHeader LoadStandardHeader();
ReducedFileHeader LoadReducedHeader();

void PutStandardHeader (const std::string& fileType, int formatVersion,
const std::string& name,
const std::string& author,
const std::string& description,
const std::string& creationProgram);

void PutReducedHeader(const std::string& fileType, int formatVersion);

int GetCurrentPosition();
int GetFileSize();
std::string GetFileName();

bool Open(const std::string& fileName);

void Close();

FileIoHandle();
~FileIoHandle();
};







The reason I'm confused is that is handles several strings perfectly, then croaks on the last two.

PS: Does anyone know how to make the C++ standard library just create a file
instead of giving a bad stream if it could not be created? I have to create testFile.txt for it.

[Edited by - Drakkcon on January 10, 2006 7:10:00 PM]

Share this post


Link to post
Share on other sites
Verify the file contents with a hex editor; that will at least tell you whether there is a problem on the writing side (there could still be one on the reading side if you find one in writing ;) )

Share this post


Link to post
Share on other sites
Thanks, Zahlman! That was a good suggestion, I don't know why I didn't think of doing that! After looking at testFile.txt through a text editor it had all of the information. Now I just have to find out where in the file reading is screwing up.


PS: I still can't figure out how to make it to where it will make testFile.txt without me having to create a file called that. It crashes :/

going to sleep

[Edited by - Drakkcon on January 10, 2006 10:40:16 PM]

Share this post


Link to post
Share on other sites
Quote:
Original post by Drakkcon
No, I'm not just loading one char into the buffer. There are two GetChunk() functions, one takes a size parameter (for arrays) and one doesn't.

Oops, sorry, missed that. I'm not used to little scroll boxes on my screen (I have a big screen, I'm used to big windows). My apologies.

You do have a memory leak there, though. Delete what you new. In fact, if you use std::vector<char> instead of new char[], overload the function to pass in the std::string by reference, and use its range assignment operator to transfer the contents of the vector to the string, you will have elimnated lifetime concerns without introducing any more overhead. In fact, if you're using an efficient pooling allocator (like the one that comes with most standard libraries), it's more efficient to do it that way. But I digress.
Quote:

The reason I don't derive from std::fstream is because, well, I just like it this way better :) (I'll probably end up doing it your way.) My IoHandle does contain fstream however:

Don't derive from an IOStream, encapsulate it like you're doing.

Quote:

The reason I'm confused is that is handles several strings perfectly, then croaks on the last two.

Have you considered that, since you're using C-style null-terminated strings, that you have a null termination issue? Think hard about where the null at the end of the string you're reading in is coming from.

Quote:

PS: Does anyone know how to make the C++ standard library just create a file
instead of giving a bad stream if it could not be created? I have to create testFile.txt for it.


Well, the standard doesn;t actually have a lot to say other than the meaning of the ios_base::openmode flags is implementation defined.

I would suggest you start with something like

std::ofstream m_file("testFile.txt", ios_base::out|ios_base::trunc);

This works without error in my dev environment (Linux/GCC).

Share this post


Link to post
Share on other sites
Quote:

You do have a memory leak there, though. Delete what you new. In fact, if you use std::vector<char> instead of new char[], overload the function to pass in the std::string by reference, and use its range assignment operator to transfer the contents of the vector to the string, you will have elimnated lifetime concerns without introducing any more overhead. In fact, if you're using an efficient pooling allocator (like the one that comes with most standard libraries), it's more efficient to do it that way. But I digress.

Are you sure? The "newed" variable is passed to an std::string. I'm pretty sure that std::string will delete the memory in its destructor (if it doesn't please let me know). That said, I like your way of handling it a lot better. I shall probably implement it that way. Thanks!

Quote:


Have you considered that, since you're using C-style null-terminated strings, that you have a null termination issue? Think hard about where the null at the end of the string you're reading in is coming from.

You might be right. I read the data in pascal-style (a separate variable calculates the size), but there's also a null terminator in there (for easier string loading). I'm going to check.

Quote:

Well, the standard doesn;t actually have a lot to say other than the meaning of the ios_base::openmode flags is implementation defined.

I would suggest you start with something like



std::ofstream m_file("testFile.txt", ios_base::out|ios_base::trunc);


This works without error in my dev environment (Linux/GCC).


Nice, I hadn't thought of using the trunc flag. Thanks!


Thanks for all of your help. Rate++.

Share this post


Link to post
Share on other sites
Quote:
Original post by Drakkcon
Are you sure? The "newed" variable is passed to an std::string. I'm pretty sure that std::string will delete the memory in its destructor (if it doesn't please let me know).


I'm letting you know. std::string copies the contents of the buffer you offer to its operator=(const char*) function. Otherwise, myString="something" would cause an awful lot of grief.

Share this post


Link to post
Share on other sites
That makes sense. Thanks a lot! I'll use vectors then.

Are you saying to do it like this:

void FileIoHandle::GetChunk(std::string& str, int size)
{
using namespace std;

if(!SafeForIo())
throw FileIoException
("Error-Fatal: File stream, unsafe for io.");

vector<char> buffer(size);

for(int i = 0; i < size; i++)
buffer[i] = GetChunk<char>();

vector<char>::iterator beginning;
vector<char>::iterator end;
beginning = buffer.begin();
end = buffer.end();

str.reserve(size);
str.assign(beginning, end);
}





[Edited by - Drakkcon on January 11, 2006 1:22:15 PM]

Share this post


Link to post
Share on other sites
0_0

After modifying my program with the above function, my output file is now 50MB large. I changed it again and now it's always 22MB.

There's definately a missing NULL somewhere....

Share this post


Link to post
Share on other sites
Alright, I finally found the problems.

The first was that I was missing a NULL(I think).
Also, I hadn't specified binary mode when opening a file. <<Stupid<<

Also, that serialization link was interesting Zahlman, thanks.

Oh, and setting the output mode to ios::out | ios::trunc made it create the file, awesome.

[Edited by - Drakkcon on January 11, 2006 7:38:11 PM]

Share this post


Link to post
Share on other sites

This topic is 4355 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this