"Write access violation" Exception thrown in C++ program

[ClobberXD]

 

I am having a write access violation exception being thrown in this program.

typedef struct EventFormat
{
  char* notes;
  int ID;
}

int main()
{
    .
    .
    .
    
    ifstream db_in(input.txt);
    
	EventFormat temp;
	vector<EventFormat> Event;
				
	// Load struct array
	while(!db_in.eof())
	{
		db_in.get(temp.notes, '\n');	/********************* EXCEPTION THROWN HERE!!! ********************/
	  
		Event.push_back(temp);
	}
  
    .
    .
    .
}

I believe something is (obviously) wrong with char* EventFormat.notes but can't find out what? Initialization? Memory-allocation? Quick help please... Thanks!

[Yamoto42]

Well, you've never shown what EventFormat.notes points to.  It is a pointer...

[ClobberXD]

5 minutes ago, Yamoto42 said:

Well, you've never shown what EventFormat.notes points to.  It is a pointer...

So I do that using 'new' operator?

[Yamoto42]

16 minutes ago, Anand_Geforce said:

So I do that using 'new' operator?

Almost, it is actually the new[] operator, and if you ever need to change it use the delete[] operator.  The operators for arrays are technically different than the operators for individuals.  The new[] operator does contain the number of elements in the brackets, but delete[] does not.

One thing to always remember is that old c-style strings were never actually strings, but character arrays.  Strings imply certain qualities(metadata), such as length that arrays do not possess.  Just because something is a char* doesn't make it a string.  It is just an array of bytes, not even guaranteed to be ASCII...although the specification leaves whether they are signed or unsigned to the implementation.  Since C doesn't have that metadata concept as part of the language, the convention was a null terminated character array.

 

 

And just for completeness...although you can ignore this part:

 

I emphasize the term 'array' also, as in C and C++ arrays aren't actually arrays. An array actually implies an object with metadata...again such as length and type.  Similarly, all arrays are themselves just pointers that hide the pointer arithmetic.  The different operators are for memory management purposes, as they are implemented slightly differently.  This doesnt mean much for primitives, but for actual objects with destructors it makes A LOT of difference.

[Mr_KoKa]

You can use std::getline to read line from stream directly to std::string. It will make it easier for you because you will not have to allocate and free memory by yourself.

You would need change notes type from char* to std::string.

[Yamoto42]

8 minutes ago, Mr_KoKa said:

You can use std::getline to read line from stream directly to std::string. It will make it easier for you because you will not have to allocate and free memory by yourself.

You would need change notes type from char* to std::string.

And technically there's the rule of 3 (or 5) with his current design...I just wish it was mandatory to learn basic C before C++ so people at least had an idea what was actually going on...

I'm not saying to re-invent the wheel...but at least understand why it's round.

[ClobberXD]

20 minutes ago, Yamoto42 said:

And technically there's the rule of 3 (or 5) with his current design...I just wish it was mandatory to learn basic C before C++ so people at least had an idea what was actually going on...

I'm not saying to re-invent the wheel...but at least understand why it's round.

I have learnt basic C, but that's it. I realized that I signed up for a dumbass C and C++ combo course, shortly after finishing it, where they only teach you to get an input from a user at runtime, and output variables to CMD (only CMD - they didn't even mention anything else - no APIs, GUIs, STL, bitwise operations, etc.) Technically I've learnt C & C++, but I know nothing about it... Sorry about it!

[ClobberXD]

26 minutes ago, Yamoto42 said:

And technically there's the rule of 3 (or 5) with his current design...I just wish it was mandatory to learn basic C before C++ so people at least had an idea what was actually going on...

I'm not saying to re-invent the wheel...but at least understand why it's round.

And what am I to do here anyway?

[Mr_KoKa]

When your EventFormat has pointer, and you will eg. assign it to another EventFormat instance, the pointer will be copied, not the data that the pointer points to.

So if both instances shares pointer and one instance is freed with a destructor that shoudl delete[] pointer, the other instance still has a pointer that points at freed memory block.

 

#include <iostream>
#include <cstring>

struct Struct {
	size_t size;
	char *array;
	
	Struct(){
		this->array = nullptr;
	}
	
	~Struct(){
		std::cout << "Will free on pointer: " << (int)this->array << std::endl;
		delete[] this->array;
	}
};

int main(){
	
	{
		Struct a;
		
		{
			Struct b;
			
			a.size = 64;
			a.array = new char[64];
			
			b = a;		
		} //b destructor will fire up, it will free its array, but since a.array has same pointer...
		
		//doing anything with a.array here may cause seg fault.
		
	} //a destructor will fire up

	std::cin.ignore();
	
	return 0;
}

In result you will get:

Will free on pointer: 7957984
Will free on pointer: 7957984

 

If we overload assignment operator in our Struct:

Struct & operator= (const Struct &a){
	delete[] this->array;
		
	this->size = a.size;
	this->array = new char[a.size];
	
	std::memcpy(this->array, a.array, a.size);
}

It will take care about our pointer so two instances won't share pointer but have their own set of data, so if one will be destructed, it wont wipe the others data.

(It wouldn't be wiped immediately, but further memory allocations might take freed memory and overwrite it)

 

After this fix output would be like this:

Will free on pointer: 11431464
Will free on pointer: 11431392

 

operator= is one thing of the three (or five [or O]) you would need to take care of if you will use them. More about it here:

https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)

[Unimportant]

On top off all that has already been said i'd like to point out the eof detection in the loop is wrong. As follows:

 

	while(!db_in.eof())		//We may not have reached EOF when we get here so we enter the loop...
	{
		db_in.get(temp.notes, '\n');	//We may reach EOF here, the data read is invalid, temp.notes contains invalid data...
	  
		Event.push_back(temp); //Invalid data saved...
	}

If '\n' is your delimiter anyway you should use getline and test it immediately:

	while(db_in.getline(temp.notes, x))	//Where x is the size of the notes buffer.
		Event.push_back(temp);

 

[Yamoto42]

38 minutes ago, Anand_Geforce said:

And what am I to do here anyway?

TLDR - use an std::string and istream::getline()...both of which you can look up the documentation for.

The hard way (ie. wanting to learn how it works under the hood)...and feel free to ignore this part, but I figured since this is all about learning why not take the opportunity...

 

You were going to run into the problem that @Mr_KoKa was describing, where they all pointed to the same memory.  This would ultimately bring up two design principles.

 

1) Resource Acquisition Is Initialization (RAII).

Basically your class should only manage that single resource, being the memory in the array.  It should be acquired (allocated, in this case) in the constructor, and freed in a destructor.  The single resource doesn't come into play here, but the acquisition and releasing does.  What this fixes is that each instance of the class will have it's own unique char array.  However, it does not yet fix the value.

 

2)  The rule of 3. (5 if you're using C++ 11)

If you ever have a case where you implement a constructor (ie. most of the time), you should also implement all of the following:

 

destructor

copy constructor

copy assignment operator

In the copy functions is where you would copy the contents of the array from the old instance to the new instance created within the vector.

 

C++11 added move constructor and assignment operators (hence 5 instead of 3), but those are for efficiency, not for necessary resource management.