[C++/Qt] Speicherzugriffsfehler

[Commodore]

Ich habe folgenden Code:

void game::next_row(){
    int tmp[9];
    for(int i = 0 ; i < 9 ; ++i){
        tmp[i] = entries.back()[i + 1];
    }
    std::vector<int*>::iterator last_arr = entries.end();
    --last_arr;
    for(std::vector<int*>::iterator it = entries.begin() ; it != last_arr ; ++it){
        for(int i = 1 ; i < 10 ; ++i){
            if((*it)[i] > 0){
                push((*it)[i]);
            }
        }
    }
    for(int i = 0 ; i < 9 ; ++i){
        if(tmp[i] > 0){
            push(tmp[i]);
        }
    }
    resize(sizeHint());
    update();
}

Diese Funktion ist ein Slot und verbunden mit dem clicked()-Signal eines Buttons. Wenn ich den Code nun kompiliere und ca 3-5 mal auf den Button drücke, stürzt das Programm ab und ich kriege einen Speicherzugriffsfehler.

Dieser Fehler tritt aber nicht immer auf. Manchmal kann ich die Funktion problemlos mehrfach aufrufen, manchmal bekomme ich schon beim ersten Aufruf eine Fehlermeldung.

entries ist ein std::vector<int*>, dabei besteht jedes array in dem Vektor aus 10 Elementen. Das Element 0 hat eine gesonderte Bedeutung, die Elemente 1-9 sollen sukzessive aufgefüllt werden. Dies geschieht in der push()-Methode, wobei immer nach 9 Elementen ein neues Array erstellt wird, welches als nächstes gefüllt wird. Die push()-Methode funktioniert einwandfrei, in Testläufen kann ich sie problemlos 3000 mal aufrufen und es gibt keinen Fehler.


Wenn ich jedoch eine der beiden unteren for-Schleifen auskommentiere, stürzt das Programm nicht ab. Einzeln funktionieren die Schleifen also, aber gemeinsam nicht.

Hat irgendjemand eine Idee wie ich das Problem beheben kann? Ich nutze Qt 4.7.0 und Ubuntu 10.10 [64 bit].

 

[mermshaus]

Das ist jetzt geraten:

entries ist ein std::vector<int*>, dabei besteht jedes array in dem Vektor aus 10 Elementen.

Das sieht mir von der Deklaration her nicht zweidimensional aus, sonder eher wie eine eindimensionale Liste von int-Pointern (?).

Kannst du das Beispiel vielleicht irgendwie vereinfachen, so dass man es lokal testen könnte?

 

[Commodore]

Es ist zweidimensional, weil jeder zeiger auf ein array aus 10 integers zeigt.

Die push sieht wie folgt aus:

void game::push(int num){
    if(last_col == 10){
        int* row = new int[10];
        for(int i = 0 ; i < 10 ; ++i){
            row[i] = 0;
        }
        entries.push_back(row);
        last_col = 1;
    }
    (entries.back())[last_col] = num;
    ++last_col;
}

Im konstruktor der Klasse wird zudem last_col auf 10 gesetzt, damit beim ersten Aufruf von push() ein neues array erstellt wird.

Zu Anfang wird der Vektor zudem mit 27 Werten gefüllt:

char init_values[] = "123456789111213141516171819";
for(int i = 0 ; i < 27 ; ++i){
    push((int)(init_values[i] - '0'));
}

Das sollte eigentlich reichen, um es lokal zu testen.

 

[mermshaus]

Wäre es möglich, dass entries durch das wiederholte push() so groß wird, dass es während des Iterierens in next_row() im Speicher verschoben wird und der Iterator-Pointer in den falschen Bereich zeigt?

(Ich glaube, die letzte for-Schleife in next_row() ist unproblematisch.)

 

[Commodore]

Klingt einleuchtend, aber daran wird das Problem nicht liegen. Das Problem läge dann in der mittleren Schleife, aber wenn ich nur die mittlere und die obere Schleife übriglasse gibts keine Probleme.


Edit: Du hattest recht, wenn ich im Konstruktor entries.reserve(1000); aufrufe gibts keinen segfault. Ich denke ich werde das Problem lösen indem ich einen Gewissen Wert vorher reserviere [100 sollten ausreichen] und anschließend nicht mit dem iterator, sondern mittels Schleife und entries arbeite.

Vielen dank, daran habe ich gestern mindestens 3 Stunden lang gesessen :)

 

[mermshaus]

In meinem Test hatte ich auch erst den Segfault, aber jetzt läuft er sauber durch.

Deshalb poste ich mal meinen Code. Vielleicht hilft dir das. Ich packe die hinzuzufügenden Elemente jetzt erst in einen neuen Vector und füge sie nach dem Iterieren hinzu.

#include <iostream>
#include <vector>

class Game {
    int last_col;
    std::vector<int*> entries;
public:
    void init();
    void push(int);
    void next_row();
    void dump_entries();
};

int main(int argc, char * argv[])
{
    int i;
    
    Game mygame;    
    mygame.init();      
    
    for (i = 0; i < 10; ++i) {
        mygame.next_row();        
        mygame.dump_entries();    
        std::cout << "\n";
    }       
    
    return 0;
}

void Game::init()
{    
    last_col = 10;    
    
    // Push three rows of values into <entries>
    char init_values[] = "123456789111213141516171819";        
    for (int i = 0; i < 27 ; ++i) {
        push((int)(init_values[i] - '0'));
    }
}

void Game::dump_entries()
{
    int c = 0;
    
    std::vector<int*>::iterator it_outer;
    
    for (it_outer = entries.begin(); it_outer < entries.end(); ++it_outer) {
        std::cout << ++c << ": ";
        for (int i = 0; i < 10; ++i) {
            std::cout << (*it_outer)[i];
        }        
        std::cout << "\n";
    }    
}

void Game::push(int num)
{
    if (last_col == 10) { // Add space for a new row
        int* row = new int[10];
        //int row[10];
        for (int i = 0; i < 10; ++i) {
            row[i] = 0;
        }
        entries.push_back(row);
        last_col = 1;
    }
    
    // Add element
    
    (entries.back())[last_col] = num;
    ++last_col;
}  

void Game::next_row()
{    
    int i;
    
    // Copy last row from <entries> to <tmp>
    //int* tmp = new int[9];
    int tmp[9];
    for(i = 0 ; i < 9 ; ++i){
        tmp[i] = entries.back()[i + 1];
    }
    
    // Iterate over <entries>
    std::vector<int*>::iterator it;
    
    std::vector<int> elements;    
    
    for (it = entries.begin(); it < entries.end(); ++it) {
        for(i = 1 ; i < 10 ; ++i){            
            if ((*it)[i] > 0){
                elements.push_back((*it)[i]);
            }
        }
    }
    
    std::vector<int>::iterator it2;
    for (it2 = elements.begin(); it2 < elements.end(); ++it2) {
        push((*it2));
    }
    
    for (i = 0 ; i < 9 ; ++i) {
        if (tmp[i] > 0) {
            push(tmp[i]);
        }
    }
}

Kompiliert/gestartet per:

$ g++ test.cpp && ./a.out

Ich habe nicht verstanden, was das Programm tun soll. Kann also sein, dass ich da teilweise Unsinn fabriziert habe. Möglicherweise geht es nur deshalb. :)

 

[Commodore]

Hab meinen Beitrag editiert kurz bevor du geschrieben hast ;)

Edit: Funktioniert jetzt wunderbar, konnte alles auch ein wenig verkürzen weil das tmp nur ein Versuch war den segfault zu beheben:

    const int max = entries.size();
    for(int i = 0 ; i < max ; ++i){
        for(int j = 1 ; j < 10 ; ++j){
            if(entries[i][j] > 0)
                push(entries[i][j]);
        }
    }