delete verhält sich in main anders als in member function

otze

Auch wnen wir dir sagen: "zeig mal Code"

solltest du immer noch den Sticky oben beachten:

reduzier deinen Code soweit, das zwar noch der Fehler auftritt, aber alles unnötige entfernt wurde.

Sone

Ich sag dir gleich eine Sache: Versuche so wenig Speiche wie möglich selbst zu allokieren. Wieso nicht gleich Stackobjekte?

Und wozu überhaupt

FMatrix<double>* atrix = NULL;
    atrix = new FMatrix<double>(2,3);
    delete atrix;

? Du nutzt atrix doch nicht einmal...

otze

ich befürchte, hacker, dass das testcode ist um den Fehler zu provozieren.

Dein code ist prinzipiell unglaublich unübersichtlich. Wenn du das Problem angehst, ist es wohl auch leichter den Fehler zu finden - oder du behebst ihn "versehentlich".

1.Viel zu viel rum-gezeigere.

string *name = new std::string(fileName); //warum mit new?

Matrix<double>* matrix;
double *rhs;
double *solution;
Cube<int> *cube;

Warum sind das alles Zeiger? Warum keine STL container?

2. Speicherlecks. Destruktor, copy-ctor und assignment-operator fehlen völlig. Früher oder später explodiert dir das also sowieso.

3. Du initialisierst die Klasse nicht richtig im konstruktor. Deswegen hast du vielleicht so viele Zeiger, um "nicht initialisiert" zu kennzeichnen. Das ist aber schon ein grundsätzliches Problem! Ist die Modellierung vielleicht Fehlerhaft?

bool LinSolvers::isValidNode(int i, int j){
    int i1,j1,k1,i2,j2,k2;
    getFullIndex(i, i1,j1,k1);
    i1++;//cube starts at 1
    j1++;
    k1++;
    if (!isValidIndex(i1,j1,k1))
        return false;
    getFullIndex(j, i2,j2,k2);
    i2++;//cube starts at 1
    j2++;
    k2++;
    for (int ii=-1; ii<2;ii++)
        for (int jj=-1; jj<2;jj++)
            for (int kk=-1; kk<2;kk++){
                if ((ii!=0)&&(jj!=0)&&(kk!=0)) // This is because the kernel contains 19 elements instead of 27
                    continue;
                if (!isValidIndex(i2+ii,j2+jj,k2+kk))
                    continue;
                if (((*cube)(i1,j1,k1)==(*cube)(i2+ii,j2+jj,k2+kk))&&((*cube)(i1,j1,k1)!=0)){
                    return true;
                }
            }
    return false;
}

Wenn cube nicht zufälligerweise in void LinSolvers::createSymbolicSystem() erstellt wurde, crasht es hier direkt - wenn du Glück hast. Das Problem hängt eng mit 1. und 3. zusammen

void LapackSolverBanded::setElement(int i, double value){
    rhs[i] = value;
}
double LapackSolverBanded::getElement(int i){
    return rhs[i];
}

Woher weißt du, ob es bei rhs an der Stelle i überhaupt ein Element gibt? Warum erstellst du rhs nicht im Kontruktor?

double A[3][3] = {{3,1,3},{1,2,3},{2,6,5}};
for (int i = 0; i < N; i++){
    for (int j = 0; j < N; j++){
    //cout << i << "  " << j << endl;
    setElement(i,j,A[i][j]);
}

Was für einen Wert hat N? Da das mit unbekannten konstanten erstellt wird, weiß ich das nicht - aber falls du diese konstanten änderst, hast du hier auf jedenfall einen bug - wenn du Glück hast einen crash, wenn du Pech hast seltsame Fehler, die nur manchmal auftreten!

So, ich hör mal auf, aber dein Problem ist sehr sehr grundlegend.
Zwing dich mal dazu, deinen code so zu schreiben, das du:
a) Keine Init funktionen hast.
b) Alle Werte direkt im ctor so initialisiert werden, dass du damit arbeiten kannst.
c) Dich im weiteren Verlauf immer auf diese Werte beziehst. Keine magic numbers.
d) Du destruktor, copy-ctor und Zuweisungsoperator korrekt schreibst.

Das dürfte eine ziemlich lange Arbeit werden, aber dann hast du evtl chancen dass du auch langfristig funktionierenden code haben könntest.

Grad gesehen, das N wohl 16 sein dürfte. Das dürfte im aktuellem Fall dein Problem sein. Ändert aber nichts an deinem grundsätzlichem Designproblem.

J.Bourne

Hallo Leute,
vielen Dank euch allen und KMT im Besonderen!

Zum Fehler: Wie bereits prophezeit, ist er bei der recht umständlichen Eingrenzung verschwunden. Eine Zeit lang konnte ich ihn rekonstruieren, aber nicht festnageln. Zur Zeit glaube ich, dass es gar kein "Quelltext-Fehler" war, sondern dass er daher rührte, dass ich ein Objektfile einer anderen Klasse mit gelinkt habe(versehentlich->makefile). Zumindest konnte ich den Fehler durch das Entfernen aller Objektdateien beheben.

Ich finde es großartig, dass ihr mir von euch aus so viele wertvolle Hinweise gegeben habt. Ich schreibe zwar in letzter Zeit immer mehr Code, aber es gibt eigentlich niemanden, der mich auf solche strukturellen Dinge hinweisen würde.

KMT schrieb:

1.Viel zu viel rum-gezeigere.
string *name = new std::string(fileName); //warum mit new?
Matrix<double>* matrix;
double *rhs;
double *solution;
Cube<int> *cube;
Warum sind das alles Zeiger? Warum keine STL container?

Zu 1.
Der string-Pointer ist ein Relikt.
solution und rhs sind pointer, da diese auf C-Arrays zeigen, die später einer Library-Funktion übergeben werden sollen, die nur solche akzeptiert.
matrix und cube als pointer, damit ich dynamisch andere Instanzen erzeugen und zuweisen kann (neue Dimension oder Größe). Wäre es besserer Stil keine Pointer zu benutzen und der Klasse die Fähigkeit zu geben Größe etc. zu ändern?

KMT schrieb:

2. Speicherlecks. Destruktor, copy-ctor und assignment-operator fehlen völlig. Früher oder später explodiert dir das also sowieso.

Zu 2.
Destruktoren fehlen sicherlich. Ich habe bisher keinen Bedarf für copy-ctor und assignment-operator gesehen. Sollte man sie trotzdem immer initalisieren?

KMT schrieb:

3. Du initialisierst die Klasse nicht richtig im konstruktor. Deswegen hast du vielleicht so viele Zeiger, um "nicht initialisiert" zu kennzeichnen. Das ist aber schon ein grundsätzliches Problem! Ist die Modellierung vielleicht Fehlerhaft?

Zu 3.
Nun ich initialisiere alles, was beim Erzeugen des Objektes bekannt ist... Wenn dies mehr sein sollte, ist es vermutlich echt ein Modellproblem.

KMT schrieb:

bool LinSolvers::isValidNode(int i, int j){
    int i1,j1,k1,i2,j2,k2;
    getFullIndex(i, i1,j1,k1);
    i1++;//cube starts at 1
    j1++;
    k1++;
    if (!isValidIndex(i1,j1,k1))
        return false;
    getFullIndex(j, i2,j2,k2);
    i2++;//cube starts at 1
    j2++;
    k2++;
    for (int ii=-1; ii<2;ii++)
        for (int jj=-1; jj<2;jj++)
            for (int kk=-1; kk<2;kk++){
                if ((ii!=0)&&(jj!=0)&&(kk!=0)) // This is because the kernel contains 19 elements instead of 27
                    continue;
                if (!isValidIndex(i2+ii,j2+jj,k2+kk))
                    continue;
                if (((*cube)(i1,j1,k1)==(*cube)(i2+ii,j2+jj,k2+kk))&&((*cube)(i1,j1,k1)!=0)){
                    return true;
                }
            }
    return false;
}

Wenn cube nicht zufälligerweise in void LinSolvers::createSymbolicSystem() erstellt wurde, crasht es hier direkt - wenn du Glück hast. Das Problem hängt eng mit 1. und 3. zusammen

Zu 4.
Ich bin mir nicht sicher, ob ich das richtig verstehe...
Geht es darum, dass es eher eine Methode ist, die zu cube gehört? Und darum, dass sie eine Struktur von cube voraussetzt, die nicht vorgeschrieben ist?

KMT schrieb:

void LapackSolverBanded::setElement(int i, double value){
    rhs[i] = value;
}
double LapackSolverBanded::getElement(int i){
    return rhs[i];
}
Woher weißt du, ob es bei rhs an der Stelle i überhaupt ein Element gibt? Warum erstellst du rhs nicht im Kontruktor?

Zu 5.
Fehlender Kontrollmechanismus->ok
Ich initialisiere nicht, weil ich mehrfach verschiedene Daten in rhs schreiben will.

KMT schrieb:

double A[3][3] = {{3,1,3},{1,2,3},{2,6,5}};
for (int i = 0; i < N; i++){
    for (int j = 0; j < N; j++){
    //cout << i << "  " << j << endl;
    setElement(i,j,A[i][j]);
}
Was für einen Wert hat N? Da das mit unbekannten konstanten erstellt wird, weiß ich das nicht - aber falls du diese konstanten änderst, hast du hier auf jedenfall einen bug - wenn du Glück hast einen crash, wenn du Pech hast seltsame Fehler, die nur manchmal auftreten!

Zu 6.
Ja, das ist nicht konsistent.

KMT, um deine abschließenden Ratschläge erfüllen zu können wird mir wohl nichts anderes übrig bleiben, als das Ganze neu zu strukturieren und zu modellieren.

Vielen Dank noch mal

solution und rhs sind pointer, da diese auf C-Arrays zeigen, die später einer Library-Funktion übergeben werden sollen, die nur solche akzeptiert.
matrix und cube als pointer, damit ich dynamisch andere Instanzen erzeugen und zuweisen kann (neue Dimension oder Größe). Wäre es besserer Stil keine Pointer zu benutzen und der Klasse die Fähigkeit zu geben Größe etc. zu ändern?

Für arrays mit variabler Größe gibt es vector. Der hat den großen Vorteil, das er dir Speicherverwaltung abnimmt, und du so nicht so leicht Fehler mit der Speicherverwaltung machen kannst. Du kannst einen vector auch jederzeit als array übergeben, falls eine Funktion ein array erwartet. (vector.data() )
Selbst wenn du irgendwelche Anforderungen hast, die vector (oder ein anderer container der STL) nicht erfüllt, schreib dir eine eigene kleine Klasse, die sich um die Verwaltung kümmert. So muss sich die Klasse, die die eigentliche Arbeit macht nicht um diese Verwaltungsaufgaben kümmern, und wird deutlich übersichtlicher.

Destruktoren fehlen sicherlich. Ich habe bisher keinen Bedarf für copy-ctor und assignment-operator gesehen. Sollte man sie trotzdem immer initalisieren?

Deklarier sie zumindest private. Wenn du dann ausversehen eine Kopie irgendwo erzeugst, bekommst du zumindest direkt einen compilerfehler, statt dich auf die Suche nach irgendwelchen Speicherzugriffsfehlern zu machen. Wenn du Zeiger verwendest erzeugen die vom compiler automatisch erzeugten eigentlich immer für Fehler.

Nun ich initialisiere alles, was beim Erzeugen des Objektes bekannt ist... Wenn dies mehr sein sollte, ist es vermutlich echt ein Modellproblem.

LapackSolverBanded solver;
    cout << "initMatrix" << endl;
    solver.initMatrix();
    cout << "initRhs" << endl;
    solver.initRhs();

Hm, wenn du initMatrix() / initRhs() aufrufst, ist doch gar nicht so viel neues bekannt. Könntest du doch also direkt im Konstruktor aufrufen.
Falls es tatsächlich einmal sein sollte, das etwas nicht bekannt ist, wenn du ein Objekt erstellen willst, heißt es normalerweise, dass du zu früh versuchst, das Objekt zu erstellen. Erst alles zusammensuchen was man braucht, dann erstellen und verwenden.

Ich bin mir nicht sicher, ob ich das richtig verstehe...
Geht es darum, dass es eher eine Methode ist, die zu cube gehört? Und darum, dass sie eine Struktur von cube voraussetzt, die nicht vorgeschrieben ist?

Mir fällt aber auf, das du createSymbolicSystem() im Konstruktor aufrufst. Passt also eigentlich alles, sorry hab ich übersehen. Sonst hätte es passieren können, das du "cube" mit "*cube" dereferenzierst, obwohl es noch 0 wäre.

Mal eine Idee für den Anfang: Schreib dir mal eine Klasse für rhs.
Also Konstruktor nimmst du die Funktion

void loadRhsFromFile( char fileName[]);

Sprich, du übergibst den Dateinamen, und dann hast du ein gültiges RHS. Zu testzwecken kannst du noch einen zweiten Konstruktor schreiben, dem man die Daten direkt übergeben kann. Da passen dann auch deine Funktionen getElement und setElement rein, eventuell auch andere Funktionen (ich schau mal nicht alles durch). Als nächstes kannst du dir überlegen, ob rhs wirklich ein member von deinem solver sein soll, und nicht lieber ein Parameter, den man übergibt, wenn er gebraucht wird. Versuch den Ansatz "Ich speicher erstmal alle Daten in einer Klasse, und rechne dann damit" zu vermeiden. Daten erstellen ist eine Aufgabe, die gar nichts mit deinem Algorithmus zu tun hat.

hustbaer

@J.Bourne
Implementier' den Zuweisungsoperator für deine Matrix-Klasse, dann kannst du die auch jederzeit ändern.
BTW: C++ implementiert den Zuweisungsoperator auch automatisch für dich, aber in diesem Fall keinen passenden.

Oder gleich noch besser: ändere deine Matrix Klasse so dass sie std::vector verwendet, statt selbst mit new[] und delete[] rumzumachen.
Dann funktioniert nämlich auf einmal der compiler-generierte Zuweisungsoperator.

J.Bourne

Vielen Dank noch einmal!
Das ist wirklich hilfreich. Ich werde es dann mal versuchen in die Tat um zu setzen.
Viele Grüße

pumuckl

J.Bourne schrieb:

Zu 1.
Der string-Pointer ist ein Relikt.

Der war auch als er noch kein Relikt war ziemlich sicher schon überflüssig. Strings benutzt man so gut wie immer solo, nicht über Pointer.

solution und rhs sind pointer, da diese auf C-Arrays zeigen, die später einer Library-Funktion übergeben werden sollen, die nur solche akzeptiert.

Trotzdem vector benutzen, gibt ja vector::data()

Wäre es besserer Stil keine Pointer zu benutzen und der Klasse die Fähigkeit zu geben Größe etc. zu ändern?

Meist ja.

[quote="KMT"]Zu 2.
Destruktoren fehlen sicherlich. Ich habe bisher keinen Bedarf für copy-ctor und assignment-operator gesehen. Sollte man sie trotzdem immer initalisieren?
[quote="KMT"] Wenn du einen Konstruktor schreibst, der mehr macht als nur Werte zu setzen (also wie hier z.B. Speicher allokiert), musst du fast zwangsläufig einen Dtor und einen Copy-Ctor und op= definieren. Falls du sie nicht brauchst, deklariere sie nur, und zwar als private, damit der compiler sie dir nicht "aus versehen" definiert ohne dass du merkst dass da Mist passiert.

Ich initialisiere nicht, weil ich mehrfach verschiedene Daten in rhs schreiben will.

Ist kein Grund, rhs nicht zu initalisieren

J.Bourne schrieb:

Zur Zeit glaube ich, dass es gar kein "Quelltext-Fehler" war, sondern dass er daher rührte, dass ich ein Objektfile einer anderen Klasse mit gelinkt habe(versehentlich->makefile). Zumindest konnte ich den Fehler durch das Entfernen aller Objektdateien beheben.

Das kann nur dann die Ursache gewesen sein, wenn du in mehreren Objektdateien unterschiedliche Definitionen gleichnamiger inline-Funktionen oder gleichnamiger Templates hattest. Dann kann allerdings Blödsinn bei rumkommen. Andernfalls haben die weggefallenen Objektdateien einfach nur die Auswirkungen des undefinierten Verhaltens geändert.