C++ iostream binary reading and writing issues

[Metritutus]

I am trying to write to a file in binary with iostream, and then read that file back. If I understand correctly I should be able to read entire objects etc in to a file, and then read them back again.

However, the output I get generally seems...corrupted. I believe where I am going wrong may be in how I am calculating the size.

First, with write():

#include <cstdlib>
#include <iostream>
#include <fstream>

int main()
{
	char *memoryBlock;
	char *memoryBlockTwo;

	std::ifstream::pos_type size;// The number of characters to be read or written from/to the memory block.

	std::ofstream myFile;
	myFile.open("Example", std::ios::out | std::ios::app | std::ios::binary);

	if(myFile.is_open() && myFile.good())
	{
		std::cout<<"File opening successfully completed."<<std::endl;
		memoryBlock = "THEN";
		myFile.write(memoryBlock, 4);
	}
	else
	{
		std::cout<<"File opening NOT successfully completed."<<std::endl;
	}
	myFile.close();

Now this appears to work correctly, as I'm writing 4 characters. I can also write (sizeof(char)*4) and this also seems to work, and might well be wiser if I want to write another datatype (but cast as a char obviously)?

If I open the file in notepad, I can see no formatting changes seem to have been applied, it's not nonsense characters, I can actually read them. I can only assume this is correct and would change if I was writing anything other than a char to the file.

Assuming I'm actually doing the above correctly, the problem is with read(). In order to code read, I followed the guide [url=http://www.cplusplus.com/doc/tutorial/files/]here[/url] (under the 'Binary files' heading).

Here's the other half of what I have so far:

	std::ifstream myFileInput;
	myFileInput.open("Example", std::ios::in | std::ios::binary | std::ios::ate);

	if(myFileInput.is_open() && myFileInput.good())
	{
		std::cout<<"File opening successfully completed.  Again."<<std::endl;

		std::cout<<"READ:"<<std::endl;
		size = myFileInput.tellg();

		memoryBlockTwo = new char[size];
		myFileInput.seekg(0, std::ios::beg);// Get a pointer to the beginning of the file.
		myFileInput.read(memoryBlockTwo, size);

		std::cout<<memoryBlockTwo<<std::endl;

		delete[] memoryBlockTwo;
		std::cout<<std::endl<<"END."<<std::endl;
	}
	else
	{
		std::cout<<"Something has gone disasterously wrong."<<std::endl;
	}
	myFileInput.close();
	return 0;
}

The only difference between this and the example in the link I provided is that I attempt to print out what I have received from the file, on the assumption that as I am retrieving what was originally a char array and placing it in another char array, I should have the same value as what I wrote previously.

However instead (as the program appends, this is what happens if I only run the program once and have it create the file), this is the output:

File opening successfully completed.
File opening successfully completed.  Again.
READ:
THEN²²²²}zra

END.
Press any key to continue . . .

It's almost as though it starts printing random memory when it runs out of stuff to read (except for the fact that the 'random output' after the word 'THEN' is consistently the same up to the last ², those four ²'s are always there, I'm wondering if they represent something I should be checking for that I do not currently understand). I did wonder if it was not picking up the end-of-file properly, and specified the number '4', or (sizeof(char)*4) in the size parameter for read(), but suffered from the same issue. The contents of the file when viewed with notepad after that first run:

THEN

Any advice as to what I am doing wrong (as well as any other tips) would be greatly appreciated, thanks.

[Overv]

You're missing your null terminator, which is why it continues reading memory after "THEN", there is nothing wrong with your file code.

Except for one thing that is.

[cpp]char* memoryBlock;
memoryBlock = "THEN";[/cpp]

This is abuse of a C++ specification flaw (as I was informed), please don't do it. Actually allocate the memory:

[cpp]char memoryBlock[5];
strcpy( memoryBlock, "THEN" );[/cpp]

You should now understand why the array size is 5.

[SatriAli]

When you read in the string from the file, you don't append a null character to it.
See [URL]http://en.wikipedia.org/wiki/Null-terminated_string[/URL]

If you want a safe and easy solution that handles null-termination automatically, just use a std::string.

Edit: Ninja'd by Overv

[Metritutus]

[QUOTE=Overv;35710480]You're missing your null terminator, which is why it continues reading memory after "THEN", there is nothing wrong with your file code.

Except for one thing that is.

[cpp]char* memoryBlock;
memoryBlock = "THEN";[/cpp]

This is abuse of a C++ specification flaw (as I was informed), please don't do it. Actually allocate the memory:

[cpp]char memoryBlock[5];
strcpy( memoryBlock, "THEN" );[/cpp]

You should now understand why the array size is 5.[/QUOTE]

Altering the statement to be one character longer than the actual text does indeed seem to amend the issue:

myFile.write(memoryBlock, (sizeof(char)*5));

But...abuse of a C++ specification flaw you say (with regards to using char* rather than an array of specific length)? How do you mean?

EDIT:

[QUOTE=SatriAli;35710512]
If you want a safe and easy solution that handles null-termination automatically, just use a std::string.[/QUOTE]

Wouldn't std::string have a lot of other stuff as part of it when I tried to write it to the file rather than a basic char array? For these early tests of mine, wouldn't it be harder to see if I was doing stuff correctly?

[SatriAli]

[QUOTE=Metritutus;35710521]Wouldn't std::string have a lot of other stuff as part of it when I tried to write it to the file rather than a basic char array? For these early tests of mine, wouldn't it be harder to see if I was doing stuff correctly?[/QUOTE]
You can access the underlying array if you want to, there may be a specific method in std::string to get the pointer, but alternatively I think the standard says strings need to use contiguous memory so [I]&myString[0][/I] should work too.
Edit: Scratch that, seems the standard doesn't specify that strings must use contiguous memory (although I'd imagine all implementations do, certainly the ones I know of). So if you want to play it safe, just allocate an additional character when you read in the string and set it to '\0'.

size = myFileInput.tellg();
memoryBlockTwo = new char[size+1];
memoryBlockTwo[size] = '\0';
myFileInput.seekg(0, std::ios::beg);// Get a pointer to the beginning of the file.
myFileInput.read(memoryBlockTwo, size);

[Overv]

You're assigning the memory address of a constant string to a pointer, which is unsafe behaviour and can crash your program. The compiler assumes the string will not be modified, but you're assigning it to a character array that can be modified freely (AKA not constant).

[QUOTE=SatriAli;35710594]You can access the underlying array if you want to, there may be a specific method in std::string to get the pointer, but alternatively I think the standard says strings need to use contiguous memory so [I]&myString[0][/I] should work too.[/QUOTE]

Yes, or just call the function that returns the pointer: [url=http://www.cplusplus.com/reference/string/string/c_str/]c_str()[/url].

[SatriAli]

[QUOTE=Overv;35710600]Yes, or just call the function that returns the pointer: [url=http://www.cplusplus.com/reference/string/string/c_str/]c_str()[/url].[/QUOTE]
That only returns a constant pointer though. You should definitely use c_str() when writing the contents of a string to file, but it won't help with reading the file back into the string.

And to clarify about strings and contiguous memory, see this blog comment by Herb Sutter [url]http://herbsutter.com/2008/04/07/cringe-not-vectors-are-guaranteed-to-be-contiguous/#comment-483[/url]
In short the pre-c++11 standard didn't actually require it, but it was always the case nonetheless.

[Metritutus]

[QUOTE=Overv;35710600]You're assigning the memory address of a constant string to a pointer, which is unsafe behaviour and can crash your program. The compiler assumes the string will not be modified, but you're assigning it to a character array that can be modified freely (AKA not constant).[/QUOTE]

I'm afraid I'm still not sure I follow, how is the char* a constant? I thought one of the good things about using it was that it was not a fixed size, ie I could change it to be an entirely different string.

Do you have anything I could read to help clear up my mind about this? Also does this affect the reading part of my program where I read the file in to a char*? Only that file could be any length, and I might want to reuse the char* again. Is this why I would use the new[] and delete[] operators?

[Overv]

[QUOTE=SatriAli;35710713]That only returns a constant pointer though. You should definitely use c_str() when writing the contents of a string to file, but it won't help with reading the file back into the string.

And to clarify about strings and contiguous memory, see this blog comment by Herb Sutter [url]http://herbsutter.com/2008/04/07/cringe-not-vectors-are-guaranteed-to-be-contiguous/#comment-483[/url]
In short the pre-c++11 standard didn't actually require it, but it was always the case nonetheless.[/QUOTE]

I think I missed the part where you simply allocate a char array, read the file into it and construct a string from it.

[dajoh]

[QUOTE=Overv;35710600]You're assigning the memory address of a constant string to a pointer, which is unsafe behaviour and can crash your program. The compiler assumes the string will not be modified, but you're assigning it to a character array that can be modified freely (AKA not constant).[/QUOTE]

It's not going to crash unless you write to it. You don't have to write it to the buffer manually though, you can just do:

[cpp]
char x[5] = "test";
[/cpp]

[Protocol7]

I believe that the problem is that you're assigning the address of the string to the pointer, not that the char* is a constant, but as stated shouldn't be a problem unless you do something you shouldn't (funny how that works).

The new operator just declares a new variable that can access the free store. Delete just calls the destructor to free up the free store. Memory management as to avoid memory leaks.

[dajoh]

[QUOTE=Protocol7;35711220]I believe that the problem is that you're assigning the address of the string to the pointer, not that the char* is a constant, but as stated shouldn't be a problem unless you do something you shouldn't (funny how that works).[/QUOTE]

The problem is that when compiled, "THEN" ends up in a read-only region, writing to it causes an access violation. The compiler [b]should[/b] spit out an error saying that you can't convert from [i]const char *[/i] to [i]char *[/i] implicitly, why it doesn't is beyond me, but MSVC is generally really crappy.

[QUOTE=Protocol7;35711220]The new operator just declares a new variable that can access the free store. Delete just calls the destructor to free up the free store.[/QUOTE]

what

[Protocol7]

[QUOTE=dajoh;35711282]what[/QUOTE]

[url]http://publib.boulder.ibm.com/infocenter/comphelp/v8v101/index.jsp?topic=%2Fcom.ibm.xlcpp8a.doc%2Flanguage%2Fref%2Fcplr381.htm[/url]

Free store.

I guess I should clarify the operators themselves don't modify the variable, it just changes how the memory is handled by the variable.

(In case you can't tell I'm pretty lackluster at explanations)

Also, I hate MSVC. If I'm doing C++ I am using GCC and at the very least vim.

[SatriAli]

[QUOTE=Overv;35711117]I think I missed the part where you simply allocate a char array, read the file into it and construct a string from it.[/QUOTE]
That also works. Reading directly into a resized string just cuts out a few steps and removes any manual memory management, my comment was only relevant to that specific scenario.

[Overv]

[QUOTE=dajoh;35711210]It's not going to crash unless you write to it. You don't have to write it to the buffer manually though, you can just do:

[cpp]
char x[5] = "test";
[/cpp][/QUOTE]

That's what I meant by "can".

[QUOTE=SatriAli;35711452]That also works. Reading directly into a resized string just cuts out a few steps and removes any manual memory management, my comment was only relevant to that specific scenario.[/QUOTE]

A resized string is manual memory management.

[Lord Ned]

[QUOTE=Overv;35710480]You're missing your null terminator, which is why it continues reading memory after "THEN", there is nothing wrong with your file code.

Except for one thing that is.

[cpp]char* memoryBlock;
memoryBlock = "THEN";[/cpp]

This is abuse of a C++ specification flaw (as I was informed), please don't do it. Actually allocate the memory:

[cpp]char memoryBlock[5];
strcpy( memoryBlock, "THEN" );[/cpp]

You should now understand why the array size is 5.[/QUOTE]
This is correct. I was under the impression that if you assigned "char * memoryBlock ="THEN";", you're assigning memoryBlock to a statically allocated piece of memory when the program loads. You then go back and load other data into it - This is fishy and what happens when you try to read more memory into it than was originally allocated.

Also if you take of the std::ios::binary it should detect line breaks and null terminators.

[Protocol7]

And this is why I hate strings.

[Metritutus]

[QUOTE=Lord Ned;35711519]
Also if you take of the std::ios::binary it should detect line breaks and null terminators.[/QUOTE]

Then it would not be writing to the file in binary like I want?

Also, I seem to have further issues.

Currently (the reading part of the program is unchanged):

	char memoryBlock[5];
	char *memoryBlockTwo;
	std::ifstream::pos_type size;// The number of characters to be read or written from/to the memory block.

	std::ofstream myFile;
	myFile.open("Example", std::ios::out | std::ios::app | std::ios::binary);

	if(myFile.is_open() && myFile.good())
	{
		std::cout<<"File opening successfully completed."<<std::endl;
		strcpy_s(memoryBlock, "THIS");
		myFile.write(memoryBlock, (sizeof(char)*6));
	}

When I try to print out the result the first time, all seems well.

When I check the file, it says the string, but it also has 'NUL' highlighted in black after it. If I run my program again to append to the file, in the program it prints out what I first added. If I look at the file in notepad, it's been scrambled in to asian lettering.

[Lord Ned]

Why not use the << operator?

[cpp]

ofstream myfile;
myfile.open ("example.txt");
myfile << "Writing this to a file.\n";
myfile.close();
[/cpp]

[b]Also: You're not calling myFile.close();[/b]

[Metritutus]

[QUOTE=Lord Ned;35711890]Why not use the << operator?

[cpp]

ofstream myfile;
myfile.open ("example.txt");
myfile << "Writing this to a file.\n";
myfile.close();
[/cpp]

[b]Also: You're not calling myFile.close();[/b][/QUOTE]

I am calling it, but it's not in the latest excerpt I posted. You can view the complete code in my first post, aside from the changes I've made that are included in the post I made before yours.

And I'm not using the << operator because apparently it's not efficient as I am writing to the file in binary. I do not need to format any of the text assuming I am actually writing text in the first place (which I am for learning purposes right now).

[Protocol7]

[QUOTE=Lord Ned;35711890][b]Also: You're not calling myFile.close();[/b][/QUOTE]

I think this is your problem, you might have some stray data entering your ostream.

edit: maybe not, let me pull up some old homework, I think I can find some examples and compare side by side

[Metritutus]

For a moment I thought I'd worked out what was wrong; 'THIS' is four letters long, not five, so I was writing junk memory to the file along with what I wanted. So I altered the numbers accordingly as shown below. Unfortunately the program now gives a "Debug Assertion Failed!" error, stating 'Expression: (L"Buffer is too small" && 0)' when it reaches strcpy_s().

I'm interpreting this as memoryBlock[4] being too small to hold four chars!? What am I missing here?

	char memoryBlock[4];
	char *memoryBlockTwo;
	std::ifstream::pos_type size;// The number of characters to be read or written from/to the memory block.

	std::ofstream myFile;
	myFile.open("Example", std::ios::out | std::ios::app | std::ios::binary);

	if(myFile.is_open() && myFile.good())
	{
		std::cout<<"File opening successfully completed."<<std::endl;
		strcpy_s(memoryBlock, (sizeof(char)*4),"THIS");
		myFile.write(memoryBlock, (sizeof(char)*5));
	}

[Protocol7]

That's because you just omitted the slot for the null terminator.

[Metritutus]

[QUOTE=Protocol7;35714095]That's because you just omitted the slot for the null terminator.[/QUOTE]

So the null terminator is part of the char array? I was trying to apply it during write().

Now I have:
char memoryBlock[5];
strcpy_s(memoryBlock, (sizeof(char)*5),"THIS");
myFile.write(memoryBlock, (sizeof(char)*5));

I am now back to the issue I specified in [url=http://facepunch.com/threads/1179772?p=35711755&viewfull=1#post35711755]post 18[/url].

[Protocol7]

Post the whole, current code so I can run it myself. I think it's because the read function is told to read the terminator too (i.e. the stream too big and it is reading the null terminator when it shouldn't).

[Overv]

By the way, stop using [i]sizeof(char)[/i], it's defined to be 1 by the spec.

[Protocol7]

I scraped together your code from the snippets you posted and it works fine for me.

[IMG]http://puu.sh/rt9B[/IMG]

My code:

[cpp]#include <cstdlib>
#include <iostream>
#include <fstream>
#include <stdio.h>
#include <string.h>

int main()
{
char memoryBlock[5];
char *memoryBlockTwo;

std::ifstream::pos_type size;// The number of characters to be read or written from/to the memory block.

std::ofstream myFile;
myFile.open("Example", std::ios::out | std::ios::app | std::ios::binary);

if(myFile.is_open() && myFile.good())
{
std::cout<<"File opening successfully completed."<<std::endl;
strcpy(memoryBlock, "THIS");
myFile.write(memoryBlock, (sizeof(char)*6));
}
else
{
std::cout<<"File opening NOT successfully completed."<<std::endl;
}
myFile.close();

std::ifstream myFileInput;
myFileInput.open("Example", std::ios::in | std::ios::binary | std::ios::ate);

if(myFileInput.is_open() && myFileInput.good())
{
std::cout<<"File opening successfully completed. Again."<<std::endl;

std::cout<<"READ:"<<std::endl;
size = myFileInput.tellg();

memoryBlockTwo = new char[size];
myFileInput.seekg(0, std::ios::beg);// Get a pointer to the beginning of the file.
myFileInput.read(memoryBlockTwo, size);

std::cout<<memoryBlockTwo<<std::endl;

delete[] memoryBlockTwo;
std::cout<<std::endl<<"END."<<std::endl;
}
else
{
std::cout<<"Something has gone disasterously wrong."<<std::endl;
}
myFileInput.close();
return 0;
}
[/cpp]

[Metritutus]

[QUOTE=Protocol7;35714642]I scraped together your code from the snippets you posted and it works fine for me.[/QUOTE]

What happens if you run it multiple times? It does not append correctly. It will display okay in the program if you run it once, but not twice. Also the contents of the file if viewed with notepad appear somewhat strange.

[Protocol7]

I see what you mean.

[IMG]http://puu.sh/rts9[/IMG]

What's the desired behavior? THISTHISTHISTHIS[NUL]?

[Metritutus]

[QUOTE=Protocol7;35715024]I see what you mean.

[IMG]http://puu.sh/rts9[/IMG]

What's the desired behavior? THISTHISTHISTHIS[NUL]?[/QUOTE]

Well I would assume that when I call read() I retrieve all the values I've written intact.