Zwalnianie pamięci, program przestał działać

transporter22 · 7 Styczeń 2015 20:34

Po pierwszym uruchomieniu programu czasem wywala “program przestał działać” dzieje sie to tylko wtedy kiedy od razu z menu wybiorę zakończ. Podejrzewał, że problem jest związany z zwalnianiem pamięci. Uwaga używam biblioteki curses.h (http://cpp0x.pl/kursy/Kurs-New-Curses-C++/Wstep/84).

Program po kompilacji http://www.speedyshare.com/bZV8E/gauss.exe

#include curses.h
#include iostream
#include cmath

using namespace std;

bool zakoncz(){
    char pomocnicza;
    printw("Zakoncz?(y/n): ");
    pomocnicza=getch();
    if(pomocnicza=='y' || pomocnicza=='Y')
        return false;
    return true;
}
void wyswietl(char *Tz,float **T,int n){
    printw(" **************************************************** \n");
    for(int i=0;in;i++)
        printw("X%c\t",Tz[i]);
    printw("\n");
    for(int i=0;in;i++){
        for(int j=0;jn+1;j++)
            printw("%.3f\t",T[i][j]);
        printw("\n");
    }
    printw(" **************************************************** \n");
}
float** tablica(int n){
    float **tablica= new float *[n];
    for(int i=0;in;i++)
        tablica[i]=new float [n+1];
    return tablica;
}
char* tablicaznakow(int n){
    char *Tablicaznakow;
    Tablicaznakow=new char [n];
    for(int i=0;in;i++)
        Tablicaznakow[i]=i+49;
    return Tablicaznakow;
}
float** wczytajdane(int n){
    printw("Podaj ilosc niewiadomych:\t");
    scanw("%d",n);
    char pomocnicza;
    char *Tz=tablicaznakow(n);
    float **Tx=tablica(n);
    printw(" **************************************************** \n");
    for(int i=0;in;i++){
        for(int j=0;jn+1;j++){
            printw("[%d][%d]:\t",i,j);
            scanw("%f",Tx[i][j]);
        }
        clear();
    }
    clear();
    wyswietl(Tz,Tx,n);
    printw("OK?(y/n): ");
    pomocnicza=getch();
    if(pomocnicza=='n' || pomocnicza=='N'){
        clear();
        wczytajdane(n);
    }
    clear();
    return Tx;
}
float**kopiadanych(float**T,int n){
    float **kopia=tablica(n);
    for(int i=0;in;i++)
        for(int j=0;jn+1;j++)
            kopia[i][j]=T[i][j];
    return kopia;
}
int menu(){
    int wx=0;
    int wy=0;
    getmaxyx(stdscr,wx,wy);
    int znak;
    short int ktory=1;
    const short int min_wybor = 1;
    const short int max_wybor = 6;
    const char text1[]="Eliminacji Gaussa: pelnym wyborem elementu maksymalnego ";
    const char text2[]="Eliminacji Gaussa: wyborem elementu maksymalnego w kolumnie ";
    const char text3[]="Eliminacji Gaussa: wyborem elementu maksymalnego w wierszu ";
    const char text4[]="Eliminacji Gaussa: podstawowa ";
    const char text5[]="Wczytaj nowe dane";
    const char text6[]="Zakoncz";
    do{
        switch(ktory){
        case 1:
            attron(A_REVERSE);
            mvprintw(wx/2-3,wy/2-sizeof(text1)/2,text1);
            attroff(A_REVERSE);
            mvprintw(wx/2-2,wy/2-sizeof(text2)/2,text2);
            mvprintw(wx/2-1,wy/2-sizeof(text3)/2,text3);
            mvprintw(wx/2,wy/2-sizeof(text4)/2,text4);
            mvprintw(wx/2+1,wy/2-sizeof(text5)/2,text5);
            mvprintw(wx/2+2,wy/2-sizeof(text6)/2,text6);
            mvprintw(wx/2+3,wy/2,"");
            break;
        case 2:
            mvprintw(wx/2-3,wy/2-sizeof(text1)/2,text1);
            attron(A_REVERSE);
            mvprintw(wx/2-2,wy/2-sizeof(text2)/2,text2);
            attroff(A_REVERSE);
            mvprintw(wx/2-1,wy/2-sizeof(text3)/2,text3);
            mvprintw(wx/2,wy/2-sizeof(text4)/2,text4);
            mvprintw(wx/2+1,wy/2-sizeof(text5)/2,text5);
            mvprintw(wx/2+2,wy/2-sizeof(text6)/2,text6);
            mvprintw(wx/2+3,wy/2,"");
            break;
        case 3:
            mvprintw(wx/2-3,wy/2-sizeof(text1)/2,text1);
            mvprintw(wx/2-2,wy/2-sizeof(text2)/2,text2);
            attron(A_REVERSE);
            mvprintw(wx/2-1,wy/2-sizeof(text3)/2,text3);
            attroff(A_REVERSE);
            mvprintw(wx/2,wy/2-sizeof(text4)/2,text4);
            mvprintw(wx/2+1,wy/2-sizeof(text5)/2,text5);
            mvprintw(wx/2+2,wy/2-sizeof(text6)/2,text6);
            mvprintw(wx/2+3,wy/2,"");
            break;
        case 4:
            mvprintw(wx/2-3,wy/2-sizeof(text1)/2,text1);
            mvprintw(wx/2-2,wy/2-sizeof(text2)/2,text2);
            mvprintw(wx/2-1,wy/2-sizeof(text3)/2,text3);
            attron(A_REVERSE);
            mvprintw(wx/2,wy/2-sizeof(text4)/2,text4);
            attroff(A_REVERSE);
            mvprintw(wx/2+1,wy/2-sizeof(text5)/2,text5);
            mvprintw(wx/2+2,wy/2-sizeof(text6)/2,text6);
            mvprintw(wx/2+3,wy/2,"");
            break;
        case 5:
            mvprintw(wx/2-3,wy/2-sizeof(text1)/2,text1);
            mvprintw(wx/2-2,wy/2-sizeof(text2)/2,text2);
            mvprintw(wx/2-1,wy/2-sizeof(text3)/2,text3);
            mvprintw(wx/2,wy/2-sizeof(text4)/2,text4);
            attron(A_REVERSE);
            mvprintw(wx/2+1,wy/2-sizeof(text5)/2,text5);
            attroff(A_REVERSE);
            mvprintw(wx/2+2,wy/2-sizeof(text6)/2,text6);
            mvprintw(wx/2+3,wy/2,"");
            break;
        case 6:
            mvprintw(wx/2-3,wy/2-sizeof(text1)/2,text1);
            mvprintw(wx/2-2,wy/2-sizeof(text2)/2,text2);
            mvprintw(wx/2-1,wy/2-sizeof(text3)/2,text3);
            mvprintw(wx/2,wy/2-sizeof(text4)/2,text4);
            mvprintw(wx/2+1,wy/2-sizeof(text5)/2,text5);
            attron(A_REVERSE);
            mvprintw(wx/2+2,wy/2-sizeof(text6)/2,text6);
            attroff(A_REVERSE);
            mvprintw(wx/2+3,wy/2,"");
            break;
        case 7:
            break;
        }
        znak=getch();
        clear();
        if(znak==119 ktory!=min_wybor)
            ktory--;
        else if(znak==115 ktory!=max_wybor)
            ktory++;
    }while(znak!=10);
    return ktory;
}

int maxymalnykolumna(float **T,int n,int krok){
    int max_x=krok;
    for(int x=krok;xn;x++)
        if(abs(T[krok][x])abs(T[krok][max_x]))
                max_x=x;
    return max_x;
}
int maxymalnywiersz(float **T,int n,int krok){
    int max_y=krok;
    for(int y=krok;yn;y++)
        if(abs(T[y][krok])abs(T[max_y][krok]))
                max_y=y;
    return max_y;
}
int maxymalny(float **T,int n,int krok,int* max_y){
    int max_x=krok;
    *max_y=krok;
    for(int y=krok;yn;y++)
        for (int x=krok;xn;x++)
            if(abs(T[y][x])abs(T[*max_y][max_x])){
                max_x=x;
                *max_y=y;
            }
    return max_x;
}
int zamianakolumn(char *Tz,float **T,int n,int krok,int x){
    int max_x;
    if(-1!=x)
        max_x=x;
    else
        max_x=maxymalnykolumna(T,n,krok);
    for(int i=0;in;i++)
        swap(T[i][krok],T[i][max_x]);
    swap(Tz[krok],Tz[max_x]);
    return ++krok;
}
int zamianawiersza(char*,float **T,int n,int krok,int y){
    int max_y;
    if(-1!=y)
        max_y=y;
    else
        max_y=maxymalnywiersz(T,n,krok);
    for(int i=0;in+1;i++)
        swap(T[krok][i],T[max_y][i]);
    return ++krok;
}
int zamianapelna(char *Tz,float **T,int n,int krok,int){
    int max_y=0;
    int max_x=maxymalny(T,n,krok,max_y);
    zamianakolumn(Tz,T,n,krok,max_x);
    zamianawiersza(Tz,T,n,krok,max_y);
    return ++krok;
}
int zmianapodstawa(char*,float**,int,int krok,int){
    return ++krok;
}

void eliminacjiGaussa(char *Tz,float **T,int n,int (*zamiana)(char*,float**,int,int,int),int krok=0){
    long double pomocnicza;
    while(krokn-1){
        krok=zamiana(Tz,T,n,krok,-1);
        for(int i=krok;in;i++){
            if(0!=T[krok-1][krok-1])
                pomocnicza=T[i][krok-1]/T[krok-1][krok-1];
            else{
                printw("Niedozwolona operacja: nan\n");
                krok=n;
                break;
            }
            for(int j=krok-1;jn+1;j++)
                T[i][j]=(T[i][j]-pomocnicza*T[krok-1][j]);
        }
    }
}
void wynik(char *Tz,float **T,int n){
    int a;
    float pomocnicza=0;
    float *Tw;
    Tw=new float [n];
    for(int i=n;0i;i--){
        if(0!=T[i-1][i-1])
            Tw[i-1]=(T[i-1][n]-pomocnicza)/T[i-1][i-1];
        else{
            printw("Niedozwolona operacja: nan\n");
            break;
        }
        printw("X%c = %.3f\n",Tz[i-1],Tw[i-1]);
        a=1;
        pomocnicza=0;
        for(int j=i-1;jn;j++){
            if(i-10)
                pomocnicza+=Tw[n-a]*T[i-2][n-a];
            a++;
        }
    }
}
int main(){
    initscr();
    bool exit=true;
    int ktory;
    int n=0;
    float **T;
    float **Tkopia;
    char *Tz;
    char *Tzkopia;
    T=wczytajdane(n);
    Tz=tablicaznakow(n);
    while(exit==true){
        ktory=menu();
        Tkopia=kopiadanych(T,n);
        Tzkopia=tablicaznakow(n);
        switch(ktory){
        case 1:
            eliminacjiGaussa(Tzkopia,Tkopia,n,zamianapelna);
            wyswietl(Tzkopia,Tkopia,n);
            wynik(Tzkopia,Tkopia,n);
            exit=zakoncz();
            break;
        case 2:
            eliminacjiGaussa(Tzkopia,Tkopia,n,zamianakolumn);
            wyswietl(Tzkopia,Tkopia,n);
            wynik(Tzkopia,Tkopia,n);
            exit=zakoncz();
            break;
        case 3:
            eliminacjiGaussa(Tzkopia,Tkopia,n,zamianawiersza);
            wyswietl(Tzkopia,Tkopia,n);
            wynik(Tzkopia,Tkopia,n);
            exit=zakoncz();
            break;
        case 4:
            eliminacjiGaussa(Tzkopia,Tkopia,n,zmianapodstawa);
            wyswietl(Tzkopia,Tkopia,n);
            wynik(Tzkopia,Tkopia,n);
            exit=zakoncz();
            break;
        case 5:
            T=wczytajdane(n);
            Tz=tablicaznakow(n);
            break;
        case 6:
            exit=false;
            break;
        }
        clear();
    }
    for(int i=0;in;i++)
        delete[] T[i];
    delete[] T;
    delete[] Tz;
    for(int i=0;in;i++)
        delete[] Tkopia[i];
    delete[] Tkopia;
    delete[] Tzkopia;
    endwin();
    return 0;
}

Ryan · 8 Styczeń 2015 11:02

Zwalniasz niezaalokowaną pamięć w linii “delete[] T_;” pod koniec main. Wynika to z tego, że nie radzisz sobie poprawnie z tablicami dwuwymiarowymi: wczytajdane() woła tablica(), a ta alokuje [n][n+1]. Zwalniając pod koniec main iterujesz po n+1 elementów, choć masz ich n. Wystarczy ustawić breakpoint w tym miejscu w kodzie, żeby to zaobserwować._

Nieszczególnie mogę Ci poradzić co masz zrobić z tym kodem, poza przepisaniem go porządnie. Funkcje o nazwach tablica(), kopia(), kopiadanych() i im podobne w żaden sposób nie informują czytelnika o tym, co kod stara się zrobić. Tablica znaków, którą generujesz, jest zbędna. Dane mógłbyś z powodzeniem przechowywać w jednowymiarowej tablicy - uprościłoby to alokację i zwalnianie i nie wpłynęło negatywnie na dostęp do danych.

transporter22 · 8 Styczeń 2015 14:03

Poprawiłem rzeczywiście błędnie rozumiałem dealokacje pamięci, podsumowując ten problem rozwiał moje wątpliwości.

Co do zastąpienia tablicą jednowymiarową, nie za bardzo nawet mam pomysł nad rozwiązaniem tego problemu przychodzi mi tylko takie rozwiązanie:( może jakaś wskazówka?)

for(i=0;i<n;i++){
        for(j=0;j<n+1;j++)
             cin >> T[i*n+j];

Hm jest mi potrzebna, gdyż przy zamianie kolumn w macierzy muszę zamienić niewiadome w równaniu (przykład poniżej). Chyba, że widzisz jakieś inne rozwiązanie to chętnie też bym się z nim zapoznał.

kopiadanych() tworzy kopie wprowadzonych danych gdyż oryginał, może przydać się później.

tablica() alokuje pamięć dla tablicy dwuwymiarowej i zwraca adres do tej tablicy( po prostu nie robię tego w main żeby później nie klepać kodu kilkakrotnie).

Ogólnie program polega na rozwiązaniu układu równań gdzie na: (n) miejscach to niewiadome a na (n+1) to wyraz wolny.

Ryan · 8 Styczeń 2015 18:49

Tak jest OK. Ale jeśli nie musisz rozróżniać kolumn od wierszy (np. czytasz kolejne wartości z konsoli), to możesz to zrobić tak:

for (int i = 0; i < w * h; ++i) {
  cin >> tab[i];
}

Jeśli potrzebujesz aktualne X i Y do zadania odpowiedniego pytania użytkownikowi, możesz tak jak Ty chcesz, albo dodać tymczasową zmienną indeksującą (pozbędziesz się obliczeń w pętli). Jak ze wszystkim w programowaniu: nie ma jednego słusznego rozwiązania - należy dobrać takie, które w danym miejscu sprawdzi się najlepiej. Co znaczy najlepiej? Zależy co w danym miejscu w kodzie jest ważne. Ja cenię sobie przede wszystkim czytelność i przewidywalność działania kodu, a to niestety jest mocno umowna kwestia.

Swoją drogą: złym pomysłem jest mieszanie iostream i pdcurses. To, czy odczyt i zapis z/do buforów (klawiatura/ekran) są ze sobą synchronizowane jest zależne od implementacji. A że pdcurses jest biblioteką zewnętrzną, istnieje spora szansa, że coś się pokiełbasi, jeśli będziesz przeplatał getch() z cin. Skoro używasz curses, używaj ich konsekwentnie.

Nie, bo Twoje kolejne niewiadome to kolejne wiersze (czy tam rzędy) macierzy, prawda? Więc indeks zmiennej jest funkcją jednej z pozycji. Skoro wyświetlasz z ciągiem formatującym, to nic nie stoi na przeszkodzie, żebyś użył printw(“x%d”, some_idx), prawda? Nie przechowujesz tam, z tego co widziałem, niczego czego nie możesz w dowolnej chwili “wyczarować” z dostępnych zmiennych.

Może się przydać, czy się przydaje? Implementowanie rzeczy na zapas rzadko ma sens. A moje czepialstwo nie dotyczyło tego co robi, a nazewnictwa. Rozumiem co znaczy “kopia danych” no i siłą rzeczy uruchomiłem ten kod w debugerze, więc widziałem co robi. Ta aplikacja ma wiele różnych danych, a kopiadanych() duplikuje pewien konkretny wycinek danych aplikacji. Nazwa funkcji powinna to odzwierciedlać.

Nie inaczej jest z innymi funkcjami w Twoim kodzie. I ze zmiennymi: nic nie stoi na przeszkodzie, żebyś n zmienił na np. matrix_width i wprowadził matrix_height (czy tam mat_w/mat_h), które opisuje wysokość. Znikają +1 w wielu miejscach, a zmiennym nadajesz poprzez nazwę konkretną funkcję. Dzięki temu śledząc program można zrozumieć jaka była intencja autora (i jeśli działanie programu nie odzwierciedla intencji, to wiemy gdzie jest błąd). Odrobina komentarzy też by nie zaszkodziła. Nawet jeśli piszesz dla siebie - wyrabiaj sobie dobre nawyki jak najszybciej, bo później będziesz cierpiał pisząc coś poważniejszego z takimi przyzwyczajeniami.

transporter22 · 8 Styczeń 2015 20:38

Dzięki, każde właściwe rady się ceni.

Ryan · 8 Styczeń 2015 20:46

Jeszcze jedna rzecz. Cały ten switch w menu możesz zmienić na coś takiego (pisane z palca, więc pewnie gdzieś się walnąłem, ale ogólna zasada powinna być OK):

char *teksty[] = {text1, text2, text3, text4, text5, text6}; // ktory bedzie 0..5, nie 1..6 jak masz obecnie
for (int i = 0; i < 6; ++i) {
  if (ktory == i) attron(A_REVERSE);
  mvprintw(wx / 2 - 3 + i, wy / 2 - strlen(teksty[i]) / 2, teksty[i]);
  if (ktory == i) attroff(A_REVERSE);
}
mvprintw(wx/2+3,wy/2,"");

Podobnie ma się sprawa ze switchem w main.

transporter22 · 8 Styczeń 2015 21:07

W sumie faktycznie, prosty if a cieszy. Hm tak to jest jak się piszę na ostatnią chwile po świętach.

Jakoś, tego nie mogę ogarnąć.( nie wyobrażam sobie zamiany kolumn z zapamiętaniem indeksów tablicy bez jakiej kol wiek tablicy czy char czy tam int którę zapamiętają te zmiany).

Ryan · 8 Styczeń 2015 21:25

Zmęczenie się wkradło - faktycznie, zamieniasz kolejność. T_T

Żeby pozbyć się dodatkowej tablicy jedyne co mogę zaproponować, to dodatkowy wymiar istniejącej - trzymać tam możesz ID (float jest w stanie poprawnie reprezentować liczby całkowite od 0 do 2^24, więc nie będziesz miał problemu z precyzją). Ale jeśli zostawisz dodatkową tablicę, to sugeruję zmienić ją z char* na chociażby int* - char wygląda tutaj dziwnie (to indeks, a nie literka, poza tym ograniczasz się do 9-10 wymiarów).

transporter22 · 8 Styczeń 2015 21:32

Tak wiem, wcześniej przechowywałem a,b,c,… i stąd się wzięło char :P Dzięki piwo i Ci się należy. Hehe ps. z forum można więcej wynieść niż z wykładu, parodia polskich uczelni.

Ryan · 8 Styczeń 2015 21:38

Uczelnie są od pokazywania czego można się nauczyć, a nie od nauczania. Ale z każdego miejsca - forum, książki czy uczelni - wyciąga się tyle, ile się chce wyciągnąć. Uważam, że z odpowiednim podejściem wszędzie można wyłapać coś wartościowego. Spytaj tych z podejściem roszczeniowym, czy można coś wyciągnąć z forum, a powiedzą Ci, że nie.