Destruktor wird scheinbar grundlos aufgerufen

Jeremin

Moin moin

In meinem aktuellen Projekt hat sich grad ein für mich nicht erkennbarer Fehler eingeschlichen, und zwar wird ein Destruktor ohne delete oder ähnliches aufgerufen (aus einer anderen Klasse heraus)

betroffene Funktion:

int CPatching::loadPatchingFile(string path){
    //filenamen erzeugen
    string patchPath = path + "\\Patch.ptch";
    string ficturePath = path + "\\Fictures.fict";

    int returnValue = 0;

    //kopieren
    char * patchPathChar = new char[patchPath.size()+1];
    char * ficturePathChar = new char[ficturePath.size()+1];
    strcpy(patchPathChar, patchPath.c_str());
    strcpy(ficturePathChar, ficturePath.c_str());

    patchSave myPatchSaveFile;

    //Patchinfos laden
    ifstream fin(patchPathChar, ios::in | ios::binary);
    if (fin.is_open()){
        fin.read((char*) &myPatchSaveFile, sizeof(patchSave));

        //Dimmer-Infos kopieren
        for (int i=0; i<NDIMCHANNELS; i++){
            this->dimCh[i] = myPatchSaveFile.dimCh[i];
            this->dimChAlias[i] = myPatchSaveFile.dimChAlias[i];
        }
        //Ficture-Infos kopieren
        for (int i=0; i<NFICTURES; i++){
            this->fictureAlias[i] = myPatchSaveFile.fictureAlias[i];
        }

        for (int i=0; i<512; i++)
            this->available[i] = myPatchSaveFile.available[i];
    }
    else
    {
        returnValue -= 1;
    }

    CFicture aFicture;

    //alle gespeicherten Fictures laden
    fin.close();
    if (returnValue == 0){
        fin.open(ficturePathChar, ios::in | ios::binary);
        if (fin.is_open()){
            for (int i=0; i<NFICTURES; i++){
                if (myPatchSaveFile.ficturesUsed[i]){
                    fin.read((char*) &aFicture, sizeof(CFicture));
                    if (fictures[i] != NULL)
                        delete fictures[i];
                    fictures[i] = new CFicture(aFicture);
                }
                else{
                    if (fictures[i] != NULL)
                        delete fictures[i];
                    fictures[i] = NULL;
                }
            }
        }
        else
            returnValue -= 2;
    }

    delete [] patchPathChar;
    delete [] ficturePathChar;

    return returnValue;
}

Am Ende dieser Funktion wird der Destruktor von CFicture aufgerufen.

CFicture::~CFicture(){
    if (nullOutputValue!=NULL)
        delete nullOutputValue;
}

Also etwas genauer: Wenn ich mit dem Debuger bei

return returnValue

angekommen bin, springt das Programm zu Destruktor von CFicture.

Hier noch das Headerfile von CPatching (falls es benötigt werden sollte:

#ifndef CPATCHING_H
#define CPATCHING_H

#include <string>
#include <vector>
using namespace std;

//CONST
const short NFICTURES = 60;
const short NDIMCHANNELS = 120;

//forward declaration
class CFicture;
class CDMX;
class CCue;
class CMemory;
class CChaser;
class CPreset;

struct patchSave{
    bool ficturesUsed[NFICTURES];
    int dimCh[NDIMCHANNELS];

    bool available[512];
    char fictureAlias[NFICTURES][128];
    char dimChAlias[NDIMCHANNELS][128];
};

/**
 * @brief Diese Klasse managed das ganze Patching, also wo welches Ficture, welcher Dimmerkanal ist (bzw. welche Adresse es/er hat)
 *\details Im Prinzip besteht CPatching aus einem Array von Pointern auf CFicture, um somit Zugriff auf diese zu erhalten.
 *Ausserdem kontrolliert sie auch alle Dimmerkanaele.
 */
class CPatching{
public:
    CPatching();
    ~CPatching();

    //playback
    void playMemory(CMemory* memory, int fade = 100);
    void playChaser(CChaser* chaser, int fade = 100);
    void playPreset(CPreset* preset);

    void stopMemory(CMemory* memory);

    //INPUT
    //originaler Input
    void origDimChChange(int dimCh, int value);
    void origGroupChange(int ficture, int attribute, int value);

    //cue Input
    void realDimChChange(int dimCh, int value);

    //PATCHING

    //channels available?
    bool channelsAvailable(int startAdress, int count = 1, int offset = 1, int channels = 1);
    void refreshChannelsAvailable();

    //ficture patch
    CFicture * newFicture(const short ficture);
    int deleteFicture(const short ficture);

    CFicture * getFicture(const short ficture) {return this->fictures[ficture];}
    CFicture ** getFictures(){return this->fictures;}

    //dim patch
    int getDimCh(const int channel){return this->dimCh[channel];}
    int* getDimCh(){return this->dimCh;}

    void setDimCh(int patchCh, int DmxCh){this->dimCh[patchCh] = DmxCh;}
    void deleteDimCh(int channel){this->dimCh[channel] = -1;}

    //aliase
    void setFictureAlias(string alias, int ficture);
    string getFictureAlias(int ficture);
    string* getFictureAlias();

    void setDimChAlias(string alias, int channel);
    string getDimChAlias(int channel){return this->dimChAlias[channel];}
    string* getDimChAlias(){return this->dimChAlias;}

    //DMX
    int getDmxStatus() {return dmxStatus;}  

    //LOAD & SAVE
    int savePatchingFile(string path);
    int loadPatchingFile(string path);

    //SONSTIGES
    //get values
    void getDimChValues(int (&values)[NDIMCHANNELS]);

private:    
    //dim input
    void setDimChValue(int dimCh, int value);

    //hilfsfunktionen
    int valueInRange(int value);

    CFicture **fictures;
    int dimCh[NDIMCHANNELS];

    //originaler Output - ohne Cues
    short origDimChValues[NDIMCHANNELS];
    short origFictureHtpValues[NFICTURES];

    //realer Output - mit Cues
    short realDimChValues[NDIMCHANNELS];
    short realFictureHtpValues[NFICTURES];

    //aktive Cues
    vector<CCue *> activeCues;
    short cueControlledDimChs[NDIMCHANNELS];
    short cueControlledFictures[NDIMCHANNELS];

    //dmxout
    CDMX *dmxOut;
    int dmxStatus;

    bool available[512];
    string fictureAlias[NFICTURES];
    string dimChAlias[NDIMCHANNELS];
};

#endif //CPATCHING_H

Ich habe jetzt den Nachmittag damit verbracht, zu versuchen, herauszubekommen, wieso der Destruktor aufgerufen wird, leider bis jetzt ohne Ergebnis :S
Danke schonmal für die Hilfe, falls ihr noch weiteren Sourcecode benötigen solltet sagt Bescheid.

PS: IDE ist QT, compiliert wird das ganze mit MinGW, falls das von Belang sein sollte.

Das ist dann wohl die lokale Variable

CFicture aFicture;

, die am Ende der Funktion zerstört wird.

Mechanics

Das ist ein Stack Objekt und der Destuktor wird aufgerufen, wenns out of scope läuft. Probier nicht rum, lern einfach C++.

Ein Destruktor wird immer dann aufgerufen, wenn ein Objekt zerstört wird. Mit delete lässt sich ein Objekt zerstören, das mit new angefordert wurde. Sobald ein Objekt aber seinen scope verlässt, wird es automatisch zerstört.

CFicture aFicture;

//snip
return returnValue; //Funktion wird verlassen, aFicture wird zerstört-> Destruktoraufruf!

Jeremin

Da hab ich vor lauter Bäumen den Wald wohl nicht mehr gesehen, danke euch

Probier nicht rum, lern einfach C++.

Was denkst du, wozu mach das Ganze?

Gruss
jeremin

Darf man fragen mit welchem Buch du lernst?

Mechanics

Jeremin schrieb:

Probier nicht rum, lern einfach C++.

Was denkst du, wozu mach das Ganze?

Learning by doing hilft in der Phase nicht so viel, zumindest bei C++. Das muss man einfach wissen. Und bevor du nicht ein Grundlagenbuch (oder Tutorial oder was auch immer) durchgearbeitet hast, wirst du die ganze Zeit Probleme haben, die du nicht verstehst.

Jeremin

Ich versteh das Problem ja durchaus, ich hab nur übersehen, dass ich eine lokale Instanz von CFicture erzeugt habe. Es ist mir durchaus bewusst, dass der Destruktor nach dem Verlassen der Funktion aufgerufen wird, jetzt da ich die Ursache kenne.

Wobei das Problem evt. an meinem Programmierstil liegen könnte (obwohl ich versuche den zu bessern). Falls du den kritisieren möchtest, ich bitte darum

Aber ich bin ziemlich sicher, dass auch Leuten, die Tag täglich mit C++ zu tun haben noch Flüchtigkeitsfehler unterlaufen, wenn auch deutlich weniger häufig als mir.

Gruss
Jeremin

PS: Buch ist C++ Primer Plus, wobei dieses Projekt eher der Vertiefung des Gelernten dient.

Mechanics

Jeremin schrieb:

Wobei das Problem evt. an meinem Programmierstil liegen könnte (obwohl ich versuche den zu bessern). Falls du den kritisieren möchtest, ich bitte darum

Nee, da hab ich eigentlich nichts besonderes zu kritisieren. Außer vielleicht dass man Parameter möglichst als konstante Referenzen übergeben sollte. Das Programm ist viel zu klein, um irgendwelche Aussageb über den Stil machen zu können.
Du brauchst dir meine Kritik nicht so zu Herzen nehmen. Der Fehler war nur offensichtlich und deswegen hab ich gedacht, dir ist nicht klar, wie Stack Objekte funktionieren. Wenn das nicht der Fall ist, ist ja alles in Ordnung

Jeremin schrieb:

Wobei das Problem evt. an meinem Programmierstil liegen könnte (obwohl ich versuche den zu bessern). Falls du den kritisieren möchtest, ich bitte darum

Warum kopierst du die Pfade mit strcpy? Du hast doch schon patchPath.c_str() entdeckt, das kannst du auch direkt benutzen.

Hm, ein paar Gedanken zu dem code:
Warum verwendest du char*?

ifstream fin(patchPath.c_str(), ios::in | ios::binary);

geht doch genauso. Spart dir das new/delete.

fin.read((char*) &myPatchSaveFile, sizeof(patchSave));

(char*) als C-style cast bin ich nicht unbedingt ein fan von. Besser reinterpret_cast<char*>()

Als letztes kannst du dir noch überlegen (auch wenn man da geteilter Meinung sein kann) ob du deine Funktion nicht in 2 Teile teilen willst. Prinzipiell lädst du ja einmal das patch file, und einmal fictures, auch wenn das etwas zusammenhängt.

Ich meine damit in etwa sowas:

patchSave LoadPatchFile(string path)
{
path+="\\Patch.ptch";
patchSave myPatchSaveFile;

    //Patchinfos laden
    ifstream fin(path.c_str(), ios::in | ios::binary);
    if (fin.is_open()){
        fin.read((char*) &myPatchSaveFile, sizeof(patchSave));

        //Dimmer-Infos kopieren
        for (int i=0; i<NDIMCHANNELS; i++){
            this->dimCh[i] = myPatchSaveFile.dimCh[i];
            this->dimChAlias[i] = myPatchSaveFile.dimChAlias[i];
        }
        //Ficture-Infos kopieren
        for (int i=0; i<NFICTURES; i++){
            this->fictureAlias[i] = myPatchSaveFile.fictureAlias[i];
        }

        for (int i=0; i<512; i++)
            this->available[i] = myPatchSaveFile.available[i];
    }
    else
    {

       // returnValue -= 1;
       //Das geht so natürlich nicht mehr, ich würde hier eine exception werfen
    }
return myPatchSaveFile;

}

void LoadFictures(string path, const patchSave& myPatchSaveFile)
{
    path += "\\Fictures.fict";
    CFicture aFicture;

    //alle gespeicherten Fictures laden
    ifstream fin(path.c_str(), ios::in | ios::binary);
    if (fin.is_open()){
        for (int i=0; i<NFICTURES; i++){
            if (myPatchSaveFile.ficturesUsed[i]){
                fin.read((char*) &aFicture, sizeof(CFicture));
                if (fictures[i] != NULL)
                    delete fictures[i];
                fictures[i] = new CFicture(aFicture);
            }
            else{
                if (fictures[i] != NULL)
                    delete fictures[i];
                fictures[i] = NULL;
            }
        }
    }
    else
            //returnValue -= 2;
            //das würde hier zwar gehen, aber ich würde trotzdem eine exception bevorzugen
}

void CPatching::loadPatchingFile(string path){
   //falls du excepctions wirfst kannst du sie entweder hier fangen und als errorcode zurückgeben, oder gleich weiterfliegen lassen - soll sich doch der aufrufer drum kümmern. Wenn LoadPatchFile wirft, wird LoadFictures auch nicht ausgeführt, darauf muss man also nicht prüfen
   patchSave myPatchFile = LoadPatchFile(path);
   LoadFictures(path, myPatchFile);
}

Weiter würd ich den returnValue nicht mit -= zuweisen, da muss man sich ja immer den ganzen code anschaun, nur um zu wissen, was tatsächlich zurückgegeben wird. Besser direkt zuweisen - wenns geht.

Soll nur ein Anreiz zum nachdenken sein, gibt hier sicher 50 Leute die vorallem so ne Aufteilung für total dumm halten. :> Ich mags, weil es die einzelnen Aufgaben im code klarer darstellt. Andere sehen das sicher anders.

Michael E.

Sehe ich auch so mit der Aufteilung. Der nächste wichtige Schritt ist dann, aus CFicture **fictures ein vector<CFicture> fictures zu machen, sodass man die ganzen news und deletes los wird.

Jeremin

Gute Frage wieso ich char* verwende, ich glaube, ursprünglich habe ich es mit .c_str() gemacht, doch dann hab ich das aus irgendeinem Grund geändert (mal schauen, vielleicht finde ich raus, wieso).

fin.read((char*) &myPatchSaveFile, sizeof(patchSave));

Wir hatten das in der C++ Übungen so gelöst, daher ging ich davon aus, dass das ein vernünftiger Weg sei. Um reinterpret_cast habe ich bis jetzt einen Bogen gemacht, da dieser ja nicht ganz ungefährlich sei.

Die Aufteilung würde Sinn machen, dass baue ich in nächster Zeit um. Genauso mit exeptions, die sind eigentlich schon länger geplant, bin aber noch nicht dazu gekommen.

Der nächste wichtige Schritt ist dann, aus CFicture **fictures ein vector<CFicture> fictures zu machen, sodass man die ganzen news und deletes los wird.

Das Problem hierbei ist, dass ich explizit auf Fictures zugreifen möchte. Zum Beispiel möchte ich die Möglichkeit haben, auf Ficture4 zugreifen zu können, auch wenn vorher nach gar nichts gepatch ist.
Wenn ich das einem Vector lösen würde, müsste ich quasi die Versetzung von 4 auf 0 irgendwo mitführen, was mir recht umständlich vorkam.
Mir ist klar, dass CFicture **fictures; ein eher unsauberes Konstrukt darstellt, aber mir ist keine vernünftige Alternative dazu eingefallen.
Würdet ihr sagen, dass es sich lohnt, da noch etwas Hirnschmalz zu investieren und eine Lösung mit std::vector zu finden?

Danke für die Tipps
Jeremin

314159265358979

Jeremin schrieb:

Wir hatten das in der C++ Übungen so gelöst, daher ging ich davon aus, dass das ein vernünftiger Weg sei. Um reinterpret_cast habe ich bis jetzt einen Bogen gemacht, da dieser ja nicht ganz ungefährlich sei.

C-Casts sind noch viel gefährlicher als reinterpret_cast, da du damit auch CV-Qualifier wegcasten kannst.

Oder ums anders zu sagen - C-Style casts statt reinterpret_cast zu verwenden, weil dieser gefährlich ist, ist so ähnlich wie eine Atom-Bombe statt einer Splittergranate zu verwenden, weil die Splittergranate so stark explodiert.