Problem with ifstream returning fail()

[CppFTW]

I am really stumped with a problem my program keeps giving me. Randomly, a file I try to open returns fail() to ifstream and can't be read from. I have a feeling it has to do with these 2 functions I have been using.
Please help.

Thanks!

PS: I tried boost filesystem to get the file size and I still get the same problem.

//Moves the file to 'fileS'. 0 returned if succesful
int FileToString(const string &file_path, string &fileS)
{
	ifstream input(file_path.c_str(), ios::binary);
	if (input.fail())
	{
		return -1;
	}
	long size = GetFileSize(input);
	char* buffer = new char[size];
	input.read(buffer, size);
	fileS.assign(buffer, size);
	delete buffer;
	input.close();
	return 0;
}

long GetFileSize(ifstream &file)
{
    long temp = file.tellg();
    file.seekg(0L, ios::end);
    long end = file.tellg();
    file.seekg(temp, ios::beg);
    return end;
}

[Ancient Dragon]

Is the file more than 2 gig in size? Can you replicate the problem using the same file with a program that just calls those two functions?

[CppFTW]

No, the file is like less than 1MB.
I tried replicating the error with just those 2 functions using the same file, but it didn't work.

If it helps, I am using Vista, Visual C++ 2008 Express, and the file is in the folder C:/Cpp/programs/[something]/
I am using forward slashes and reading from the file works in a separate program.
However, if I try using ifstream and testing fail() after opening the file instead of using FileToString, I still get the error.
I checked the code and couldn't find anything wrong. What I did notice though was that calling FileToString too many times in succession on different files led to it returning -1

PS: I also tried compiling with GCC and I got the same error.

[CppFTW]

Okay wow this is really weird...
In a futile attempt to reproduce the error, I moved everything from the start until the problem piece of my source code to another project in Visual C++ and it worked...
I tried copying my whole project's source to another project hoping that it would still work. Nope.
Do the future pieces of source code (that aren't even executed yet) affect the previous pieces?

This has to be a compiler error then... Maybe even a Vista problem... sad

I don't know. It might very likely be the 2 functions I put in the first post.

[Ancient Dragon]

Can you zip up that file and post it here. I ran your program 2 million times without an error on a ReadMe.txt file.

[CppFTW]

Wait, give me 10 minutes.
I'll try to run the exact same program on an XP machine and see what happens.

[CppFTW]

Uh, I couldn't get the .exe file to run on the XP machine.

Okay, instead of giving you the file (I tried opening it with those functions and it worked...), let me give you the least code I can that reproduces this error.

I attached it.

Just open main.cpp and add functions.h to the project.
Let me know if you can run the code without getting the error "An error occurred when opening temp.dat"

And, I release all the source code attached to the public domain so don't worry about that.

Sorry for all the inconvenience and thanks a lot.

[CppFTW]

Did you guys get the same error?

[kvprajapati]

Here is a problem:
Use \r instead of \n.

data = Tokenize(s, '\r');

[CppFTW]

Here is a problem:
Use \r instead of \n.

data = Tokenize(s, '\r');

Hmm, changing that does fix the program.
You must have spent quite a bit of time looking through my code.
Thanks!

But now I am confused....
I checked out this post.
I guess my next questions would be:
1. Why does the program work when I comment out dummy(fileName); ? Why does it work when I modify files.lst to have only 1 line in it?
2. On that link above I read that Macs, Linux, and Windows all use different things (\r\n, \n, \r, ???), so what should I do to make my code portable and not give me problems?

Thanks.

PS: You guys can use the very handy Tokenize function I made in all your programs as a reward :D I made it public domain anyways, so anyone can use it.

---I feel sad that I wasted so much time with something so small... :(

[kvprajapati]

Thanks.

Everyone knows that differences and they could program accordingly.

[Ancient Dragon]

I just got back from work and looked at the zip file you posted.

The program fails when function dummy() calls FileToString() because the file name (first parameter) contains the '\n' at the end of the filename, so the os can't find that file.

To correct that you will have to modify function Tokenize() to strip the character from the string.

vector<string> Tokenize(string data, char delimiter)
{
	vector<string> pieces;
	unsigned int a = 0;
	unsigned int b;
	while (true)
	{
		b = (unsigned int)(data.find(delimiter, a));
		if (b == string::npos)
			break;
                pieces.push_back( data.substr(a, b - a-1)); 
		a = b + 1;
	}
	pieces.push_back(data.substr(a));
	return pieces;
}

[ArkM]

The wittingly wrong statement in OP code is delete buffer; - must be delete[]buffer; . Yet another defect: insufficient error check-up.

long GetFileSize(ifstream &file)
{
    static const streamoff badPos(-1);
    if (!file)  // test added
        return -1;
    streamoff temp = file.tellg();
    if (temp != badPos && file.seekg(0L, ios::end)) {
        streamoff end = file.tellg();
        if (end != badPos && file.seekg(temp, ios::beg))
            return static_cast<long>(end);
    }
    return -2;
}

/// Load the file to fileS string, return filesize. 
/// Non-positive returned if failed
long FileToString(const string &file_path, string &fileS)
{
	ifstream input(file_path.c_str(), ios::binary);
    if (!input) { // simple and clear stream state test
        fileS.clear(); // no contents
        return -1;
    }
	long size = GetFileSize(input);
    if (size <= 0) { // filesize test added.
        fileS.clear();
        return -2;
    }
	char* buffer = new char[size];
    if (input.read(buffer, size)) {
        fileS.reserve(size+size/10); // add reserve
	    fileS.assign(buffer, size);
    } else {
        size = -3;
    }
	delete [] buffer; // [] inserted!
	//input.close(); // no need: let destructor works
	return size;
}

About wrong filelist parsing (adatapost's tip):

If you want a portable code, never use FileToString to load line-oriented filelist file. It's the case where binary (low-level) stream/string processing is an absolutely irrelevan method. The old good std::getline is the best and the portable tool to "tokenize" a simple text file!

By the way, never add non-inline function definitions in .h files! Have you ever heard about separate compilation? Only function prototypes are placed in .h files.

[CppFTW]

First, sorry for the late response. I was sleeping and forgot to mention it in the last post.

@Ancient Dragon: I kind of get what you are saying because the file names do contain things like '\n' or '\r' in them, but your change to the Tokenize function breaks its generic functionality. This code doesn't work correctly:

int main()
{

	vector<string> temp = Tokenize("blah;blah;blah;blah;", ';');
	for (int i = 0; i < temp.size(); i++)
	{
		cout << temp[i] << endl;
	}

	return 0;
}

But, I get what you mean by having to take those \r or \n out of my file names.

@ArkM: Thanks for going through my code. But, I am a bit confused about some of your fixes:
1. At line 31, why do I need to reserve size + size/10 ? Shouldn't it just be size ?
2. For FileToString , I was thinking of just returning the error code and not the file size because I can just do fileS.size() . Is this a bad programming habit on my part?

Thanks all. I "approve" of all your posts :D (reputation)

[ArkM]

1. Well, reserve size (if no need in some postprocessing)
2. Tastes differs; for example, I prefer to return bool in such cases. I think, returned size if more useful than 0. Moreover, zero on success is not a natural code, look at:

if (!FileToString(...)) { // Haha, if NOT means SUCCESS

[CppFTW]

Okay.

Thanks all. I understand why I was getting my problem now and also was able to fix some more problems in my code.

[ArkM]

Okay.

Thanks all. I understand why I was getting my problem now and also was able to fix some more problems in my code.

If so, mark this thread as solved ;)

[CppFTW]

Okay, done.