Speicherleck

SeppJ

Manuelle Speicherverwaltung möglichst nicht selber machen, sondern die Standardbibliothek machen lassen. Falls man es unbedingt selber machen möchte: Ressourcen von Ressourcenhalterklassen verwalten lassen (RAII). Genau eine Ressource je Halter, niemals mehr. Fertige Ressourcenhalterklassen gibt es wieder in der Standardbibliothek. Falls man es unbedingt selber machen möchte: Bei der Implementierung der Ressourcenhalterklassen die wichtigsten Grundkonzepte beachten: RAII, Regel der großen Drei (oder neuerdings: Regel der großen Fünf), eventuell noch Copy&Swap-Idiom.

Die Ressource ist bei dir Speicher.

P.S.: Wieso malloc statt new? Wieso die static-Variablen? Wieso short als Datentyp? Insgesamt: Einige fragwürdige Designentscheidungen, die nach voreiliger (und falscher) Optimierung aussehen. Erst einmal sauber programmieren und testen, dann Mikrooptimierungen einbauen (und zwar solche, die auch funktionieren!).

knivil

Wenn C++, dann benutze doch erstmal die Typen der Standardbibliothek, bspw. std::list, std::vector. Es empfiehlt sich beispielsweise http://www.amazon.de/The-Programming-Language-Bjarne-Stroustrup/dp/0321563840/ref=sr_1_2?ie=UTF8&qid=1372076923&sr=8-2&keywords=c%2B%2B

Das ist die kryptischte und langsamste Implementierung eines Bigints, die ich je gesehen habe. Jedes Detail ist schlecht und das Grunddesign ist ein Performancekiller.

Das Speicherleck dürfte daher zustandekommen, dass du für " _twTlz temp; " vergessen hast, del() aufzurufen (bzw. aus unerfindlichen Gründen del() nicht im Destruktor aufrufst).

Obstsalat

Ich nutze malloc und free, weil wir das in der Uni grade so machen in C und ich das in c++ einfach so übernehme, damit ich das nebenbei etwas üben kann.

Der short int ist einfach daraus entstanden, dass ich den Speicherbedarf (wenn schon irgendwo n Speicherleck ist) klein halten will.

[quote="mmmalloc"]ganz viel malloc, niemals free - das kann, zumindest auf Dauer, nicht gut gehen.
quote]

ich habe eine funktion, die die ganze Liste durchgeht und jedes Element wieder freed. Wie gesagt, am ende der Main rufe ich für beide objekte die del() funktion auf.

Btw benutze ich malloc 2 mal im ganzen Code. Wo siehst du ganz viel malloc?

Ich würde halt scon gerne wissen, wo nun mein Leck ist. Das Programm läuft richtig, halt nur mit der unschönen Eigenschaft, dass es den Speicher füllt.

Obstsalat

heulsuse schrieb:

Das ist die kryptischte und langsamste Implementierung eines Bigints, die ich je gesehen habe. Jedes Detail ist schlecht und das Grunddesign ist ein Performancekiller.

Das Speicherleck dürfte daher zustandekommen, dass du für " _twTlz temp; " vergessen hast, del() aufzurufen (bzw. aus unerfindlichen Gründen del() nicht im Destruktor aufrufst).

ich hatte del() schon im destruktor, aber dann kommt es zur heapzerstörung.

wie kann ich temp mit del() löschen? Hab ich auch schon gedacht, aber ich muss das temp ja zurückgeben. wenn ich das danach noch del() will, kommt es ja gar nicht zum delete, weil return aus der funktion führt.

SeppJ

Obstsalat schrieb:

Der short int ist einfach daraus entstanden, dass ich den Speicherbedarf (wenn schon irgendwo n Speicherleck ist) klein halten will.

Und es ist völlig falsch. Du erreichst nicht im geringsten, was du damit vor hast und das Vorhaben geht am Problem vorbei.

ich habe eine funktion, die die ganze Liste durchgeht und jedes Element wieder freed. Wie gesagt, am ende der Main rufe ich für beide objekte die del() funktion auf.

Und zwar machst du es falsch. Mach es so wie vorgeschlagen dann kann so ein Fehler gar nicht auftreten, weil zu jedem malloc automatisch(!) ein passendes free kommen würde.

Btw benutze ich malloc 2 mal im ganzen Code. Wo siehst du ganz viel malloc?

Es geht nicht um das Vorkommen im Code, sondern darum, wie oft und worauf es aufgerufen wird. Was bei dir offensichtlich falsch ist, was kein Wunder ist, da du alles von Hand machst und selber nicht mehr durchblickst. Lass den Compiler für dich arbeiten! Ein Computer ist dafür gebaut, den Durchblick zu behalten.

Ich würde halt scon gerne wissen, wo nun mein Leck ist. Das Programm läuft richtig, halt nur mit der unschönen Eigenschaft, dass es den Speicher füllt.

heulsuse schrieb:

Das Speicherleck dürfte daher zustandekommen, dass du für "_twTlz temp;" vergessen hast, del() aufzurufen (bzw. aus unerfindlichen Gründen del() nicht im Destruktor aufrufst).

ich hatte del() schon im destruktor, aber dann kommt es zur heapzerstörung.

Dann hast du noch mehr Fehler drin. Du hast den Überblick verloren.

wie kann ich temp mit del() löschen?

Wie schon oft im Thread gesagt: Gar nicht. Es ist zwar möglich, aber tu es nicht! Du kommst dabei nur selber durcheinander. Nutze die Mittel der Sprache! Mit Smartpointern kannst du gar nicht mehr versehentlich Speicherlöcher bauen, bloß noch durch absichtlichen Missbrauch. Wenn du die Standardbibliothek benutzt, dann brauchst du dafür nicht einmal eine Zeile programmieren und der gesamte Code fällt auf wenige Zeilen zusammen.

Obstsalat schrieb:

heulsuse schrieb:

Das ist die kryptischte und langsamste Implementierung eines Bigints, die ich je gesehen habe. Jedes Detail ist schlecht und das Grunddesign ist ein Performancekiller.

Das Speicherleck dürfte daher zustandekommen, dass du für " _twTlz temp; " vergessen hast, del() aufzurufen (bzw. aus unerfindlichen Gründen del() nicht im Destruktor aufrufst).

ich hatte del() schon im destruktor, aber dann kommt es zur heapzerstörung.

wie kann ich temp mit del() löschen? Hab ich auch schon gedacht, aber ich muss das temp ja zurückgeben. wenn ich das danach noch del() will, kommt es ja gar nicht zum delete, weil return aus der funktion führt.

Ok, ich habe falsch geguckt.

Deine Schleife macht im Prinzip

for(int i=1;i!=1000000;i++) {
  ehemals_temp = twTlzAdd(zahl1, zahl2);
  zahl1 = ehemals_temp;
}

Das Problem ist, dass der Wert, der vorher in zahl1 stand, nicht gelöscht wird.

Der Fix wäre also:

for(int i=1;i!=1000000;i++) {
  ehemals_temp = twTlzAdd(zahl1, zahl2);
  zahl1.del();
  zahl1 = ehemals_temp;
}

Obstsalat

heulsuse schrieb:

Der Fix wäre also:

for(int i=1;i!=1000000;i++) {
  ehemals_temp = twTlzAdd(zahl1, zahl2);
  zahl1.del();
  zahl1 = ehemals_temp;
}

Das ist die Lösung. Es funktioniert. Vielen Dank für den Hinweis. Super wäre noch wenn jemand erklären könnte, was bei der Zuweisung passiert?

Ich dachte zahl1 = zahl2 würde zahl1 überschreiben, sodass kein speicher verloren geht. Anscheind ist das nicht so. Was passiert da?

Nathan

Der Zuweisungsoperator wird aufgerufen.
Der standardmäßig generierte macht eine komponentweise Zuweisung.
In deinem Fall: laenge = other.laenge; prev = other.prev; ...
Du musst aber vorher den Speicher freigeben (Regel der großen Drei!)
Aber nutze bitte Smartpointer oder RAII Klassen, dein Leben ist einfacher.
Oder am simpelsten std::vector, aber nicht dieses Gefrimmel.
Und selbst wenn du malloc() und free() im Kurs lernst, C++ new und delete machen das gleiche nur besser.

Obstsalat

Ok, ich habe jetzt den Zuweisungsoperator selbst definiert. Quasi wie der Defauflt nur mit dem del() drin.

void operator= (_twTlz zahl){

			del();
			laenge = zahl.laenge;

			anf = zahl.anf;
			pos = zahl.pos;
			end = zahl.end;
			prev = zahl.prev;
		}

Dann kann ich auch meine erste (vorher Fehlerhafte) Schleife verwenden. Ich bedanke mich für die viele Hilfe hier. cucu

Nathan

Gut, wenn du jetzt aber den Destruktor implementierst, so dass der del() aufruft crashst.
Weil jeder Kopie dann del() aufruft und Speicher zweimal freigegeben wird.