[c++] wycieki pamięci

Witajcie,

zauwazylem, ze przy wykonywaniu tego kodu:

CMantaRay3DSObject obj;
for(int i=0; i<pDoc->obj_v.size(); i++)
{
	obj = pDoc->obj_v[i];
	obj.draw();
}

aplikacja "puchnie" i nie bardzo wiem, gdzie lezy przyczyna. Domyslac sie jedynie moge, ze chodzi o przeciazony operator=
pDoc->obj_v jest wektorem obiektow CMantaRay3DSObject.
Klasa CMantaRay3DSObject wyglada mniej wiecej tak:

// mantaray3dsobject.h
class CMantaRay3DSObject
{
...
	CMantaRay3DSObject();
	~CMantaRay3DSObject();
	CMantaRay3DSObject(const CMantaRay3DSObject&);

	vertex_type	*vertex;
	polygon_type	*polygon;
	mapcoord_type	*mapcoord;

	void draw();

	CMantaRay3DSObject& operator=(const CMantaRay3DSObject& obj)
	{
		if(&obj != this)
		{
			...
			
			vertex = new vertex_type[size_vertices];
			polygon = new polygon_type[size_polygons];
			mapcoord = new mapcoord_type[size_mapcoord];

			memcpy(vertex, obj.vertex, size_vertices*sizeof(vertex_type));
			memcpy(polygon, obj.polygon, size_polygons*sizeof(polygon_type));
			memcpy(mapcoord, obj.mapcoord, size_mapcoord*sizeof(mapcoord_type));
		}
		return *this;
	}
};
// ======================
// mantaray3dsobject.cpp
CMantaRay3DSObject::CMantaRay3DSObject() : increase(5)
{
...
	vertex = new vertex_type[size_vertices];
	polygon = new polygon_type[size_polygons];
	mapcoord = new mapcoord_type[size_mapcoord];
}

CMantaRay3DSObject::CMantaRay3DSObject(const CMantaRay3DSObject& obj) : increase(obj.increase)
{
	...    
	vertex = new vertex_type[size_vertices];
	polygon = new polygon_type[size_polygons];
	mapcoord = new mapcoord_type[size_mapcoord];

	memcpy(vertex, obj.vertex, size_vertices*sizeof(vertex_type));
	memcpy(polygon, obj.polygon, size_polygons*sizeof(polygon_type));
	memcpy(mapcoord, obj.mapcoord, size_mapcoord*sizeof(mapcoord_type));
}

CMantaRay3DSObject::~CMantaRay3DSObject()
{
	delete []vertex;
	delete []mapcoord;
	delete []polygon;
}

Bede wdzieczny za wszystkie krytyczne ale rowniez konstruktywne uwagi :)

pozdrawiam
Jacek

W operatorze przypisania nie musisz alokować pamięci, bo już ją zaalokował kontruktor. Wystarczy przenieść zawartość (no chyba, że size_xxx może się róznić to trzeba przealokować pamięć)

        CMantaRay3DSObject& operator=(const CMantaRay3DSObject& obj)
        {
                if(&obj != this)
                {
                        memcpy(vertex, obj.vertex, size_vertices*sizeof(vertex_type));
                        memcpy(polygon, obj.polygon, size_polygons*sizeof(polygon_type));
                        memcpy(mapcoord, obj.mapcoord, size_mapcoord*sizeof(mapcoord_type));
                        increase = obj.increase;
                }
                return *this;
        }

No a w konstruktorze kopiującym wystarczy użyć operatora przypisania:

CMantaRay3DSObject::CMantaRay3DSObject(const CMantaRay3DSObject& obj) 
{
        ...   
        vertex = new vertex_type[size_vertices];
        polygon = new polygon_type[size_polygons];
        mapcoord = new mapcoord_type[size_mapcoord];
        
        *this = obj;
}

Dzieki za podpowiedz :) juz jest wszystko PRAWIE w porzadku ;)
Okazalo sie, ze gdy konstruktor kopiujacy wywoluje operator= wszystko jest w porzadku natomiast gdy wywolywany operator= jest w innym przypadku, aplikacja sie wysypuje. Jest to w miejscu:

CMantaRay3DSObject obj;
for(int i=0; i<pDoc->obj_v.size(); i++)
{
	obj = pDoc->obj_v[i];  // tu
	obj.draw();
}

czyli obiekt obj zostal utworzony (oraz jego tablice) z domyslnymi wartosciami, czyli tablica np vertex z obj moze miec inna dlugosc niz ta z pDoc->obj_v[i] i przy kopiowaniu:

memcpy(vertex, obj.vertex, size_vertices*sizeof(vertex_type));

moze sie wysypac.

Napisalem, ze jest PRAWIE w porzadku bo przy kopiowaniu tablicy polygon aplikacja sie wysypuje(czyli tak jak powinna ze wzgledu na rozna dlugosc tablicy) ale przy kopiowaniu tablicy vertex nie zauwaza zadnej nieprawidlowosci :]

zrobilem wydruk w operatorze= i

this->size_vertices = 5;
obj->size_vertices = 650;

this->size_polygons = 5;
obj->size_polygons = 650;

i badz tu madry czlowieku.

Może chodzi o rozmiar alokowanej pamięci w operatorze '='
brak paru elementów kodu ale chyba :
Zamiast :

        CMantaRay3DSObject& operator=(const CMantaRay3DSObject& obj)
        {
                if(&obj != this)
                {
                        memcpy(vertex, obj.vertex, size_vertices*sizeof(vertex_type));
                        memcpy(polygon, obj.polygon, size_polygons*sizeof(polygon_type));
                        memcpy(mapcoord, obj.mapcoord, size_mapcoord*sizeof(mapcoord_type));
                        increase = obj.increase;
                }
                return *this;
        }

powinno być :

        CMantaRay3DSObject& operator=(const CMantaRay3DSObject& obj)
        {
                if(&obj != this)
                {

                       delete [] vertex ;
                       delete []  polygon;  // jeśli konstruktor alokuje pamięć 
                                                    // o rozmiarze ? zwolnić ją
                       delete [] mapcoord ;

//------------------------Przydzielić o rozmiarae na podstawie obiektu wzorca 

                        vertex = new vertex_type[obj.size_vertices];
                        polygon = new polygon_type[obj.size_polygons];
                        mapcoord = new mapcoord_type[obj.size_mapcoord];

                        memcpy(vertex, obj.vertex, obj.size_vertices*sizeof(vertex_type));
                        memcpy(polygon, obj.polygon, obj.size_polygons*sizeof(polygon_type));
                        memcpy(mapcoord, obj.mapcoord, obj.size_mapcoord*sizeof(mapcoord_type));
                        increase = obj.increase;
                }
                return *this;
        }

vertex = new vertex_type[obj.size_vertices];
memcpy(vertex, obj.vertex, obj.size_vertices*sizeof(vertex_type));

Jeśli rozmiar size_vertices jest zmienny , musisz przydzielić ilość pamięci na podstawie
obiektu 'wzorcowego' (obj) .
W obiekcie do którego wykonujesz przypisanie może być ona inna .
Tylko nie widać właśnie z kawałka kodu co to jest to size_vertices i td...

Dzejo, tak mniej wiecej ostatecznie zrobilem po tym, co napisal Adf88.

Tylko zastanawia mnie to, dlaczego jedna z tych tablic kopiuje sie bez problemu, kiedy nie powinna w przypadku kiedy w operatorze= nie alokuje na nowo pamieci :)

PS. skad Wam sie wzielo increase = obj.increase; - increase to stala ;)

Jak na to patrzę, to jeszcze bardziej lubię Javę. ;)
A co do tematu, to polecam użycie std::vector i olanie tablic. Tymi wskaźnikami to się pochlastać mozna...

PS. skad Wam sie wzielo increase = obj.increase; - increase to stala

A nie wiem , nie zwróciłem uwagi , chyba 'kopiowanie i wklejanie ' coś zmącilo
... kod przekopiowałem od Adf88 , olać to ...

Krolik napisał(a)

Jak na to patrzę, to jeszcze bardziej lubię Javę. ;)
A co do tematu, to polecam użycie std::vector i olanie tablic. Tymi wskaźnikami to się pochlastać mozna...

takie rzeczy wlasnie sie robia jak ludzie uparcie nie korzystaja z tego co jest dostepne i wszystko od zera pisza.. to nie jest kwestia jezyka, tylko zlego sposobu nauczania

Sugerowałbym użycie keyword explicit, wyeliminuje to problemy, z operatorem przypisania, który wcale nie zostaje użyty, mimo deklaracji, lub rezygnacje z operatora przypisania.

Błędy w kodzie Adf88 polegają na wywoływaniu konstruktora kopiującego poprzez przypisanie *this = obj, i w następstwie tworzeniu nieskończonej pętli rekurencyjnej.

Gosciu, p i e r d o l y gadasz :]

@quetzalcoatl_
swego czasu robilem w innym projekcie porownanie co bedzie szybsze, czy zastosowanie kontenera vector, czy wlasny wskaznik na obiekty. Vector zamulal a mi zalezy na tym, zeby to dzialalo szybko.

yacooh napisał(a)

swego czasu robilem w innym projekcie porownanie co bedzie szybsze, czy zastosowanie kontenera vector, czy wlasny wskaznik na obiekty. Vector zamulal a mi zalezy na tym, zeby to dzialalo szybko.

Jeszcze trzeba nauczyć się efektywnie używać vector'a (i ogólnie STL'a) :>

0x666 napisał(a)

Jeszcze trzeba nauczyć się efektywnie używać vector'a (i ogólnie STL'a) :>

Byc moze masz racje, ale jezeli chce uzyc elementu z tablicy to mam do niego wskaznik, jezeli chce uzyc jakiegos elementu z wektora to musze go stamtad najpierw wyciagnac. Wtedy dla obiektu oczywiscie jest uruchamiany konstruktor kopiujacy itp itd. Przy wrzucaniu obiektu do wektora rowniez sa tworzone tymczasowe obiekty...

Moze moj tok myslenia jest bledny ale na razie wiecej sie ucze programowac niz programuje, jezeli tak mozna to ujac ;)

Jezeli chce uzyc jakiegos elementu z wektora to musze go stamtad najpierw wyciagnac. Wtedy dla obiektu oczywiscie jest uruchamiany konstruktor kopiujacy itp itd.

Nic z tych rzeczy. Od tego masz iteratory, wskaźniki tudzież referencje. Możesz też zrobić vector<klasa*> i wtedy pozbędziesz się kopiowania - lepsza byłaby opcja ze smart pointer'em (nie std::auto_ptr)... w boost jest chyba shared_ptr (???) Zawsze można napisać sobie własny.

No i najważniejsze: vector to nie jedyny kontener w STL'u :P