absturz wegen delete [] im destruktor?



  • hi leute.

    ich schreibe gerade ein kleines uebungsprojekt von mir um, von C nach C++.
    und da ich noch nie viel mit klassen in C++ gemacht habe, dachte ich mir dass das eine gute uebung waere.

    auch wenn ich mich nicht sonderlich mit C++ auskenne, so habe ich grundwissen der objektorientierung aus Java gelernt.

    klappt alles soweit ganz gut, mein einziges problem das ich nicht verstehe:

    ich habe eine klasse, CHeightmap, die sieht folgendermassen aus:

    #include "CHeightmap.h"
    
    float *hmbuffer;
    float *vertices;
    float *texcoords;
    
    CHeightmap::CHeightmap() {
    
        hmbuffer = new float[DIMENSION*DIMENSION];
        vertices = new float[DIMENSION*DIMENSION*2*3*3];
        texcoords = new float[DIMENSION*DIMENSION*2*3*2];
    }
    
    CHeightmap::CHeightmap(std::string inFileName) {
    
        loadFromFilename(inFileName);
    }
    
    void CHeightmap::loadFromFilename(std::string inFileName) {
    
    // ...
    
    }
    
    CHeightmap::~CHeightmap() {
    
        delete[] hmbuffer;
        delete[] vertices;
        delete[] texcoords;
    }
    

    instanziert wird das ganze etwa so:

    #include "CHeightmap.h"
    
    CHeightmap heightmap;
    
    int main(int argc, char *argv[]) {
    
        heightmap = CHeightmap("data\\textures\\hmap.tga");
    
        // main loop
    }
    

    problem ist: wenn ich die "delete"s im destruktor wegmache, geht alles. aber mit den deletes stuerzt das programm ab. komisch ist auch, dass von dieser klasse nur 1 objekt instanziert wird, und zum zeitpunkt des absturzes muesste dieses objekt noch existieren, also wieso wird hier (scheinbar) code aus dem destruktor ausgefuehrt?

    und wenn ihr generelle fehler/probleme/unfeinheiten seht, dann bitte ich um kritik 🙂

    vielen dank im voraus,

    ---loki



  • Hi,

    Allgemein: Es gibt keinen Grund, globale Variablen anzulegen.
    Hier führt das dazu, dass Du mehrere CHeightmap-Objekte hast.

    • Wenn das erste erzeugt wird, wird Speicher per new angelegt und Deine globalen Pointer verweisen darauf (Adresse abc).
    • Wird das zweite angelegt, verweisen die Pointer auf diese (Adresse xyz; der vorher allozierte Speicher ist verwaist => Speicherleck).
    • Wird ein Objekt abgeräumt, wird der Speicher an der Stelle xyz abgeräumt.
    • beim nächsten Abräumen, verweisen die Pointer immer noch auf xyz ... und er versucht nochmal, diesen Speicher abzuräumen.

    Lösung: Keine globalen Pointer verwenden, sondern "in die Klasse legen".

    1.) In der Klassendefinition:

    class CHeightmap {
        float *hmbuffer; // jedes Objekt hat seine eigenen Pointer
        float *vertices;
        float *texcoords;
    ...
    }
    

    2.) Auch heightmap braucht nicht global zu sein:

    int main(int argc, char *argv[]) {
    
        CHeightmap heightmap("data\\textures\\hmap.tga");
    
        // main loop
    }
    

    Dieser Unterschied schient marginal zu sein, ist es aber nicht. In Deiner Variante wird erst

    • Während der initialisierungsphase des Programms: Das globale heightmap per Default-Konstruktor erzeugt, (erstes Objekt)
    • Nach Einstieg in main(): ein temporäres CHeightmap wird erzeugt (mit String-Konstruktor) (zweites Objekt)
    • der Inhalt des temporären Objekts in heightmap kopiert (mittels operator=())

    In meinem Code wird einfach direkt heightmap mittels String-Konstruktor erzeugt.

    Ach ja: Dein String-Konstruktor weist gar nicht die Variablen zu, die Dein Default-Konstruktor zuweist.... da ist auch noch was faul.

    Noch ein Tipp: Du brauchst strings auch nicht "per Kopier" zu übergeben ... "string const&" geht mindestens genausogut. 😃

    Gruß,

    Simon2.



  • ok, klingt logisch. werde die implementierung mal umgestalten.



  • so, habe das ganze mal ueberarbeitet, aber ein paar probleme und fragen sind noch offen:

    - was meinst du mit "per Kopier"?

    - soll das heissen dass ein objekt, wenn es einfach nur deklariert wird, a la:

    "Classname object;"
    

    ,

    auch gleich initialisiert wird? weil das scheint ja so zu sein.

    problem hierbei ist weiterhin, dass ich die eine instanz von CHeightmap als privates Objekt der klasse CMainLoop habe, und es somit in der klassendefinition (.h-file) deklariere, da mehrere methoden von CMainLoop auf das objekt zugreifen muessen. aber wenn die vorige frage stimmt, dann heisst das dass es eben bei der deklaration auch gleich mit dem defaultkonstruktor initialisiert wird. denn wenn ich den wegmache, bruellt der compiler.

    gleichzeitig funktioniert aber folgendes

    CHeightmap heightmap("data\\textures\\hmap.tga");
    

    in einer methode, aber wenn ich das in der klassendefinition mache, meckert der compiler:

    CMainLoop.h:25: invalid data member initialization
    CMainLoop.h:25: (use `=' to initialize static data members)

    aber wenn ich es eben in der klasse definiere und erst in der methode initialisiere, entstehen (wenn ich das jetzt richtig verstanden habe) 2 objekte, und das ist ja eins mehr als erwuenscht.

    was mache ich hier falsch?

    danke nochmal,

    ---loki



  • - was meinst du mit "per Kopier"?

    Vergiss was du von Java kennst. 😃 C++ != Java. Bei C++ ist viel Context-Abhängig, das zu unterschiedlicher Semantik führt. Bsp.:

    void foo(int i)
    {
    }
    

    i ist hier innerhalb von foo eine neu definierte Variable bzw. Objekt. D.h. auch das gleiche Verhalten bei Strings:

    void foo(std::string i)
    {
    }
    

    Ein neues eigenständiges String-Objekt. Also, wenn du foo aufrufst, wird in beiden Beispielen ein neues int und string erzeugt. Die Objekte werden dann kopiert.

    Wenn du das nicht willst, mußt du die Parameter by-Reference definieren:

    void foo(int &i)
    {
    }
    

    Entsprechend auch bei string, siehe Simons2 Post!

    soll das heissen dass ein objekt, wenn es einfach nur deklariert wird, a la:

    Ja, so ist es. Auch hier vergleichbar mit int:

    int i; // neues int-Objekt auf dem Stack instanziert
    std::string s; // neues string-Objekt auf dem Stack instanziert
    float *f; // neues pointer-Objekt auf dem Stack instanziert
    
    s = "hallo"; // s ein Literal zuweisen
    f = new float; // f eine neue float-Adresse zuweisen
    

    Nichts mit new. new ist nur für dynamisch auf dem Heap erzeugte Objekte nötig.



  • ok, danke.... jetzt wird das ganze schon klarer.

    gibt es dann irgendwie eine moeglichkeit, fuer eine klasse ein privates objekt zu deklarieren, ohne es gleichzeitig zu instanzieren, sodass es spaeter in einer methode der klasse instanziert werden kann?



  • Ja, in dem du einfach Pointer benutzt und dann mit new instazierst. Wie gesagt, new ist ja zum dynamischen instanzieren gedacht... dynamisch in dem Sinne, das du es zu jeder Zeit machen kannst.

    class X
    {
        std::string str; // wird umgehend insanziert, bzw. ist ja die Instanz selbst
        std::string *str_ptr; // nur der Pointer, zeigt ins nichts.
    
    public:
        void create()
        {
            str_ptr = new std::string();   // dynamisch erzeugt.
        }
    };
    

    Muß aber kein user defined Typ sein, kann auch ein build-in Typ sein. Also auch float. Das macht in C++ keinen Unterschied, werden alle gleich behandelt.



  • super. vielen dank, hat mir sehr geholfen 🙂



  • Das delete im Destruktor und eine Nullinitialisierung im Konstruktor nicht vergessen. 🙂



  • nur noch eine letzte frage zum thema: nachdem ich alles umgestellt habe funktioniert auch alles super, nur musste ich alle methodenaufrufe von "object.methode()" nach "object->methode()" umstellen. ist das normal, und wenn: warum? denke mir dass das was mit objekt vs. pointer auf objekt zu tun hat, aber blicke nicht ganz durch.



  • Wenn du über einen Pointer zugreifst, benötigst du ->, das ist normal (-> ist eine Kombination aus Dereferenzieren und Member-Zugriff).



  • Es ist auch kein Luxus, sich erst mit if im Destruktor zu vergewissern, ob der zu löschende Zeiger überhaupt existiert (d.h. kein NULL-Zeiger ist!).
    Irgendwie so:

    class X{
    long* ptr;
    
    public:
    ~X(){
    if(ptr)
     delete[] ptr;
    }
    };
    

    Damit geht man auf Nummer sicher, dass man nicht an der Stelle NULL zu löschen versucht.



  • Das ist Käse. Man darf Nullpointer löschen, da passiert nämlich garnix 😉
    EDIT: Insofern ist das überflüssiger Luxus :p


Anmelden zum Antworten