Dynamiczna alokacja, wektor, problemy ze zwalnianiem pamięci

0

Witam społeczność, niestety od razu z problemem.
Piszę program mający imitować w pewien sposób dziennik szkolny. Od kilku dni zmagam się z potężnym problemem związanym z dynamiczną alokacją i zwalnianiem pamięci.
Może najpierw fragmenty kodu, w których prawdopodobnie leży problem.

Napis.h

class Napis
{
	char *imie;
	char *nazwisko;
public:
	Napis();
	Napis(const char *i, const char *n);
        Napis(const Napis& wzor);

	void Zmien_Imie(const char* nowe_imie);
	void Zmien_Nazwisko(const char* nowe_nazwisko);

	~Napis();
};

Napis.cpp

Napis::Napis()
{
	imie = new char[30];
	nazwisko = new char[30];
}

Napis::Napis(const char *i, const char *n)
{
	imie = new char[30];
	nazwisko = new char[30];
	strncpy(imie, i, strlen(i)+1);
	strncpy(nazwisko, n, strlen(n)+1);
}

Napis::Napis(const Napis& wzor)
{
	imie = new char[30];
	nazwisko = new char[30];
	strncpy(imie, wzor.imie, strlen(wzor.imie) + 1);
	strncpy(nazwisko, wzor.nazwisko, strlen(wzor.nazwisko) + 1);
}

void Napis::Zmien_Imie(const char* nowe_imie)
{
	delete[] imie;
	imie = new char[strlen(nowe_imie) + 1];
	strcpy(imie, nowe_imie);
}
void Napis::Zmien_Nazwisko(const char* nowe_nazwisko)
{
	delete[] nazwisko;
	nazwisko = new char[strlen(nowe_nazwisko) + 1];
	strcpy(nazwisko, nowe_nazwisko);
}

Napis::~Napis()
{
	delete[] imie;
	delete[] nazwisko;
}

main.cpp

 int main()
{
	vector <Napis> wek;

	for (int i = 0; i < 20; i++)
	{
		Napis a("Imie", "Nazwisko");
		a.Zmien_Imie("Nowe_Imie");
		a.Zmien_Nazwisko("Nowe_Nazwisko");

		wek.insert(wek.begin(), a);
		cout << "i = " << i << endl << endl;
	}
	system("PAUSE");
}

Ograniczyłem krąg poszukiwań do tych fragmentów kodu głównego projektu. Mają one symulować działanie tej części programu (mam całą funkcję losującą uczniów i dodającą ich w odpowiednim miejscu do wektora) i prawie na pewno tutaj gdzieś występuje błąd.
Jaki? Mianowicie, od szóstej iteracji zaczynają się dziać jakieś dziwne rzeczy, po których zawartość wektora pod zerowym indeksem się zwalnia i w jej miejsce pojawiają się krzaki z pamięci. W rezultacie, kilka iteracji dalej, program się wysypuje próbując zwolnić zwolnioną pamięć (słynny błąd 'triggered a breakpoint").

Rozumiem, że ma to jakiś związek ze specyfiką tego kontenera, który przy dodawaniu elementu na początek musi sam realokować pamięć, sam przesuwać wszystkie elementy i sam je wówczas zwalniać. Nie rozumiem tylko dlaczego pomimo poprawnych (domniemując) konstruktorów oraz destruktora, dzieją się takie cuda. Ale w takim razie dlaczego nie dzieje się to przy dodawaniu np. drugiego albo trzeciego elementu (przecież wtedy także pamięć po elementach jest zwalniana przy przesuwaniu), ale dopiero od szóstego?

Problem nie występuje, gdy użyję funkcji push_back lub wek.end() jako argument do 'insert', ale to jest w pewnym sensie zrozumiałe, ponieważ żadne elementy się wówczas nie przesuwają w wektorze. Gdy zakomentuję linie destruktora problem także znika, co również jest dość oczywiste w przypadku błędu dwukrotnego zwalniania tego samego obszaru pamięci, ale to z kolei generuje wycieki, które raczej nie są mile widziane.

Jeśli to ma w czymś pomóc, w głównym projekcie wektor ma reprezentować klasę uczniów i być złożony właśnie z obiektów klasy 'Uczen', które z kolei jako jedno z pól składowych przyjmują obiekt klasy 'Napis'. Pierwotny plan zakładał alokowanie całej klasy uczniów ręcznie. Gdy to zawiodło, przestawiłem się na wektory z myślą, że coś to pomoże. Niestety niewiele dało, bo błąd jest identyczny. Stąd poszukiwania dotarły aż do klasy 'Napis'.

Z góry dziękuję za wszelkie sugestie.
Pozdrawiam serdecznie :)

0

Korzystaj z std::string zamiast char*.

class Napis
sugeruje zupełnie inne przeznaczenie, niż ma faktycznie.
0

Spróbuj przeciążyć operator =

0
carlosmay napisał(a):

Korzystaj z std::string zamiast char*.

class Napis
sugeruje zupełnie inne przeznaczenie, niż ma faktycznie.

Wolałbym raczej rozwiązać problem związany z char* zamiast rzucać całą ideę i zmieniać na string. To część projektu na zaliczenie, a typ <string> na zajęciach jak do tej pory był odrzucany ze względu właśnie na to, że wykonuje sporą część pracy za programistę, a w całej nauce kładzie się raczej nacisk na zrozumienie, niż praktyczne podejście. Na dłuższą metę w pewnych przypadkach może nie mieć to większego sensu, ale nie mam na to za bardzo wpływu.

Co do nazwy, w programie ma trochę więcej zastosowań, tutaj są jedynie te, które dotyczą problemu, żeby nie śmiecić. Projekt nie jest bardzo duży, toteż nazwy nie będą się nieprawidłowo kojarzyć. W dodatku nazwę zawsze można zmienić.

xxx_xx_x napisał(a):

Spróbuj przeciążyć operator =

Wybacz, ale nie bardzo rozumiem, w którym miejscu owe przeciążenie należałoby zastosować i jaka ewentualnie byłaby z tego korzyść.

0
wek.insert(wek.begin(), a);

Tutaj masz wykonywany copy assignment (operator=), którego nie zdefiniowałeś, czyli jest brany domyślny. A to oznacza, że takie same wskaźniki są w obu obiektach, a to oznacza, że oba obiekty próbują wykonać delete[] na takich samych wskaźnikach w destruktorze.

W klasie Napis dodaj:

Napis &operator=(const Napis &rhs)
{
	delete[] imie;
	const size_t imie_size = strlen(rhs.imie) + 1;
	imie = new char[imie_size];
	strncpy(imie, rhs.imie, imie_size);

	delete[] nazwisko;
	const size_t nazwisko_size = strlen(strlen(rhs.nazwisko) + 1;
	nazwisko = new char[nazwisko_size];
	strncpy(nazwisko, rhs.nazwisko, nazwisko_size);

	return *this;
}

Pisane z palca także sprawdzaj.
Gdybyś używał std::string to problem by się nawet nie pojawił, bo std::string ma zdefiniowany copy assignment.

Poza tym, tutaj:

Napis::Napis(const Napis& wzor)
{
    imie = new char[30];
    nazwisko = new char[30];
    strncpy(imie, wzor.imie, strlen(wzor.imie) + 1);
    strncpy(nazwisko, wzor.nazwisko, strlen(wzor.nazwisko) + 1);
}

masz buffer overflow (crash w najlepszym przypadku) jeśli wzor.imie lub wzor.nazwisko ma więcej niż 29 znaków.

Tutaj zresztą to samo:

Napis::Napis(const char *i, const char *n)
{
    imie = new char[30];
    nazwisko = new char[30];
    strncpy(imie, i, strlen(i)+1);
    strncpy(nazwisko, n, strlen(n)+1);
}
0
#include <iostream>
#include <cstdlib>
#include <cstring>
using namespace std;

class my_string {
	size_t length = 0;
	char *data = nullptr;
	
	void set_data(char const *str) {
		length = strlen(str);
		if(data) {
			delete []data;
		}
		data = strcpy(new char[length+1], str);
	}
public:
	my_string(char const *str) {
		set_data(str);
	}
	
	my_string(my_string const &other): 
		my_string(other.c_str()) {}
	
	char const * c_str() const {
		return data;
	}
	
	my_string &operator=(char const *str) {
		set_data(str);	
		return *this;
	}
	
	my_string &operator=(my_string const &other) {
		set_data(other.c_str());
		return *this;
	}
	
	~my_string() {
		delete []data;
	}
	
	friend ostream &operator<<(ostream &out, my_string const &str) {
		return out << str.c_str();
	}
	
	friend istream &operator>>(istream &in, my_string &str) {
		return in; //jestem w trakcie oglądania dr. house, jeśli chcesz sie bawic w relokacje to powodzonka
	}
};

struct person {
	my_string name, surname;
	friend ostream &operator<<(ostream &out, person const &p) {
		return out << p.name << " " << p.surname;
	}
};

int main() {
	person p = {
		"Adam", "Kowalski"
	};
	cout << p << endl;
	p.surname = "Malinowski";
	cout << p << endl;
	p.name = my_string("Janeczek");
	cout << p << endl;
	return 0;
}

http://ideone.com/ywVPfr
output:

Adam Kowalski
Adam Malinowski
Janeczek Malinowski
0

Alokacja zupełnie nie potrzebna. Skoro już ma być char to ustaw na sztywno. Dla zwyczajnych imion i nazwisk w zupełności wystarczy.


char imie[ 16 ];
char nazwisko[ 32 ];

0
mwl4 napisał(a):
wek.insert(wek.begin(), a);

Tutaj masz wykonywany copy assignment (operator=), którego nie zdefiniowałeś, czyli jest brany domyślny. A to oznacza, że takie same wskaźniki są w obu obiektach, a to oznacza, że oba obiekty próbują wykonać delete[] na takich samych wskaźnikach w destruktorze.

Sądziłem, że wszystko stoi na konstruktorze kopiującym, czyli klasycznie szukałem błędów nie tam gdzie trzeba.

W klasie Napis dodaj:

Napis &operator=(const Napis &rhs)
{
	delete[] imie;
	const size_t imie_size = strlen(rhs.imie) + 1;
	imie = new char[imie_size];
	strncpy(imie, rhs.imie, imie_size);

	delete[] nazwisko;
	const size_t nazwisko_size = strlen(strlen(rhs.nazwisko) + 1;
	nazwisko = new char[nazwisko_size];
	strncpy(nazwisko, rhs.nazwisko, nazwisko_size);

	return *this;
}

Pisane z palca także sprawdzaj.
Gdybyś używał std::string to problem by się nawet nie pojawił, bo std::string ma zdefiniowany copy assignment.

Operator przypisania poradził sobie z problemem. Teraz już wszystko działa jak należy.

masz buffer overflow (crash w najlepszym przypadku) jeśli wzor.imie lub wzor.nazwisko ma więcej niż 29 znaków.

O problemie przepełnienia bufora wiem, założyłem, że jak zostanie mi trochę czasu to poprawię ten fragment na taki jaki ma być finalnie, czyli zarezerwuje się taka ilość pamięci jaka jest potrzebna, bo w programie będzie opcja zarówno wylosowania danych ucznia, jak i podania ich z klawiatury.

Tak czy siak, dzięki wielkie! :)

1 użytkowników online, w tym zalogowanych: 0, gości: 1