Read Access Violation on Function return

[riversr54]

I have a simple little app that records weather data and does some calculations on it. The data is stored in Weather structures. The structures are stored in a Vector (vector) and then written to a binary file as structures. I'm now writing a function to read all that data from the file back in and store it on the vector, i.e. when the program initializes. My function (readWeatherData) accepts a pointer to the vector and successfully reads all the data and populates the vector with weather data. All is good to that point. The problem arises when I return from the function. The function returns nothing (void). When the return is attempted I get a "Read Access Violation".

struct Weather
{
string name;
string year;
float high;
float low;
};

void readWeatherData(vector<Weather>*);

int main()
{
vector<Weather>* weatherData = new vector<Weather>();

readWeatherData(weatherData);
}


void readWeatherData(vector<Weather>* WD)
{
string filename;
Weather weather;
WD->clear();

cout << endl << "Enter input file name: ";
cin >> filename;
ifstream inFile("C:\\mydata\\" + filename, ios::in | ios::binary);

if (inFile)
{
while (!(inFile.eof()))
{
inFile.read(reinterpret_cast<char*>(&weather), sizeof(Weather));
WD->push_back(weather);
inFile.peek(); //force file to see if it is at EOF
}
}
inFile.close();

} //when function returns, gets "Read Access Violation"

[Ganado]

Your weather structure is not POD*, i.e. it contains pointers internally. A string's data is stored on the heap. You can't get data from a file and directly read it into your Weather object.

You need to actually write the correct information to your file, in a consistent way. The two ways to do this are either by having null-terminating strings, or by having a [size of string][string bytes] format.

*https://stackoverflow.com/a/146454/8690169

_______________

And why don't you just do
vector<Weather> weatherData;?

Just curious, did you come here from Java?

[riversr54]

You stated:

You can't get data from a file and directly read it into your Weather object.

Please help me understand...because I have done exactly that. The Weather object gets populated perfectly from the file read. I can see all the specifics of the Weather object, actually 1000 of them in the vector. That part seems to be working just fine. What am I missing?

No, not from JAVA but a lot of C# experience, maybe that's my problem.

I tried declaring the vector like this:

vector<Weather> weatherData;

C++ doesn't like that, it complains about an uninitialized variable.

Thank you very much for trying to help me understand.

[jlb]

I tried declaring the vector like this:

vector<Weather> weatherData;

C++ doesn't like that, it complains about an uninitialized variable.

Did you include all the required #include files, like <vector>

The Weather object gets populated perfectly from the file read. I can see all the specifics of the Weather object, actually 1000 of them in the vector.

How exactly are you verifying that you are reading the file correctly?

I wouldn't be surprised if your "Read Access Violation" isn't being caused by trying to read() something into an empty string.

[salem c]

> Please help me understand...because I have done exactly that.
No, you haven't.

$ g++ foo.cpp
$ cat foo.cpp
#include <iostream>
#include <fstream>
#include <string>
using namespace std;

int main()
{
  string foo = "this is my string, and a fine string indeed";
  ofstream of("foo.dat");
  of.write(reinterpret_cast<char*>(&foo),sizeof(foo));
  return 0;
}
$ ./a.out 
$ hd foo.dat
00000000  20 2c e3 00 00 00 00 00  2b 00 00 00 00 00 00 00  | ,......+.......|
00000010  2b 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |+...............|
00000020

Do you see a string in the file here?

No, because what was written to the file was basically a pointer and a length.

You CANNOT just blindly write() a std::string in the manner you have done somewhere else, and then have a hope in hell of using read() to read it back in.

It might superficially work in a really tiny toy program, but that's only because any std::strings you refer to haven't gone out of scope yet.

Go back to where you created your file in the first place and make a better job of writing the data out to your file.

Then adjust your reading code to match.

[riversr54]

Please help me understand. Just before the function returns, I can see all data in the 1000 weather objects in the vector by using the Debugger in Visual Studio. The data is there, in the vector.

The data was written to the file using the technique that you provided in you example, except that I wrote it as Weather objects, not strings. I do get your point, but I just don't see why if I can see and verify that the vector contains the populated Weather objects, why there is a problem.

Below is a snippet of the file that I am reading. It was written in binary as you can see but you can see the three character month abbreviations and the years in the data, so the string data is in fact in the file.

I opened the file with a binary editor and I can in fact see all the data in the file, including the strings. I'm working on getting a small snippet of that file here as evidence.

[jlb]

Okay then post your complete program, don't forget to include the "write" function and just a small sample of your weather data.

[riversr54]

Ok, here's the entire program. It collects data from the user, stores it in Weather objects which are in turn stored in a Vector (getWeatherData). That data is written into a file as Weather objects(writeWeatherData). Lastly the data is cleared from the vector and read back into memory in the readWeatherData function. It's this function that is giving me a problem. FYI: there is a getSingleMonth function which reads a single month of data from the file based on a supplied index. That function works just fine.

After the code I have included the first couple hundred bytes of the output/input file. If you look at this in a binary editor or listing program, you can clearly see the data from the Weather object there, including the string data.

Thanks again for any insight into this problem.

#include <iostream>
#include <fstream>
#include <vector>
using namespace std;

struct Weather
{
	string name;
	string year;
	float high;
	float low;
};

void getWeatherData(vector<Weather>*);
void writeWeatherData(vector<Weather>*);
void readWeatherData(vector<Weather>*);
void getSingleMonth(long, Weather*);

int main()
{
	vector<Weather>* weatherData;
	getWeatherData(weatherData);			//get user input into weather objects and add to vector
	writeWeatherData(weatherData);			//write weather objects from vector to file
	readWeatherData(weatherData);			//read weather data OBJECTS from file and place on vector
}

void getWeatherData(vector<Weather>* WD)
{
	//gets data from the user, stores it in a temporary weather object and then adds the object to the vector
	bool more = true;
	char choice = NULL;
	while (more)
	{
		Weather tmpWeather;
		cout << "Enter the name of the Month: ";
		cin >> tmpWeather.name;
		cout << "Enter High Temp for the month: ";
		cin >> tmpWeather.high;
		cout << "Enter Low Temp for the Month: ";
		cin >> tmpWeather.low;
		WD->push_back(tmpWeather);
		cout << endl << "Enter E to Exit, M to add more: ";
		cin >> choice;
		if (choice == 'E')
			more = false;
	}
}

void writeWeatherData(vector<Weather>* WD)
{
	//writes all weather objects from the vector to the output binary file
	fstream outFile("C:\\mydata\\weather.out", ios::out | ios::binary);
	if (outFile)
	{
		for (Weather weather : *WD)
			outFile.write(reinterpret_cast<char*>(&weather), sizeof(Weather));
	}
	outFile.close();
}

void readWeatherData(vector<Weather>* WD)
{
	Weather weather;
	WD->clear();		//remove all previous data from the vector

	ifstream inFile("C:\\mydata\\weather.out", ios::in | ios::binary);
	if (inFile)
	{
		while (!(inFile.eof()))
		{
			inFile.read(reinterpret_cast<char*>(&weather), sizeof(Weather));
			WD->push_back(weather);
			inFile.peek();	//force file to see if it is at EOF
		}
	}
	inFile.close();
}

void getSingleMonth(long index, Weather* weather)
{
	//sets file position based on index and sizeof weather object then reads and returns pointer to weather object
	string filename;

	cout << endl << "Enter input file name: ";
	cin >> filename;
	ifstream inFile("C:\\mydata\\" + filename, ios::in | ios::binary);
	if (inFile)
	{
		inFile.seekg((index * (long)sizeof(Weather)), ios::beg);
		cout << "Pos: " << inFile.tellg() << "   " << "Weather Size: " << sizeof(Weather) << endl;
		inFile.read(reinterpret_cast<char*> (weather), sizeof(Weather));
	}
	inFile.close();
}

40 2f 47 01 4f 63 74 00
cc cc cc cc cc cc cc cc
cc cc cc cc 03 00 00 00
0f 00 00 00 30 2c 47 01
32 30 30 32 00 cc cc cc
cc cc cc cc cc cc cc cc
04 00 00 00 0f 00 00 00
00 00 f8 41 00 00 40 41
30 2c 47 01 41 70 72 00
cc cc cc cc cc cc cc cc
cc cc cc cc 03 00 00 00
0f 00 00 00 40 2f 47 01
32 30 31 37 00 cc cc cc
cc cc cc cc cc cc cc cc
04 00 00 00 0f 00 00 00
00 00 20 42 00 00 e8 41
e0 31 47 01 4f 63 74 00
cc cc cc cc cc cc cc cc
cc cc cc cc 03 00 00 00
0f 00 00 00 80 2d 47 01
31 39 35 39 00 cc cc cc
cc cc cc cc cc cc cc cc
04 00 00 00 0f 00 00 00
00 00 70 41 00 00 10 41

[jlb]

Where do you ever allocate memory for that weatherData pointer in main()?

Why are you trying to use a pointer in the first place, a normal non-pointer instance would be adequate.

And you really should be passing that vector to your functions using pass by reference instead of the pointers.



Edit: Oh and by the way your read and write functions will still not work as expected.

[AbstractionAnon]

Please help me understand.

Writing std::string to a file only works because your library implementation happens to store short strings directly in a string object. If your month names were longer than 15 bytes, this would not work because the library would allocate the string from the heap and store only a pointer in the string object. Then when you write a string object to disk, you write a random pointer to memory as salem c demonstrated. The implementation of std::string is private to the library. You can only rely on the documented interface.

It is an ABHORRENT practice to to assume your strings are always going to less than 16 bytes and that std::string is implemented in this manner in your run time library. As others have said, DO NOT DO THIS.

[riversr54]

I appreciate all the help but the fact remains that I have a pointer to a vector of Weather objects in my function, why does returning from that function cause a read access violation? Because it contains string objects? If that is it, what is the solution? In this particular case all the strings happen to be just 3-4 characters, so that still doesn't explain (at least not to me) why it is failing.

Jib:
If my write function isn't working then how can I read data from the file with my getSingleMonth function. It works perfectly.

AbstractionAnon:
Abhorrent? Really? maybe wrong, but Abhorrent (repugnant, disgusting), I don't think so. I haven't committed any repugnant crime or disgusting act. Maybe I'm just learning but that's pretty strong language when we're just talking about C++ Code.

I'm not trying to be stubborn, just need to figure it out and make it work.

[jlb]

If my write function isn't working then how can I read data from the file with my getSingleMonth function. It works perfectly.

You really need to try to use realistic data, especially for your strings.

That sample data is really going to hide the problems being mentioned in this thread, especially since it doesn't match your structure variables.

Try inputting something like:

May 2000 110 -110
January 1988 90 -20
December 1200 40 39

Also note that you should really try reading the input file that was created without first writing the data.

And:

I appreciate all the help but the fact remains that I have a pointer to a vector of Weather objects in my function

Why do you have a pointer to a vector? This is usually a bad practice. And exactly where are you allocating memory for this pointer?

why does returning from that function cause a read access violation?

Because what you're trying to do is wrong. As far as proof your program is crashing. There may be several different possibilities as to what is causing the problem, but the problems exist don't fool yourself.

[AbstractionAnon]

Repugnant and disgusting code, absolutely. If any programmer working for me relied on guilty knowledge of a C++ library object, they would be looking for a new job.

Trying to read/write a non-POD type to a file is a common newbie mistake. You been told multiple times it is wrong. Stop trying to defend it and move past it.

gando has given you two suggestions on how to do that.

Here's another approach. Make Weather a POD:

struct Weather
{   char name[15];
    int year;
    float high;
    float low;
};

While I generally prefer std::string over C strings, changing name to a fixed size char array will allow you to read/write Weather objects to a file.

[salem c]

Because it won't even run as posted because of this

foo.cpp: In function ‘int main()’:
foo.cpp:22:29: warning: ‘weatherData’ is used uninitialized in this function [-Wuninitialized]
  getWeatherData(weatherData);   //get user input into weather objects and add to vector 

I changed it to a regular vector reference, and removed all the Winblows cruft.
It of course crashes as expected because the serialisation is broken.

#include <iostream>
#include <fstream>
#include <vector>
using namespace std;

struct Weather {
  string name;
  string year;
  float high;
  float low;
};

void getWeatherData(vector < Weather > &);
void writeWeatherData(vector < Weather > &);
void readWeatherData(vector < Weather > &);

int main()
{
  cout << "sizeof Weather = " << sizeof(Weather) << endl;
  cout << "sizeof String  = " << sizeof(string) << endl;
  cout << "sizeof float   = " << sizeof(float) << endl;
  vector < Weather > weatherData;
  getWeatherData(weatherData);  //get user input into weather objects and add to vector
  writeWeatherData(weatherData);  //write weather objects from vector to file
  readWeatherData(weatherData); //read weather data OBJECTS from file and place on vector
}

void getWeatherData(vector < Weather > &WD)
{
  //gets data from the user, stores it in a temporary weather object and then adds the object to the vector
  bool more = true;
  char choice = '\0';           //!! NOT NULL, NULL != '\0' (which is nul)
  while (more) {
    Weather tmpWeather;
    cout << "Enter the name of the Month: ";
    cin >> tmpWeather.name;
    cout << "Enter High Temp for the month: ";
    cin >> tmpWeather.high;
    cout << "Enter Low Temp for the Month: ";
    cin >> tmpWeather.low;
    WD.push_back(tmpWeather);
    cout << endl << "Enter E to Exit, M to add more: ";
    cin >> choice;
    if (choice == 'E')
      more = false;
  }
}

void writeWeatherData(vector < Weather > &WD)
{
  //writes all weather objects from the vector to the output binary file
  fstream outFile("weather.out", ios::out | ios::binary);
  if (outFile) {
  for (Weather weather:WD)
      outFile.write(reinterpret_cast < char *>(&weather), sizeof(Weather));
  }
  outFile.close();
}

void readWeatherData(vector < Weather > &WD)
{
  Weather weather;
  WD.clear();                   //remove all previous data from the vector

  ifstream inFile("weather.out", ios::in | ios::binary);
  if (inFile) {
    while (!(inFile.eof())) {
      inFile.read(reinterpret_cast < char *>(&weather), sizeof(Weather));
      WD.push_back(weather);
      inFile.peek();            //force file to see if it is at EOF
    }
  }
  inFile.close();
}


$ g++ -Wall -std=c++11 -g foo.cpp
$ gdb -q ./a.out
Reading symbols from ./a.out...done.
(gdb) run

sizeof Weather = 72
sizeof String  = 32
sizeof float   = 4
Enter the name of the Month: September
Enter High Temp for the month: 55
Enter Low Temp for the Month: 44

Enter E to Exit, M to add more: M
Enter the name of the Month: April
Enter High Temp for the month: 44
Enter Low Temp for the Month: 33

Enter E to Exit, M to add more: E
*** Error in `./a.out': munmap_chunk(): invalid pointer: 0x00007fffffffdb50 ***

Program received signal SIGABRT, Aborted.
0x00007ffff74aa428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
54	../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  0x00007ffff74aa428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
#1  0x00007ffff74ac02a in __GI_abort () at abort.c:89
#2  0x00007ffff74ec7ea in __libc_message (do_abort=do_abort@entry=2, fmt=fmt@entry=0x7ffff7605ed8 "*** Error in `%s': %s: 0x%s ***\n") at ../sysdeps/posix/libc_fatal.c:175
#3  0x00007ffff74f9698 in malloc_printerr (ar_ptr=0x0, ptr=<optimised out>, str=0x7ffff7605f00 "munmap_chunk(): invalid pointer", action=<optimised out>) at malloc.c:5006
#4  munmap_chunk (p=<optimised out>) at malloc.c:2842
#5  __GI___libc_free (mem=<optimised out>) at malloc.c:2963
#6  0x0000000000401aac in Weather::~Weather (this=0x7fffffffdb30, __in_chrg=<optimised out>) at foo.cpp:6
#7  0x000000000040198c in readWeatherData (WD=std::vector of length 2, capacity 2 = {...}) at foo.cpp:65
#8  0x0000000000401569 in main () at foo.cpp:26 

Pay CLOSE attention to the address in the dtor.

$ hd weather.out 
00000000  30 db ff ff ff 7f 00 00  09 00 00 00 00 00 00 00  |0...............|
00000010  53 65 70 74 65 6d 62 65  72 00 00 00 00 00 00 00  |September.......|
00000020  50 db ff ff ff 7f 00 00  00 00 00 00 00 00 00 00  |P...............|
00000030  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000040  00 00 5c 42 00 00 30 42  30 db ff ff ff 7f 00 00  |..\B..0B0.......|
00000050  05 00 00 00 00 00 00 00  41 70 72 69 6c 00 62 65  |........April.be|
00000060  72 00 00 00 00 00 00 00  50 db ff ff ff 7f 00 00  |r.......P.......|
00000070  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000080  00 00 00 00 00 00 00 00  00 00 30 42 00 00 04 42  |..........0B...B|
00000090

this=0x7fffffffdb30 is what the code yanked out of the file and then tried to use it as if it were a real address. It was real, but ONLY at the moment you wrote it to the file.
At any other time, it's meaningless garbage and folly to try and use it as an address.

Compare with

void writeWeatherData(vector < Weather > &WD)
{
  //writes all weather objects from the vector to the output binary file
  fstream outFile("weather.out", ios::out | ios::binary);
  if (outFile) {
  for (Weather weather:WD) {
      size_t len = weather.name.length();
      outFile.write(reinterpret_cast < char *>(&len), sizeof(len));
      outFile.write(reinterpret_cast < const char *>(weather.name.c_str()), len);
      outFile.write(reinterpret_cast < char *>(&weather.high), sizeof(weather.high));
      outFile.write(reinterpret_cast < char *>(&weather.low), sizeof(weather.low));
    }
  }
  outFile.close();
}

void readWeatherData(vector < Weather > &WD)
{
  Weather weather;
  WD.clear();                   //remove all previous data from the vector

  ifstream inFile("weather.out", ios::in | ios::binary);
  if (inFile) {
    size_t len;
    while (inFile.read(reinterpret_cast < char *>(&len), sizeof(len))) {
      char *tmp = new char[len];
      inFile.read(reinterpret_cast < char *>(tmp), len);
      weather.name = string(tmp, len);
      inFile.read(reinterpret_cast < char *>(&weather.high), sizeof(weather.high));
      inFile.read(reinterpret_cast < char *>(&weather.low), sizeof(weather.low));

      WD.push_back(weather);
      delete [] tmp;
    }
  }
  inFile.close();
}


$ hd weather.out 
00000000  09 00 00 00 00 00 00 00  53 65 70 74 65 6d 62 65  |........Septembe|
00000010  72 00 00 5c 42 00 00 30  42 05 00 00 00 00 00 00  |r..\B..0B.......|
00000020  00 41 70 72 69 6c 00 00  5c 42 00 00 30 42        |.April..\B..0B|
0000002e

Aside from being a shorter file, you'll also notice it doesn't contain any raw memory pointers.
Sure it's a PITA to need 4 transactions per record rather than one, but there are no short cuts.

FWIW, if you were being serious about serialising a lot of data, then this recent thread is well worth a read.
http://www.cplusplus.com/forum/general/268402/


If you really want a single write() per weather data, then you HAVE to do this.

struct Weather {
  char name[20];
  char year[5];
  float high;
  float low;
};

You're only allowed the elemental types, arrays of elemental types, or nested structs containing only elemental types. Pointers and class variables do not work with simple block write calls.

[riversr54]

I finally figured it out with all you guys help. All I had to do was change my structure to POD data types. I just couldn't get the string problem in my head. I sure wish I had figured that out a few days ago.

Thanks for the help.