frage zu heap allozierung ...

hallo ich haette folgende frage zu folgendem code-fragement und
wuesste gerne eure meinung:

int* Foo() const
{
  int *arr = new int[5];
  // do something with arr here ..
  return arr;
}

int main(int argc, char *argv[])
{
  int *arr = Foo();
  return 0;
}

was mir persoenlich auf- / missfaellt ist folgendes:

Die lokale variable arr wird aufm heap angelegt aber ich sehe nirgends ein delete hierfuer (memory leak ??). Sehe ich das richtig oder gibt es fuer Standard-Datentypen sowas wie einen impliziten Array-Destruktor ??

Kommentare hierzu waeren sehr schoen.

Danke.

Bashar

pepe75 schrieb:

Die lokale variable arr wird aufm heap angelegt aber ich sehe nirgends ein delete hierfuer (memory leak ??).

Es fehlt in der Tat ein delete.

Übrigens wird arr nicht auf dem Heap, sondern auf dem Stack angelegt. Das ist eine normale Zeigervariable. Sie wird mit der Adresse eines Arrays, das auf dem Heap angelegt wurde, initialisiert.

Sehe ich das richtig oder gibt es fuer Standard-Datentypen sowas wie einen impliziten Array-Destruktor ??

Interpretationssache -- "sowas wie einen Destruktor" gibt es schon, aber was würde der hier helfen? Er wird ja nicht aufgerufen, das delete fehlt.

Richtig, hier gibt es einen Memory-Leak. Vernünftiger Ersatz wäre z.B. std::vector, damit wird der Speicher automatisch wieder aufgeräumt:

#include <vector>

std::vector<int> Foo() {
    std::vector<int> arr( 5 );
    return arr;
}

int main(int argc, char *argv[])
{
    std::vector<int> arr = Foo();
    return 0;
    // Beim Verlassen der Funktion wird der Speicher automatisch freigegeben
}

Und das const hinter der Funktion ist falsch. const können nur Methoden sein. Was soviel heißt, wie dass die Klassen-Member nicht verändert werden können/dürfen/sollen. Damit sollte auch klar sein, warum es an der Stelle falsch ist und das auch nur nicht statische Methoden mit einem const deklariert werden können.
Das können/dürfen/sollen soll ausdrücken, dass es Ausnahmen gibt.

@Paul Mueller:
Jo, das const steht deshalb da, weil ich nicht die ganze klasse in den code pasten wollte :)))

@Bashar: die lokale variable arr ist eine stack-variable (wird bei verlassen von Foo(..) vernichtet), ich meinte aber den speicher auf den arr zeigt. der wird nicht freigegeben, oder?!

danke fuer die antworten.

HighLigerBiMBam

Die Rückgabe von Pointern in Funktionen von Klassen ist nicht sonderlich guter Stil. Jemand der diese Funktion verwendet wird nie wissen können, ob er den Pointer aufräumen muss, oder ob der Pointer aufgeräumt wird. Entweder er rät richtig, oder er verursacht undefiniertes Verhalten.

@HighLigerBiMBam :
das genau war auch meine schlussfolgerung als ich den code sah (ist fremcode).
mir ging es eigentlich nur darum zu wissen, ob meine vermutung dass ein memory leak entsteht richtig ist.
trotzdem danke.

Der "Destruktor" eines rohen Zeigers tut nichts.

pepe75 schrieb:

das genau war auch meine schlussfolgerung als ich den code sah (ist fremcode).
mir ging es eigentlich nur darum zu wissen, ob meine vermutung dass ein memory leak entsteht richtig ist.

Für solche Fälle ist eine vollständige und aussagekräftige Dokumentation gut. Das nächste Problem, welches du mit diesem Code bekommst ist, wieviele Elemente hat den nun dein allokierter Speicherbereich? An der Stelle wirst du um eine gescheite Dokumentation oder Kapselung nicht mehr rum kommen. Das man den Speicher frei geben muss, ließe sich zur Not noch ermitteln. Auch womit. Wenn es dafür keine extra Funktion gibt. Aber spätestens wenn auch noch ein eigenes Speichermanagement implementiert ist hätte der Autor doch ein wenig dokumentieren sollen.
Code der keine/wenig Dokumentation hat, weil er sich selbst dokumentiert, halte ich aber prinzipiell für eine gute Idee. Sogar wesentlich sinnvoller als eine Doku zu schreiben.

HighLigerBiMBam

Selbst mit gute Doku werden bei einem solchen Konzept leicht und häufig Fehler gemacht. Es ist einfach zu vermeiden.

weicher

Es ist einfach zu vermeiden.

Das ist mir eine zu starke Einschränkung, Pointer sind auch in C++ ein legitimes Werkzeug. Man kann es so machen, dass klar ist wie man damit umgehen muss.

Ich mach es zB. immer so:

obj* Object ();

oder

obj* GetObject ();

der gelieferte Pointer muss nicht entsorgt werden.

obj* CreateObject ();

der gelieferte Pointer muss mit

obj->Destroy ()

entsorgt werden.

Herzliche Grüsse
Walter

HighLigerBiMBam

Spätestens wenn mehrere Leute dran arbeiten ist es eben nicht mehr so klar, wie es vielleicht einmal sein sollte. Um mal der Chaostheorie zu folgen wird es sicher Probleme geben. Aber wie dem auch sei, spätestens wenn jemand auf den Gedanken kommt deine

obj* GetObject ();

schön zu verpacken in seine eigene Funktion und wieder das Objekt zurückgibt stehst du wieder vor dem Dilemma. Löschen oder Nicht löschen? Und du musst umständlich in der Doku nachlesen. Durch das fehlende Highlight von new und delete sehen heapobjekte aus als würden sie bad-ptr sein oder auch der eventuelle Gebrauch von delete obj anstelle von obj->Destroy() kann auch zu nicht nachvollziehbaren Fehlern führen.

Mir fällt auch schlagartig kein einfaches Beispiel ein, wo dies unbedingt notwendig essentiell so gemacht werden muss. Ich möchte nicht sagen, dass man es komplett verteufeln oder verbieten sollte, allerdings vermeiden sollte. Andere Rückgabewerte sind einfach klarer und wartungsärmer.

Nexus

HighLigerBiMBam schrieb:

Aber wie dem auch sei, spätestens wenn jemand auf den Gedanken kommt deine
obj* GetObject ();
schön zu verpacken in seine eigene Funktion und wieder das Objekt zurückgibt stehst du wieder vor dem Dilemma.

Natürlich kann man einen Wrapper schreiben, der die konsistente Semantik wieder durcheinander bringt. Aber davor kann man sich nicht schützen, auch bei Smart-Pointern nicht.

HighLigerBiMBam schrieb:

[...] oder auch der eventuelle Gebrauch von delete obj anstelle von obj->Destroy() kann auch zu nicht nachvollziehbaren Fehlern führen.

Wer einfach so delete auf irgendeinen Zeiger anwendet, hat auch verdient, dass ihm die Anwendung um die Ohren fliegt. Das ist doch kein Grund gegen Zeiger.

Aber natürlich kann man das Risiko verringern, z.B. durch Benutzung von auto_ptr oder eines besser implementierten verschiebbaren Smart-Pointers. Aber man nimmt dem Benutzer damit halt auch Freiheit. Jedoch finde ich ein Interface schlecht, das den Benutzer zu manueller Speicherverwaltung zwingt. Das führt garantiert früher oder später zu Problemen. Man sollte mindestens noch eine RAII-Möglichkeit anbieten, wie z.B. ein Objekt, das automatisch Destroy() aufruft.

Ausserdem fände ich eine globale Freigabefunktion im Hinblick auf die globale Erzeugungsfunktion symmetrischer als eine Memberfunktion.

weicher

Mir fällt auch schlagartig kein einfaches Beispiel ein, wo dies unbedingt notwendig essentiell so gemacht werden muss.

Es geht auch nicht um die einfachen Beispiele, da bin ich vollkommen mit Dir einverstanden. Aber wir haben zB. Maschinen mit ganz unterschiedlichen Handlingsystemen von manueller Zuführung bis zu SPS gesteuerten Handlern. Da liest die CreateObject() funktion des virtuellen Handlers die Konfiguration (XML Datei) und erzeugt den richtigen Handler.

Natürlich ist das alles schön gekapselt und man arbeitet im Programm mit Referenzen, aber im Kern gibts die CreateObject () und die Destroy () Funktion.

Herzliche Grüsse
Walter

weicher

Ausserdem fände ich eine globale Freigabefunktion im Hinblick auf die globale Erzeugungsfunktion symmetrischer als eine Memberfunktion

Nur das Objekt selber weiss wie es sich zerstören muss.

Man kann da mit viel Aufwand sicher etwas bauen, aber wir Programmierer haben ja ein Gehalt

Herzliche Grüsse
Walter

HighLigerBiMBam

Meine Aussagen haben sich mehr auf public-Methoden bezogen. Bei privaten Methoden die innerhalb einer Klasse sich selbst verwalten habe ich wenig gegen Zeiger.

Die Frage des Autors sah für mich implizit anders aus, dass er eine Klasse verwendet die ein Heap-Objekt nach außen gibt, was ich nicht befürworten kann und will^^

Nexus

weicher schrieb:

Nur das Objekt selber weiss wie es sich zerstören muss.

Das ist unlogisch. Das Objekt wurde von einer anderen Funktion erzeugt. Nur die Funktion weiss, wie die Zerstörung zu erfolgen hat, sofern du nicht irgendwelche Querabhängigkeiten hast.

Beispiel:

Object* Factory1()
{
    return new Object();
}

Object* Factory2()
{
    void* memory = std::malloc(sizeof(Object));
    new (memory) Object();
    return static_cast<Object*>(memory);
}

Object* Factory3()
{
    myObjectList.push_back(Object());
    return &myObjectList.back();
}

Was schreibst du nun in den Destruktor? Ihr werdet euch zwar auf eine Allokationsmethode festgelegt haben. Aber dass diese nur das Objekt selbst kennt, kann nicht sein, da es sich nicht selbst erzeugen kann (und die Deallokation symmetrisch zur Allokation sein muss).

Insofern denke ich eher, dass die Symmetrie sogar weniger Aufwand mit sich brächte, weil sich die Allokations- und Deallokationsfunktion im gleichen Modul befinden und du bei einer neuen Implementierung nicht an verschiedenen Stellen Code ändern musst.

weicher

Ich habe natürlich immer mein Framework im Kopf und da ist das releasen des Speichers der kleinste Teil beim zerstören eines Objekts.

Da dieser Thread aber nur die heap allozierung behandelt bin ich damit eigentlich schon off Topic. Ich denke nur, man kann das nicht so isoliert betrachten, Objekte sind IMHO viel mehr als nur allozierter Speicher.

Als ich diesen Thread gesehen habe war ich am implementieren eines Fensters welches in eine DLL ausgelagert ist und da ist die Kombination von Create() und obj->Destroy() die einfachste Möglichkeit das Dingens wieder sauber loszuwerden.

Ich weiss, das ist jetzt etwas speziell. Aber das ist eigentlich genau der Punkt. Auf die Pauschale ablehnung von Sprachmitteln (über)reagiere ich etwas allergisch

Kein Schreiner wird je Sagen "Verwende NIE und NIMMER die Kreissäge, damit kannst Du Dir die Finger absägen!". Warum Programmierer genau das immer wieder tun ist mir ein Rätsel. Was passiert denn schon schlimmes wenn mal ein Pointer ins Nirwana zeigt oder man ein Memoryleak einbaut.

Das Programm stürzt beim ersten vernünftigen Test ab... Es gibt keine Verletzen und es fliesst kein Blut und man hat alle Werkzeuge um den Fehler zu beheben.

Herzliche Grüsse
Walter

krümelkacker

weicher schrieb:

Man kann es so machen, dass klar ist wie man damit umgehen muss.

Ich mach es zB. immer so:
obj* Object ();
oder
obj* GetObject ();
der gelieferte Pointer muss nicht entsorgt werden.
obj* CreateObject ();
der gelieferte Pointer muss mit
obj->Destroy ()
entsorgt werden.

Ich stimme Dir zumindest in dem Punkt zu, dass man sich überlegen sollte, wie man Funktions-Deklarationen bzgl "Besitz von Objekten" selbstdokumentierent gestalten kann. In C++0x würde ich das so machen:

Rohe Zeiger: Kein Ownership-Transfer (per Konvention).
unique_ptr: Ownership-Transfer (ergibt aus dem Verhalten von unique_ptr).

Also, ohne Ownership-Transfer:

clazz* get_obj();
void set_obj(clazz*);

und mit Ownership-Transfer:

unique_ptr<clazz> get_obj();
void set_obj(unique_ptr<clazz>);

Gruß,
kk

Bashar

weicher schrieb:

Was passiert denn schon schlimmes wenn mal ein Pointer ins Nirwana zeigt oder man ein Memoryleak einbaut.

Das Programm stürzt beim ersten vernünftigen Test ab...

Wenn das deine Einstellung ist, hast du in der Diskussion wohl nicht soviel verloren. Durchaus legitim, aber das sehen viele anders, und mit Sicherheit sehr viele von denen, die C++ heute noch die Treue halten (und nicht nur Legacy-Code warten müssen.)
Wie du ein Speicherleck durch einen Absturz finden willst, musst du aber noch verraten.