Delete in Destruktor

Hallo,

habe hier mal eine Anfängerfrage: Ich benötige eine Klasse mit einem char-Zeiger als Membervariable. Also zum Beispiel:

class Test
{
  private:
    char* bla;

  public:
   Test();
   ~Test();
};

Im Konstruktor Test() erzeuge ich eine neue Instanz des chars, also

Test::Test()
{
  bla = new char[10];
}

Bisher war ich auf dem Standpunkt, dass man diese Instanz spätestens im Destruktor wieder löschen muss:

Test::~Test()
{
  delete bla;
}

Leider bekomme ich da einen Speicherzugriffsfehler. Was mache ich falsch? Wäre für jeden Tipp sehr dankbar...

Erstens: Das heißt "delete**[]** bla;".

Zweitens: Bei Klassen mit eigener Speicherverwaltung solltest du immer Copy-Ctor und Zuweisung definieren, weil die Default-Version dir sonst den Speicher zerlegt.

CStoll (off) schrieb:

Erstens: Das heißt "delete**[]** bla;".

Hm, der Hinweis mit dem delete[] hat leider nicht geholfen.

CStoll (off) schrieb:

Zweitens: Bei Klassen mit eigener Speicherverwaltung solltest du immer Copy-Ctor und Zuweisung definieren, weil die Default-Version dir sonst den Speicher zerlegt.

Ich bin leider noch ziemlich unerfahren im Umgang mit c++. Wie sollte ein solcher Copy-Construktor denn in etwa aussehen?

Ich hab jetzt einfach mal mein Programm auf das nötigste gekürtz:

class ChordKey
{
public:
	unsigned char* key;
 	char* str() const;
	ChordKey();
	~ChordKey();
};

ChordKey::ChordKey()
{
	key = new unsigned char[20];
	memset(key, 0, 20);
}

ChordKey::~ChordKey()
{
	delete[] key;
}

char* ChordKey::str() const
{
	char* hex = new char[41];
	hex[0] = 0;
	char temp[4];
	for( uint i = 0; i < 20; i++ )
	{
		sprintf(temp, "%02x", key[i]);
		strcat(hex, temp);
	}
	hex[41] = '\0';
	return hex;
}

void print(ChordKey k)
{
	std::cout << k.str() << std::endl;
}

int main()
{
        for ( int i = 0; i < 10; i++ )
        {
                ChordKey a;
                print(a);
        }
}

Wenn man das so ausführt, wird folgender Fehler aufgeworfen:

*** glibc detected *** double free or corruption (fasttop): 0x0804a008 ***

Wenn man die Zeile delete[] key im Destruktor auskommentiert, läuft es zwar, aber ich habe dann ein memory leak...

Wäre super, wenn mir hier jemand weiterhelfen könnte.
Grüße
Inaniel

joomoo

Bei dem Funktionsaufruf von print wird die Klasse kopiert und diese Kopie löscht dann schon den Array. Das Original macht das dann nochmal und deswegen der Fehler.

Du machst dir entweder einen Kopierkonstruktor, der muss beim kopieren neuen Speicher reservieren, oder du benutzt einen Zeiger wie boost::shared_array der löscht das ganze erst wenn kein Zeiger mehr auf das Array zeigt.

Die einfachste Möglichkeit wäre std::string zu benutzen.

mfg.

.filmor

Wenn du print aufrufst, wird das ChordKey Objekt kopiert, k hat dann also den selben Pointer key wie das übergebende Objekt. Ist die Funktion nun beendet wird der Destruktor aufgerufen, der delete [] key ausführt. Später wird das allerdings für denselben Zeiger noch einmal gemacht, was die Laufzeitumgebung dir dann ankreidet. Das Programm sollte also so aussehen:

class ChordKey
{
public:
    const char* str() const;
    ChordKey();
    ChordKey (const ChordKey&); // Copy-Ctor
    ~ChordKey();
private:
    unsigned char* key; // Daten sind privat!
};

ChordKey::ChordKey()
{
    key = new unsigned char[20];
    memset(key, 0, 20);
}

ChordKey::ChordKey (const ChordKey& other)
{
    key = new unsigned char[20];
    for (int i = 0; i < 20; ++i)
        key[i] = other.key[i];
}

ChordKey::~ChordKey()
{
    delete[] key;
}

const char* ChordKey::str() const
{
    char* hex = new char[41]; // Hier ist ein RIESIGES Speicherloch!!!
    hex[0] = 0;
    char temp[4];
    for( uint i = 0; i < 20; i++ )
    {
        sprintf(temp, "%02x", key[i]); // Das ist kein C++, schau dir stringstreams an
        strcat(hex, temp);
    }
    hex[41] = '\0';
    return hex;
}

void print(const ChordKey& k) // hätte auch gewirkt
{
    std::cout << k.str() << std::endl;
}

int main()
{
    for ( int i = 0; i < 10; i++ )
    {
        ChordKey a;
        print(a);
    }
}

Vielen Dank für eure ausführlichen Antworten, .filmor und joomoo. Das hat mir sehr weitergeholfen und auch die Augen geöffnet für Probleme mit Pointern im Allgemeinen. Das memory leak in der str()-Methode hab ich nun so geflickt:

std::stringstream cur;
for ( int i = 0; i < 20; i++ )
{
	int z = key[i];
	cur << std::hex << z;
}
return const_cast<char*>(cur.str().c_str());

Auch hierfür nochmals vielen lieben Dank für den Hinweis.
Grüße
Inaniel

.filmor

Keine gute Lösung, die Rückgabe ist nur temporärer Natur (cur.str() gibt einen temporären String zurück). Was spricht gegen den C++ String? Damit sähe das ganze etwa so aus:

std::string ChordKey::str ()
{
    std::ostringstream cur;
    for (int i = 0; i < 20; ++i)
        cur << std::hex << static_cast<int> (key[i]);
    return cur.str ();
}

Wofür wird str () verwendet? Falls es für Ausgabeoperationen ist sollte auch eine Überladung des <<-Operators für ostream in Betracht gezogen werden.

Hallo,

.filmor schrieb:

Wofür wird str () verwendet?

Die Methode soll die interne 20 byte Repräsentation des keys in eine 40-stellige Hexadezimalzahl Umrechnen, die dann in der Simulation an diversen Stellen angezeigt wird.

.filmor schrieb:

Keine gute Lösung, die Rückgabe ist nur temporärer Natur (cur.str() gibt einen temporären String zurück). Was spricht gegen den C++ String?

Warum ist denn die Rückgabe nur temporär? Eigentlich spräche nichts gegen C++-Strings, aber wenn ich so wie unten implementiere, fehlen mir bei manchen keys am Ende mehrere Hex-Ziffern.

std::string ChordKey::str ()
{
    std::ostringstream cur;
    for (int i = 0; i < 20; ++i)
        cur << std::hex << static_cast<int> (key[i]);
    return cur.str ();
}

CStoll

Inaniel schrieb:

Ich bin leider noch ziemlich unerfahren im Umgang mit c++. Wie sollte ein solcher Copy-Construktor denn in etwa aussehen?

Er sollte ein eigenes Array mit new anlegen und den Inhalt deines Arrays dort reinkopieren. (Alternativ kannst du auch Referenzzählung implementieren und immer mitschreiben, wieviele Objekte gerade deinen Puffer verwenden.

Warum ist denn die Rückgabe nur temporär?

cur wird am Ende der Funktion wieder gelöscht und gibt dann auch den verwendeten Puffer wieder frei.

...fehlen mir bei manchen keys am Ende mehrere Hex-Ziffern.

Geh mal in den Debugger, um sie zu suchen