Strange Problem with file IO....

Started by
12 comments, last by Drakkcon 18 years, 3 months ago
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 =/
Advertisement
What does GetChunk<char> do?

Stephen M. Webb
Professional Free Software Developer

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;        }
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.

Stephen M. Webb
Professional Free Software Developer

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]
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 ;) )
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]
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).

Stephen M. Webb
Professional Free Software Developer

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++.
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.

Stephen M. Webb
Professional Free Software Developer

This topic is closed to new replies.

Advertisement