Funkcje do stosu - prośba o sprawdzenie kodu

0

Hejo!
ostatnio uzywam Visual Studio 2013 w wersji Pro. Napisalem sobie funkcje do stosu i prosilbym o sprawdzenie. Dodam ze gdy odpale recznie skompilowane exeki to nie ma zadnego problemu. Natomiast gdy w trybie debug zaraz po kompilacji program sie zaladuje i po wpisaniu 2[enter] i jeszcze raz dostaje blad:

o8e2cd.jpg

Dodatkowo po wywaleniu tego bledu program sam sie nie zamyka. Robie to recznie i konsola sie zamyka po dobrych 3-4 sekundach. Dlaczego?
o co chodzi?

#define _CRT_SECURE_NO_WARNINGS
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

struct lista
{
	int key;
	struct lista *next;
};

void menu()
{
	printf("\tMenu: \n1. Push (+) \n2. Pop (-) \n3. IsEmpty() \n4. First \n5. Czyszczenie \n6. Exit\n\n\tWybor: ");
}

struct lista *Push(struct lista *head, int liczba)
{
	struct lista *nowy = (struct lista*)malloc(sizeof(struct lista));
	nowy->next = NULL;
	nowy->key = liczba;
	if (head != NULL)
		nowy->next = head;
	return nowy;
}

struct lista *Pop(struct lista *head)
{
	if (head == NULL)
		printf("ERROR Stos jest pusty!\n");
	else
	{
		struct lista *temp = head;
		head = head->next;
		printf("Usuwam %d\n", temp->key);
		free(temp);
		temp = NULL;
		return head;
	}
}
	
void First(struct lista *head)
{
	if (head != NULL)
		printf("%d \n", head->key);
	else
		printf("Nic tu nie ma\n\n");
}

bool IsEmpty(struct lista *head)
{
	if (head == NULL)
		return true;
	else
		return false;
}

void Clear(struct lista *head)
{
	while (IsEmpty(head)==false)
		Pop(head);
}

int main()
{
	struct lista *head = NULL;
	Push(head, 3);
	Push(head, 2);
	Push(head, 1);
	int wybor = 0, wartosc;
	while (wybor != 6)
	{
		menu();
		
		scanf("%d", &wybor);
		switch (wybor)
		{
		case 1:
			printf("\n\tWpisz wartosc: ");
			scanf("%d", &wartosc);
			head = Push(head, wartosc);
			break;
		case 2:
			head=Pop(head);
			break;
		case 3:
			(IsEmpty(head) == true) ? printf("Stos jest pusty\n") : printf("Stos nie jest pusty\n");
			break;
		case 4:
			First(head);
			break;
		case 5:
			Clear(head);
			break;
		case 6:
			EXIT_SUCCESS;
		}
		printf("_____________________________________\n\n");
	}
	system("PAUSE");
	return 0;
}

dodanie obrazka to załączników i treści - @furious programming

1

To jest w ogóle źle napisane, bo masz wycieki pamięci itd...
Tutaj masz coś co swego czasu popełniłem:

#include <stdio.h>
#include <stdlib.h>

struct Stos{
    char *imie;
    char *nazwisko;
    struct Stos *nastepny;
};

void push(struct Stos **korzen, char *imie, char *nazwisko){
    struct Stos *element = (struct Stos*)malloc(sizeof(struct Stos));
    element->imie = imie;
    element->nazwisko = nazwisko;
    element->nastepny = NULL;

    if(*korzen == NULL){
        *korzen = element;
    }
    else{
        element->nastepny = *korzen;
        *korzen = element;
    }
}

struct Stos* pop(struct Stos **korzen){
    if(*korzen == NULL)
        return 0;
    else{
        struct Stos *wynik = *korzen;
        if((*korzen)->nastepny != NULL)
            *korzen = (*korzen)->nastepny;
        return wynik;
    }
}

void wypisz(struct Stos *korzen){
    if(korzen != NULL){
        int i=0;
        struct Stos *wsk;
        for(wsk = korzen; wsk->nastepny != NULL; wsk = wsk->nastepny){
            printf("%d. %s %s\n",i + 1, wsk->imie, wsk->nazwisko);
            i += 1;
        }
    }
}

void zniszcz(struct Stos **korzen){
    if(*korzen != NULL){
        struct Stos *temp;
        while((*korzen)->nastepny != NULL){
            temp = *korzen;
            *korzen = (*korzen)->nastepny;
            free(temp);
        }
        free(*korzen);
    }
}

int main(void)
{
    struct Stos *stos = NULL;
    push(&stos, "Grzegorz", "M.");
    push(&stos, "Magdalena", "G.");
    push(&stos, "Tomasz", "S.");
    push(&stos, "Elzbieta", "G.");

    printf("\nPrzed popem:\n");
    wypisz(stos);

    struct Stos *element = pop(&stos);

    printf("\nPo popie:\n");
    wypisz(stos);

    printf("\nPopniety: %s %s\n", element->imie, element->nazwisko);
    free(element);

    zniszcz(&stos);
    return 0;
}
0

Dzieki za odpowiedz ale jak sam napisales ze mam wycieki pamieci a sam nie wiem gdzie to mam to nawet jak przeczytalem twoj kod to tego nie widze. help plox

1

Poprawiłem Twój program. Teraz nie masz wycieków ani pomyłek. Porównaj sobie kod swój z moim poprawionym:

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

struct lista
{
    int key;
    struct lista *next;
};

void menu()
{
    printf("\tMenu: \n1. Push (+) \n2. Pop (-) \n3. IsEmpty() \n4. First \n5. Czyszczenie \n6. Exit\n\n\tWybor: ");
}

void Push(struct lista **head, int liczba)
{
    struct lista *nowy = (struct lista*)malloc(sizeof(struct lista));
    nowy->next = NULL;
    nowy->key = liczba;
    if (*head != NULL){
        nowy->next = *head;
        *head = nowy;
    }
}

struct lista *Pop(struct lista **head)
{
    if (*head == 0){
        printf("ERROR Stos jest pusty!\n");
        return 0;
    }
    else
    {
        struct lista *temp = *head;
        if((*head)->next != NULL)
            *head = (*head)->next;
        else
            *head = NULL;
        return temp;
    }
}

void First(struct lista *head)
{
    if (head != NULL)
        printf("%d \n", head->key);
    else
        printf("Nic tu nie ma\n\n");
}

bool IsEmpty(struct lista *head)
{
    if (head == NULL)
        return true;
    else
        return false;
}

void Clear(struct lista **head)
{
    if(*head != NULL){
        struct lista *temp;
        while((*head)->next != NULL){
            temp = *head;
            *head = (*head)->next;
            free(temp);
        }
        free(*head);
        *head = NULL;
    }
}

int main()
{
    struct lista *head = NULL;
    Push(&head, 3);
    Push(&head, 2);
    Push(&head, 1);
    int wybor = 0, wartosc;
    while (wybor != 6)
    {
        menu();

        scanf("%d", &wybor);
        switch (wybor)
        {
        case 1:{
            printf("\n\tWpisz wartosc: ");
            scanf("%d", &wartosc);
            Push(&head, wartosc);
            break;
        }
        case 2:{
            struct lista *element = Pop(&head);
            if(element!=NULL){
                printf("%d\n",element->key);
                free(element);
            }
            break;
        }
        case 3:{
            (IsEmpty(head) == true) ? printf("Stos jest pusty\n") : printf("Stos nie jest pusty\n");
            break;
        }
        case 4:{
            First(head);
            break;
        }
        case 5:{
            Clear(&head);
            break;
        }
        case 6:{
            Clear(&head);
            EXIT_SUCCESS;
        }
        }
        printf("_____________________________________\n\n");
    }
    return 0;
}

Błędy, które robiłeś:

  • po pierwsze do funkcji Push (tak samo Clear oraz Pop) przekazywałeś kopię wskaźnika. Wskaźnik przekazywany jest przez wartość. Żeby przekazać wskaźnik przez wskaźnik należy użyć dwóch gwiazdek;
  • po drugie nie stosowałeś zwalniania zasobów tam gdzie trzeba było - patrz użycie funkcji free;
  • po trzecie nie zerowałeś wskaźników wobec czego nawet po sprawdzeniu warunku 'czy coś tam == NULL' dostawałeś nań negatywny wynik;
0

czesc. mozesz mi wyjasnic kod:

void Push(struct lista **head, int liczba)
{
    struct lista *nowy = (struct lista*)malloc(sizeof(struct lista));
    nowy->next = NULL;
    nowy->key = liczba;
    if (head != NULL){
        nowy->next = *head;
        *head = nowy;
    }
} 

head jest wskaznikiem do wskaznika, ale nie rozumiem po co i dlaczego.

na poczatku mallocujemy miejsce w pamieci ale jezeli stos jest pusty to sie nic nie doda. ten fragment kodu chyba nie jest dobry.

1

Na początek odpowiedź na drugie pytanie:

Racja, tam powinno być jeszcze else, w którym nowy staje się korzeniem listy. W swoim wcześniejszym listingu to dodałem ale tutaj przez niedopatrzenie zapomniałem:

void Push(struct lista **head, int liczba)
{
    struct lista *nowy = (struct lista*)malloc(sizeof(struct lista));
    nowy->next = NULL;
    nowy->key = liczba;
    if (*head != NULL){
        nowy->next = *head;
        *head = nowy;
    }
    else *head = nowy;
}

Co do podwójnego wskaźnika... Wskaźnik również jest przekazywany przez wartość więc jeżeli chcesz go modyfikować to należy go przekazać przez wskaźnik. Ot po prostu dopisujesz kolejną gwiazdkę w argumencie i każdym odwołaniu do tego argumentu w ciele funkcji.

0

Em mozesz polecic jakis krotki artukul najlepiej dla zielonych dot. **? slyszalem cos o tym ale nigdy nie uzywalem.
Mam jeszcze 2 pytania. Czasami kompiluje projekt i zostaje skompilowany natomiast exe nie chce sie uruchomic i dostaje komunikat 'omowa dostepu'
Drugie pytanie jak zrobic plik *.exe tylko z jednego pliku? chodzi mi o to aby nie tworzyc za kazdym razem w VS pustego projektu, bo nawet do takiego VS dodaje jakies pliki ktorych istnienia nie rozumiem i uwazam ze tak jak np w code::blocks czy nawet devie c++ do takich programow wystarczy pojedynczy plik zrodlowy *.c.

1

Apropos podwójnego wskaźnika:
http://stackoverflow.com/questions/7271647/what-is-necessity-for-putting-double-pointer-while-adding-node-in-a-linked-list

Czasami kompiluje projekt i zostaje skompilowany natomiast exe nie chce sie uruchomic i dostaje komunikat 'omowa dostepu'

Odmowa dostępu pojawia się kiedy program jest uruchomiony (exec), a Ty chcesz go debugować. Proces jest zajęty i VS nie może dostać się do execa żeby zrobić debugowanie.

Drugie pytanie jak zrobic plik *.exe tylko z jednego pliku? chodzi mi o to aby nie tworzyc za kazdym razem w VS pustego projektu, bo nawet do takiego VS dodaje jakies pliki ktorych istnienia nie rozumiem i uwazam ze tak jak np w code::blocks czy nawet devie c++ do takich programow wystarczy pojedynczy plik zrodlowy *.c.

Nie poradzisz. Każde IDE tworzy sobie swoje pliki projektów. Chcesz mieć jeden plik to kompiluj programy z linii poleceń przez GCC:)

0

Wracaj do Push()

void Push(struct lista **head, int liczba)//oczekujemy wskaznika do wskaznika
{
    struct lista *nowy = (struct lista*)malloc(sizeof(struct lista));
    nowy->next = NULL;
    nowy->key = liczba;
    if (*head != NULL){//jezeli wskazni jest rozny od 0?
        nowy->next = *head;
        *head = nowy;
    }
    else *head = nowy;
}

nie wiem jak mam rozumiec zapis *head tutaj. I takie byc moze glupie pytanie czy zawsze NULL to to samo co 0?

i jescze do Pop()

struct lista *Pop(struct lista **head)
{
	if (*head){
		struct lista *temp = *head;
		if ((*head)->next != NULL)
			*head = (*head)->next;
		else
			*head = NULL;
		return temp;//zwracamy tutaj temp. czyli zwracamy head? to po co go tworzylismy
	}
	else
	{
		printf("ERROR Stos jest pusty!\n");
		return 0;
	}
} 
1

nie wiem jak mam rozumiec zapis *head tutaj. I takie byc moze glupie pytanie czy zawsze NULL to to samo co 0?

To jest dokładnie to

struct lista *head;

z funkcji main tylko przekazane przez wskaźnik. To jest właśnie ten wskaźnik, który został przekazany (przez wskaźnik - stąd dwie gwiazdki) jako argument funkcji push. W przeciwnym wypadku operowałbyś na kopii korzenia i miałbyś wycieki pamięci.

Co do NULL: http://www.cplusplus.com/reference/cstring/NULL/
NULL to generalnie 0x0 -> http://c-for-dummies.com/blog/?p=177

//zwracamy tutaj temp. czyli zwracamy head? to po co go tworzylismy

Temp wskazuje aktualny element Stosu. Po ustawieniu wskaźnika na aktualny element stosu możemy go przesunąć jeżeli posiada następny element.
Jeżeli zrobiłbyś:

*head = (*head)->next;

bez tempa to miałbyś wyciek pamięci, bo utraciłbyś obiekt będący głową stosu przed zamianą na next. Co innego gdybyś przechowywał w strukturze dwa wskaźniki tj. na poprzedni i następny element. Wtedy tak - mógłbyś pominąć tworzenie tempa.

PS: Co do wycieków pamięci to zainteresuj się takim programem jak valgrind dla Linuksa. Najlepiej zainstalować sobie jakiegoś Ubunciaka na wirtualu i sprawdzać tam programy.

1 użytkowników online, w tym zalogowanych: 0, gości: 1