[C]Lista jednokierunkowa - problem ze wskaźnikami

[arkadeusz91]

Witam!

Dostałem zadanie aby napisać listę jednokierunkową która będzie przechowywać dane osób. Jak na razie zrobiłem tylko część programu (muszę dodać jeszcze kilka funkcji) niemniej dodawanie elementów jak i wyświetlanie powinno już działać. Powinno, a nie działa niestety. O ile dodawanie elementu żadnych błędów nie pokazuje o tyle przy wyświetlaniu jest problem, bo wywala błąd, że nie może odczytać pamięci, czyli prawdopodobnie gdzieś namieszałem ze wskaźnikami. Niestety jestem w tym języku laikiem i nie za bardzo jestem w stanie ogarnąć gdzie popełniłem błąd. Bardzo bym prosił o przejrzenie kodu i doradzenie co skopałem.

Kod: Zaznacz cały

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


typedef struct osoba
{
	char imie[30];
	char nazwisko[30];
	int wiek;
	char wlosy[15];
	int wzrost;
	struct osoba *nastepny;
}osoba;


typedef struct lista
{
    int licznik;
	osoba *pierwszy;
}lista;


int nowaLista(lista *list)
{
    list->licznik=0;
	list->pierwszy=NULL;
	return 0;
}



int dodaj(lista *list)
{
	osoba *ten=NULL;
	osoba *ostatni=NULL;
	osoba *nowy=NULL;
	ten=(osoba*)malloc(sizeof(osoba));
	ostatni=(osoba*)malloc(sizeof(osoba));
	ten=ostatni=list->pierwszy;
	while(ten)
	{
		ostatni=ten;
		ten=ten->nastepny;
	}
	nowy=(osoba*)malloc(sizeof(osoba));
	nowy->nastepny=NULL;
	printf("Imie: ");
	getchar();
	fgets(nowy->imie, 29, stdin);
	printf("Nazwisko: ");
	fgets(nowy->nazwisko, 29, stdin);
	printf("Wiek: ");
	scanf("%d",&nowy->wiek);
	printf("Kolor wlosow: ");
	getchar();
	fgets(nowy->wlosy, 14, stdin);
	printf("Wzrost(w centymetrach): ");
	scanf("%d",&nowy->wzrost);
	++(list->licznik);
	if(!ostatni)
	{
                
                list->pierwszy=nowy;
                return 0; 
    }
	ostatni->nastepny=nowy;
	
	return 0;
}
int wyswietl(osoba *os)
{
    int i=1;
   osoba *tmp;
   tmp=os;
   while (tmp->nastepny!=NULL)
   {
       printf("%d .Imie: %c Nazwisko: %c Wiek: %d Kolor wlosow: %c Wzrost: %c cm/n",i,tmp->imie,tmp->nazwisko,tmp->wiek,tmp->wlosy,tmp->wzrost);
       i++;
       tmp=tmp->nastepny;
   } 
}
/*int usun(){}
int zapisz(){}*/


int main(int argc, char *argv[])
{
	int a;
	lista list;
	printf("LISTA

");
	
	while(2>1)
	{
		printf("Co chcesz zrobic?:
1.Wyswietl liste
2.Dodaj nowa osobe do listy
3.Wyczysc liste
4.Zapisz liste
5.Zakoncz program
");
		nowaLista(&list);
		scanf("%d",&a);
		switch(a)
		{
			case 1:
                 osoba *os;
                 os=list.pierwszy;
				wyswietl(os);
				break;
			case 2:
                   
					dodaj(&list);
					break;
			/*case 3:
				usun()
				break;
			case 4:
				zapisz()
				break;*/
			case 5:
				return 0;
				break;
		}
	}

	return 0;
}

[MichalPWr]

arkadeusz91, Z chęcią bym pomógł ale przekopanie się przez Twój kod dla mnie i być może dla innym jest nie lada wyzwaniem, a mógłbym pomóc. Zostaw tylko to co niezbędne(to w czym jest problem) i opisz ważniejsze funkcje w Twoim kodzie.

[arkadeusz91]

OK no to może tak:

Kod: Zaznacz cały

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


typedef struct osoba //struktura węzła z wymaganymi danymi
{
	char imie[30];
	char nazwisko[30];
	int wiek;
	char wlosy[15];
	int wzrost;
	struct osoba *nastepny;
}osoba;


typedef struct lista //struktura listy
{
    int licznik;
    osoba *pierwszy; //pierwszy element listy
}lista;


int nowaLista(lista *list)  //Inicjalizacja i zerowanie listy
{
    list->licznik=0;
    list->pierwszy=NULL;
    return 0;
}



int dodaj(lista *list) //dodawanie węzła do listy
{
	osoba *ten=NULL; //bieżący węzeł listy
	osoba *ostatni=NULL; //ostatni element listy
	osoba *nowy=NULL; //nowo tworzony węzeł
	ten=(osoba*)malloc(sizeof(osoba));
	ostatni=(osoba*)malloc(sizeof(osoba));
	ten=ostatni=list->pierwszy;
	while(ten) //ustawia ostatnie element na bieżący i przechodzi do następnego
 	{
		ostatni=ten;
		ten=ten->nastepny;
	}
	nowy=(osoba*)malloc(sizeof(osoba)); //alokacja pamięci i stworzenie nowego węzła
	nowy->nastepny=NULL; 
	printf("Imie: "); //pobranie danych do węzła
	getchar();
	fgets(nowy->imie, 29, stdin);
	printf("Nazwisko: ");
	fgets(nowy->nazwisko, 29, stdin);
	printf("Wiek: ");
	scanf("%d",&nowy->wiek);
	printf("Kolor wlosow: ");
	getchar();
	fgets(nowy->wlosy, 14, stdin);
	printf("Wzrost(w centymetrach): ");
	scanf("%d",&nowy->wzrost);
	if(!ostatni)
	{
                
                list->pierwszy=nowy;
                return 0; 
        }
	ostatni->nastepny=nowy;
	
	return 0;
}


int wyswietl(osoba *os) //funkcja wyświetlająca listę (NIE DZIAŁA!)
{
   int i=1;
   osoba *tmp; //bufor na aktualnie wyświetlaną osobę
   tmp=os; //podstawienie pierwszego elementu listy
   while (tmp->nastepny!=NULL) //wyświetlanie kolejnych pozycji
   {
       printf("%d .Imie: %c Nazwisko: %c Wiek: %d Kolor wlosow: %c Wzrost: %c cm/n",i,tmp->imie,tmp->nazwisko,tmp->wiek,tmp->wlosy,tmp->wzrost);
       i++;
       tmp=tmp->nastepny;
   } 
}


int main(int argc, char *argv[])
{
	int a;
	lista list;
	printf("LISTA

");
	nowaLista(&list); //wywołanie nowej listy
	
	while(2>1)
	{
		printf("Co chcesz zrobic?:
1.Wyswietl liste
2.Dodaj nowa osobe do listy
3.Zakoncz   program
");
		scanf("%d",&a);
		switch(a)
		{
			case 1:
                                osoba *os;
                                os=list.pierwszy;
				wyswietl(os); //wywołanie funkcji wyświetlającej
				break;
			case 2:
                   
					dodaj(&list); //wywołanie dodania elementu
					break;
			
			case 3:
				return 0;
				break;
		}
	}

	return 0;
}

[MichalPWr]

Zostaw tylko to co niezbędne(to w czym jest problem)

To co na prawdę skrajnie niezbędne. Mogę poświęcić 15min, żeby rozwiązać problem, ale nie godzinę na przebijanie się przez kod, z którego większość jest niepotrzebna(jeśli chodzi o problem).

[arkadeusz91]

Problem jest gdzieś w tej funkcji:

Kod: Zaznacz cały

int wyswietl(osoba *os) //funkcja wyświetlająca listę (NIE DZIAŁA!)
{
   int i=1;
   osoba *tmp; //bufor na aktualnie wyświetlaną osobę
   tmp=os; //podstawienie pierwszego elementu listy
   while (tmp->nastepny!=NULL) //wyświetlanie kolejnych pozycji
   {
       printf("%d .Imie: %c Nazwisko: %c Wiek: %d Kolor wlosow: %c Wzrost: %c cm/n",i,tmp->imie,tmp->nazwisko,tmp->wiek,tmp->wlosy,tmp->wzrost);
       i++;
       tmp=tmp->nastepny;
   }
}

toteż oznaczyłem to w komentarzu z tym że możliwe że problem jest już na poziomie tworzenia węzła lub w wywołaniu funkcji dlatego na wszelki wypadek dodałem resztę kodu. Gubię się strasznie w tych wskaźnikach...

[Dasio11]

Kod: Zaznacz cały

int wyswietl(osoba *os) //funkcja wyświetlająca listę (NIE DZIAŁA!)
{
   int i=1;
   osoba *tmp; //bufor na aktualnie wyświetlaną osobę
   tmp=os; //podstawienie pierwszego elementu listy
   while (tmp->nastepny!=NULL) //wyświetlanie kolejnych pozycji
   {
       printf("%d .Imie: %c Nazwisko: %c Wiek: %d Kolor wlosow: %c Wzrost: %c cm/n",i,tmp->imie,tmp->nazwisko,tmp->wiek,tmp->wlosy,tmp->wzrost);
       i++;
       tmp=tmp->nastepny;
   }
}

1. Składowe: imię, nazwisko i kolor włosów są c-stringami, czyli tablicami charów. Odpowiadający im specyfikator typu to %s, a nie %c. Jednocześnie, wzrost jest intem, więc powinien mu odpowiadać %d. Poza tym, znak przejścia do nowej linii to '\n', a nie '/n'.

2. Skoro warunek pętli to tmp->nastepny!=NULL, to pętla nie wykona się dla ostatniej osoby na liście, tj. takiej, której następnik jest NULLem. To oznacza, że wyświetli się cała lista z wyjątkiem ostatniej osoby. W drugą stronę, jeśli tmp jest początkowo NULLem, to odwołanie do tmp->nastepny spowoduje błąd dostępu do pamięci, bo to dereferencja wskaźnika NULL.
Zamiana na while(tmp != NULL) zlikwiduje oba problemy.

3. Funkcja wyświetl w swoim przeznaczeniu nie zwraca wyniku - dlatego powinna być typu void, a nie int.

Czyli ostatecznie, poprawiony kod to:

Kod: Zaznacz cały

void wyswietl(osoba *os) //funkcja wyświetlająca listę (NIE DZIAŁA!)
{
   int i=1;
   osoba *tmp; //bufor na aktualnie wyświetlaną osobę
   tmp=os; //podstawienie pierwszego elementu listy
   while (tmp != NULL) //wyświetlanie kolejnych pozycji
   {
       printf("%d .Imie: %s Nazwisko: %s Wiek: %d Kolor wlosow: %s Wzrost: %d cm\n",
              i,tmp->imie,tmp->nazwisko,tmp->wiek,tmp->wlosy,tmp->wzrost);
       i++;
       tmp=tmp->nastepny;
   }
}

Co trochę mniej ważne, nie potrzebujesz zmiennej tmp. Możesz od razu działać na zmiennej os, bo to i tak lokalna kopia, która zniknie po zakończeniu wykonywania funkcji.


Edit: zerknąłem jeszcze na funkcję dodaj i jest tam błąd:

Kod: Zaznacz cały

    ten=(osoba*) malloc(sizeof(osoba));
    ostatni=(osoba*)malloc(sizeof(osoba));
    ten=ostatni=list->pierwszy;

Najpierw alokujesz pamięć, a zaraz potem gubisz do niej adres, bo zmienne, które te adres zawierały, natychmiast otrzymują nową wartość list->pierwszy. Takie działanie powoduje, że zaalokowana pamięć pozostaje niedostępna dla reszty świata, a i ty nie możesz jej użyć, bo tracisz jej adres - zatem ta pamięć jest bezużyteczna aż do zakończenia działania twojego programu.
Wykreśl te dwie linijki z malloc - wystarczy ta jedna, która jest niżej:

Kod: Zaznacz cały

nowy=(osoba*)malloc(sizeof(osoba));

[arkadeusz91]

Dzięki wielkie za pomoc! Co do pierwszego punktu to tylko błędy przez niedbałość i pisanie na szybko które pewnie tak czy siak bym poprawił. Ale bardzo mi pomogłeś. Dzięki