Lista jednokierunkowa

pytanie zadane 17 lipca 2020 w C i C++ przez kaminie318 Bywalec (2,070 p.)

Witam. Mam pytanie odnośnie usuwania listy jednokierunkowej. Dlaczego podana funkcja usuwająca jest błędna ? Wyskakuje błąd i nie mam pojęcia dlaczego.

void historiaRozgrywek(historiaGry** pGlowa,char* napis)
{
    if (*pGlowa == NULL)
    {
       *pGlowa = (historiaGry*)malloc(sizeof(historiaGry));
       (*pGlowa)->ktoWygral = napis;
       (*pGlowa)->nastepny = NULL;
    }
    else
    {
        historiaGry* kolejnyRekord = (historiaGry*)malloc(sizeof(historiaGry));
        kolejnyRekord->ktoWygral = napis;
        kolejnyRekord->nastepny = *pGlowa;
        *pGlowa = kolejnyRekord;
    }
}


void usunHistorieRozgrywek(historiaGry** pGlowa)
{
    if (*pGlowa)
    {
        usunHistorieRozgrywek((*pGlowa)->nastepny);
        free(*pGlowa);
        *pGlowa = NULL;
    }
}

komentarz 17 lipca 2020 przez Wiciorny Ekspert (270,590 p.)

free(*pGlowa);
        *pGlowa = NULL;

zwalniasz wskaźnik, potem ustawiasz na null?

komentarz 17 lipca 2020 przez kaminie318 Bywalec (2,070 p.)

Słyszałem że tak się robi, aby potem uniknąć "niespodzianek" z pamięcią. Też wydaje mi się to dziwne, ale w każdym bądź razie na pewno nie w tym tkwi problem

2 odpowiedzi

odpowiedź 17 lipca 2020 przez j23 Mędrzec (194,920 p.)
wybrane 17 lipca 2020 przez kaminie318

Najlepsza

void usunHistorieRozgrywek(historiaGry* pGlowa)
{
    if (pGlowa) {
        usunHistorieRozgrywek(pGlowa->nastepny);
        free(pGlowa);
    }
}

Użycie rekurencji nie jest dobrym pomysłem. Tym bardziej, że można to w prosty sposób zrobić w pętli while.

komentarz 17 lipca 2020 przez kaminie318 Bywalec (2,070 p.)

Robiłem iteracyjnie, wyskakiwały wycieki pamięci więc chciałem kombinować rekurencją.

P.S znowu wyciek ale dlaczego :/

komentarz 17 lipca 2020 przez j23 Mędrzec (194,920 p.)

Jeszcze to:

(*pGlowa)->ktoWygral = napis;

To prawdopodobnie jest źle (zależy, jak wywołujesz historiaRozgrywek).

Wersja iteracyjna:

void usunHistorieRozgrywek(historiaGry* pGlowa)
{
    while (pGlowa) {
    	historiaGry* tmp = pGlowa->nastepny;
        free(pGlowa->ktoWygral); // <--- prawdopodobnie to też powinno być.
        free(pGlowa);
        pGlowa = tmp;
    }
}

komentarz 17 lipca 2020 przez kaminie318 Bywalec (2,070 p.)

Dlaczego jest to źle ?

komentarz 17 lipca 2020 przez kaminie318 Bywalec (2,070 p.)

Jak dodałem linijke free(pGlowa->ktoWygral); wyskakuje przerwanie, nie załadowano wntdll.pdb :/ Nie spotkałem się jeszcze z takim usuwaniem listy.

komentarz 17 lipca 2020 przez j23 Mędrzec (194,920 p.)

Jak pisałem, zależy jak wywołujesz historiaRozgrywek. Jeśli podajesz jako argument łańcuch "tymczasowy", to element listy będzie zawierać nieprawidłowy wskaźnik (bo kopiujesz tylko wartość wskaźnika, a nie zawartość spod niego). IMO tak to powinno wyglądać:

void historiaRozgrywek(historiaGry** pGlowa, const char* napis)
{
	...
	(*pGlowa)->ktoWygral = strcpy(malloc(strlen(napis) + 1), napis);
	...
}

Funkcję usuwającą masz wyżej.

komentarz 17 lipca 2020 przez kaminie318 Bywalec (2,070 p.)

Zgadza się, są to łańcuchy znaków podawane w zależności od sytuacji w grze(wygrałeś, przegrałeś, remis), jednak cały czas mam wycieki, mimo poprawek, meh ten c.

komentarz 17 lipca 2020 przez j23 Mędrzec (194,920 p.)

Pokaż, jak poprawiłeś historiaRozgrywek.

komentarz 17 lipca 2020 przez kaminie318 Bywalec (2,070 p.)

void historiaRozgrywek(historiaGry** pGlowa,const char* napis)
{
    if (*pGlowa == NULL)
    {
       *pGlowa = (historiaGry*)malloc(sizeof(historiaGry));
       (*pGlowa)->ktoWygral = strcpy(malloc(strlen(napis) + 1), napis);
       (*pGlowa)->nastepny = NULL;
    }
    else
    {
        historiaGry* kolejnyRekord = (historiaGry*)malloc(sizeof(historiaGry));
        kolejnyRekord->ktoWygral = strcpy(malloc(strlen(napis) + 1), napis);
        kolejnyRekord->nastepny = *pGlowa;
        *pGlowa = kolejnyRekord;
    }
}

komentarz 17 lipca 2020 przez j23 Mędrzec (194,920 p.)

Jeśli usunHistorieRozgrywek jest dokładnie taka, jak podałem - iteracyjna, z dwoma wywołaniami free, to wycieków nie szukałbym w tych funkcjach.

komentarz 17 lipca 2020 przez kaminie318 Bywalec (2,070 p.)

Zakomentowałem wszystkie funkcje związane z listą i jest okej, co prawda mam tam jeszcze dynamiczną tablicę ale ona nie powoduje wycieków. Nie wiem co jest grane, może main coś rozjaśni.

int main()
{
    _CrtSetDbgFlag(_CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF);
    char wybor;
    int licznikZwyciestwKomputera = 0, licznikTwoichZwyciestw = 0;
    historiaGry* glowaListy = (historiaGry*)malloc(sizeof(historiaGry));
    glowaListy = NULL;
    //OTWORZ PLIK
    do 
    {
        int* planszaDoGry = malloc(25 * sizeof(int));
        zasadyGry();
        plansza(planszaDoGry);
        printf("Komputer: O, Ty: X\nWybierz,czy chcesz zaczac pierwszy(1) czy drugi(2)? ");
        int ktoZaczyna;
        scanf("%d", &ktoZaczyna);
        for (unsigned int kolej = 0; kolej < 25 && rezultatGry(planszaDoGry) == 0; kolej++)
        {
            if ((kolej + ktoZaczyna) % 2 == 0)
            {
                //ruchKomputera(planszaDoGry);
            }
            else
            {
                rysujPlansze(planszaDoGry);
                ruchGracza(planszaDoGry);
            }
        }
        switch (rezultatGry(planszaDoGry))
        {
        case 0:
            printf("REMIS! Dobra robota!\n");
            //REMIS
            break;
        case 1:
            rysujPlansze(planszaDoGry);
            printf("PRZEGRALES! Przykro mi\n");
            break;
        case -1:
            rysujPlansze(planszaDoGry);
            printf("WYGRALES!!! Gratulacje!\n");
            historiaRozgrywek(&glowaListy, "ZWYCIESTWO: TY");
            break;
        }

        system("PAUSE");
        UsunPlansze(planszaDoGry);
        printf("Czy chcesz grac dalej(Y or N)?");
        wybor = _getch();
        printf("\n\n");
    } while (wybor=='Y' || wybor=='y');
    //ZAMKNIJ PLIK
    pokaListe(glowaListy);
    usunHistorieRozgrywek(glowaListy);
    return 0;
}

komentarz 17 lipca 2020 przez j23 Mędrzec (194,920 p.)

historiaGry* glowaListy = (historiaGry*)malloc(sizeof(historiaGry));
glowaListy = NULL;

Po co ten malloc, skoro ma być NULL?

komentarz 17 lipca 2020 przez kaminie318 Bywalec (2,070 p.)

mallokiem rezerwuje pamięć, a przyrównanie do nulla jest po to, aby zabezpieczyć się przed wskazaniem na coś, zawsze robiłem tak z listą i działało.

1	komentarz 17 lipca 2020 przez j23 Mędrzec (194,920 p.) To powoduje wyciek. Przypisanie nulla powoduje, że to, co przydzieliłeś wcześniej, zostaje "zgubione". Ten malloc jest zbędny, wystarczy przypisanie nulla.

komentarz 17 lipca 2020 przez kaminie318 Bywalec (2,070 p.)

Dziękuję W życiu nie wpadłbym na to. Czy to jest jakaś różnica względem c++? Tam zawsze tak robiłem i chyba nigdy nie miałem wycieków z tego powodu. W ogóle teraz jak na to patrze to dziwne wydaje mi się że korzystam z pamięci która nie jest zaalokowana, tylko wskazuje na NULL :P Chociaż w funkcji i tak ją alokuje gdy pGlowa nie wskazuje na nic.

komentarz 17 lipca 2020 przez j23 Mędrzec (194,920 p.)

Czy to jest jakaś różnica względem c++?

Jeśli chodzi o ręczne zarządzanie pamięcią, nie ma różnicy.

W C++ nie musisz implementować list, bo te są już w bibliotece standardowej.

Tam zawsze tak robiłem i chyba nigdy nie miałem wycieków z tego powodu.

No właśnie, chyba...

1	komentarz 17 lipca 2020 przez Oscar Nałogowiec (29,320 p.) @kaminie318, to strcpy(malloc(strlen... robi to samo co funkcja strdup.

odpowiedź 17 lipca 2020 przez Oscar Nałogowiec (29,320 p.)

O ile dobrze domyśliłem się deklaracji to w wywołaniu rekurencyjnym (linia 23) wywołujesz funkcję z parametrem historiaGry* a powinno być historiaGry**.