Copy Constructors

[wasabi]

So, I'm currently completely over my head creating a program that is, well, completely over my head. However, I've been doing just fine (ish) until I suddenly got the following error.

#include <iostream>
#include <string.h>
#include "iup.h"
#include "iupkey.h"
using namespace std;

struct elem
{
	unsigned int* elementos;
	unsigned int num;
	elem();
	elem(const elem &obj);
	elem& operator=(const elem &obj);
	~elem();
};
elem::elem(const elem &obj)
{
	unsigned int i;
	num=obj.num;
	elementos=new unsigned int[obj.num];
	for(i=0;i<num;i++)
		elementos[i]=obj.elementos[i];
}
elem::elem()
{
	num=0;
	elementos=NULL;
}
elem::~elem()
{
	delete[] elementos;
}
elem& elem::operator=(const elem &obj) //assignment operator definition
{
	unsigned int i;
	if (this != &obj) 
	{
		delete [] elementos;
		num=obj.num;
		elementos=new unsigned int[obj.num];
		for(i=0;i<num;i++)
			elementos[i]=obj.elementos[i];
	}
	return *this;
}
elem elementos(unsigned int falha, int TXT) //TXT
{
	char test[10];
	elem elem;
	unsigned int i;
	test[0]='\0';
	while(strcmp("%",test))
		fscanf_s(txt," %s",test,150);
	fscanf_s(txt," %d",&elem.num);
	elem.elementos=new unsigned int[elem.num];
	for(i=0;i<elem.num;i++)
		fscanf_s(txt," %d",&elem.elementos[i]);
	return elem;
}


int main(int argc, char* argv[])
{	
	fopen_s(&txt,"C:\\Documents and Settings\\pnacht\\My Documents\\Visual Studio 2008\\Projects\\busca\\busca\\modelo_45_1.txt","r");
	elem elem=elementos(5,0);
	return 0;
}

The file itself is of the form

Falha 1
% 60

1 86 101 180 198 272 320 364 420 451
528 556 620 654 708 766 797 879 924 964
1048 1054 1139 1160 1207 1248 1266 1307 1360 1401
1424 1468 1506 1531 1567 1613 1620 1639 1704 1711
1724 1739 1766 1803 1818 1826 1830 1833 1838 1847
1865 1888 1937 1942 1947 1956 1970 1973 1984 2024

So, basically, elementos() moves to the '%', reads the number of elements in the list and then tosses each element into the array within the elem class. This was working just fine before, but I've clearly done changed something to the copy constructor, since it now calls an access violation when transfering obj.num's value to this->num.

This is far from the whole code (1500 lines and counting!), but I've compiled and run it, and it gives the same error as the real thing.

[kempofighter]

Your copy constructor and assignment operator look fine to me and elementos is properly returning by value. Have you used the debugger? It looks like you are using the secure, windows c i/o functions so you must have the ability to step into the software using the visual studio debugger. I wonder if some problem with the file i/o is causing some undefined behavior. I don't see why you would get an access violation while the copy construction is happening in this case. I don't use the windows libraries or C I/O libraries at all so I can't really help you with that. Let us know if you find anything out using the debugger.

[PanGalactic]

I don't see where you give us the error message you are now getting.

Your fscanf_s() call is unsafe. The length parameter should match the size of your buffer. It doesn't. Your buffer is 10 characters and you allow fscanf_s() to fill 150.

[guestgulkan]

elem elementos(unsigned int falha, int TXT) //TXT
{
	char test[10]; //This is only 10 chars 
	elem elem;
	unsigned int i;
	test[0]='\0';
	while(strcmp("%",test))
		fscanf_s(txt," %s",test,150); //but we read 150 chars - we will end up with array overflow
	fscanf_s(txt," %d",&elem.num);
	elem.elementos=new unsigned int[elem.num];
	for(i=0;i<elem.num;i++)
		fscanf_s(txt," %d",&elem.elementos[i]);
	return elem;
}

We have a mismatch between the the size of the test buffer and the number of chars read by fscanf_s function. The value passed to fscanf_s need to be the same size of smaller than the buffer - NOT greater.


BY the way, that function is not written out correctly - shouldn't it be something like:
elem elementos(unsigned int falha, FILE* txt) and the main function isn't quite right either.

[wasabi]

Woops on the array overflow. I had test[150] before but noticed that was way too big for its purpose, so I brought it down to 10. However, given how the error I'm getting is in the copy constructor (I saw this by using Visual's step-by-step debugger).

And the int TXT thing is merely a differentiator. The real program has a triple overflow of elementos() (the elements can be picked up through a .txt file (this case), a .dat file or through manual input). Meanwhile, FILE* txt is a global variable. Last I checked, C++ differentiates between upper and lower case. This part of the program is the algorithm that goes with a UI, which requires an absurd amount of global variables.

I can't test to see if the array overflow is the source of the problem because I don't have the code in front of me right now, but I doubt it.

On a slight tangent... can't one almost make all variables declared in main as global? After all, if it's in main, it's never going to be out of scope until the end of the program anyways... (Obviously, iterators or variables with absolutely no inter-function use would just confuse the program).

[guestgulkan]

I can't test to see if the array overflow is the source of the problem because I don't have the code in front of me right now, but I doubt it.

I bet you it is.
To prove a point - just change the line
elem elem=elementos(5,0);
to elementos(5,0); (in other words just make the function call - don't do the assignment) and I bet you it will crash.

[wasabi]

Damn you guestgulkan and your correctness.

Indeed, it worked.

Still don't understand why the error only showed up half-way through the copy-constructor, though... :S

[guestgulkan]

This sort of problem (copying stuff from one place to another) cannot be be picked up by the compiler during the compiling phase. It is a run-time error and occurs when the program is actually running.

The error isn't happening during the copy constructor or assignment operator.

This statement:
elem elem=elementos(5,0); is actually a two part statement.
before the compiler can call the = assignment operator (or possibly the copy constructor), it must deal with the expression on the right hand side elementos(5,0).
The result of this will then be used in the assignment or copy constructor (whichever way the compiler decides to play it).