Wskaźnik do elementu listy

Question

Witam.

Pracuję nad listą jednokierunkową. Mam spory problem aby zrozumieć jak przekazać łańcuch znaków do elementu listy. Zapisując listę, za pomocą wskaźnika, losowo generowanymi danymi okazuje się, że wszystkie dane mają wartość ostatniego wylosowanego elementu. Przy przesyłaniu "gotowego" łańcucha nie ma takiego problemu.

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

typedef struct node {
    int a;
    char *b;
    struct node * next;
} list;

list *createList(){
    list * head = NULL;
	head = malloc(sizeof(list));
    return NULL;
}
list *insert(list **head, int _a, char *_b){
    list * temp;
    temp = malloc(sizeof(list));
    temp->a = _a;
    temp->b = _b;
    temp->next = *head;
    *head = temp;
}

void showList(list * head){
    list * temp = head;
    if(temp == NULL)
        printf("Empty \n");
    while (temp != NULL) {
        printf("%d  %s",temp->a, temp->b);
        printf("\n--------------------------------------------------------\n");
        temp = temp->next;
    }
}
int main (){
    time_t czas,start=time(NULL);
    time(&czas);
    srand(time(&czas));
    int a=0,cnt=0;
    char b[4];
    list *list;
	list = createList();

    while(cnt<10){
        a=( rand()%70+50);
        for(int i=0;i<3;i++)
            b[i]=(rand()%26+65);
        cnt++;
        insert(&list,a,b);
        printf("b = %s \n",b);//wyswietla dobrze
    }
    printf("Podaj 3 znaki \n");
    scanf("%s",b);
    insert(&list,1,b);
    showList(list);//zamienia szystko na aktualne b

    printf("Podaj 3 znaki \n");
    scanf("%s",b);
    insert(&list,2,b);
    insert(&list,3,"dziala");//w ten sposob dziala
    showList(list);// ostatnia wartosc b jest przypisana do wszystkich elementow listy
    return 0;
}

Wydawało mi się, że rozumiem wskaźniki ale chyba nie do końca. Czy ktoś mógłby wytłumaczyć dlaczego tak się dzieje.

 

Answer 1

1. list *createList(){
       list * head = NULL;
       head = malloc(sizeof(list));
       return NULL;
   }

   Podręcznikowy wręcz wyciek pamięci. Alokujesz pamięć, zapisujesz jej adres do zmiennej lokalnej, a potem bezsensownie zwracasz NULL, przez co ani nie masz do niej dostępu, ani nie możesz jej zwolnić. Po prostu zwróć zmienną `head`.

2. list *insert(list **head, int _a, char *_b){
       list * temp;
       temp = malloc(sizeof(list));
       temp->a = _a;
       temp->b = _b;
       temp->next = *head;
       *head = temp;
   }

   Wg deklaracji funkcji ma ona zwracać wskaźnik, a nie zwraca niczego.

3. time_t czas,start=time(NULL);
   time(&czas);
   srand(time(&czas));

   Zmienne `czas` i `start` nie są nigdzie używane. Drugie wywołanie funkcji `time` jest bezcelowe. Wystarczyło po prostu wywołać:

   srand(time(NULL));

4. printf("b = %s \n",b);//wyswietla dobrze

   Napis w tablicy `b` nie posiada znaku terminacji '\0' na końcu, co oznacza, że np. `printf` nie wie, jak długi jest string. Wystarczy zrobić tak:

   for(int i=0;i<3;i++)
       b[i]=(rand()%26+65);
   b[3] = '\0';

5. Jest parę innych drobnych błędów, m.in. nie zwalniasz zaalokowanej pamięci, ale nie powinno to wpływać na działanie programu.

EDIT:

Zauważyłem właśnie, że ciągle używasz tej samej tablicy `b`, dla każdego elementu listy. Przez to każdy wskazuje na tę samą pamięć, co skutkuje nadpisywaniem stringów.

Comments

Dzięki.

Jeżeli są jeszcze jakieś błędy śmiało pisz. Na błędach najlepiej się uczy.

A co do mojego problemu, zrobiłem tablicę dwuwymiarową i działa. Czy jest to słuszny trop? Wtedy automatycznie wypełnia mi listę bez nadpisywania.

---

Tablica dwuwymiarowa (tablica stringów) jest tutaj jak najbardziej odpowiednim rozwiązaniem.

Jeżeli są jeszcze jakieś błędy śmiało pisz. Na błędach najlepiej się uczy.

1. Struktura `list` mogłaby zawierać własną tablicę znaków. Wtedy nie musiałbyś tworzyć jej poza listą w funkcji `main`. Aby dużo nie modyfikować kodu, wystarczy zmienić pole `char *b` na `char b[4]` oraz kopiować napis w funkcji `insert` za pomocą `strcpy`.
2. Zamieniłbym typ zwracany `insert` na `void`.

3. int a=0,cnt=0;

   Tutaj niepotrzebnie inicjalizujesz zmienną `a`, jak zaraz później ją nadpisujesz.

4. list *createList(){
       list * head = NULL;
       head = malloc(sizeof(list));
       return head;
   }

   Tutaj tworzysz pierwszy element listy, ale nie inicjalizujesz go. Będzie więc on zawsze posiadał śmieci. Dopiero pozostałe elementy dodane za pomocą `insert` będą poprawnie utworzone. Poprawiona funkcja mogłaby wyglądać tak:

   list *createList(int _a, char *_b){
       list * head = NULL;
       head = malloc(sizeof(list));
       head->a = _a;
       head->b = _b;
       head->next = NULL;
       return head;
   }
   

5. Dodałbym funkcję `removeList`, aby ją wykonać na końcu programu.

---

Zmieniłem char *b` na `char b[4]  i przypisałem strcpy(temp->b ,_b); .Taki sposób chyba dużo praktyczniejszy. Zapis char *tab a char tab[ ] to jednak duża różnica . Dzięki za pomoc.

---

Mógłbyś pokazać ostateczny kod? Chciałbym się upewnić, czy dobrze zrozumiałeś wszystkie moje wskazówki :)

---

Oto i kod. Wszelkie wskazówki mile widziane.

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

typedef struct node {
    int a;
    char b[4];          //zmiana
    struct node * next;
} list;

list *createList(int _a, char *_b){
    list * head = NULL;
    head = malloc(sizeof(list));
    head->a = _a;
    strcpy(head->b ,_b);
    head->next = NULL;
    return head;
}
void  insert(list **head, int _a, char *_b){
    list * temp;
    temp = malloc(sizeof(list));
    temp->a = _a;
    strcpy(temp->b ,_b);
    temp->next = *head;
    *head = temp;
    
}

void showList(list * head){
    list * temp = head;
    if(temp == NULL)
        printf("Empty \n");
    while (temp != NULL) {
        printf("%d  %s",temp->a, temp->b);         printf("\n---------------------------------------------\n");
        temp = temp->next;
    }
}
int main (){
    srand(time(NULL));
    int cnt=0;

    list *list;
    list = createList(1,"ABC");

    while(cnt<10){
        char b[4];
        int a=( rand()%70+50); //tutaj chyba lepiej
        for(int i=0;i<3;i++)
            b[i]=(rand()%26+65);
        b[3] = '\0';
        cnt++;
        insert(&list,a,b);
    }
    showList(list);
    return 0;
}

 

---

Kod wygląda dobrze. Boli mnie jedynie, że nie zwalniasz na końcu pamięci, ale w tak małym programie nie jest to obligatoryjne.

---

Z tą pamięcią chodzi C o to, że nie usuwam elementów listy? Możesz wytłumaczyć o co chodzi? Bo program na pewno nie jest gotowy i muszę dopisać jeszcze parę rzeczy.

---

Funkcją `malloc` alokujesz pamięć, zaś `free` zwalniasz. Funkcja `removeList` mogłaby wyglądać tak:

void removeList(list *head) {
    while (head != NULL) {
        list *next = head->next;
        free(head);
        head = next;
    }
}

---

Ok. Zabieram się aby napisać resztę programu. Dzięki za pomoc oraz cenne wskazówki.

Temat można chyba zamknąć.

---

się dorwałem do kompilatora więc czy mogło by być tak

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

typedef struct node {
    int a;
    char *b;
    struct node * next;
} list;

list *createList(){
    list * head = NULL;
    head = malloc(sizeof(list));
    return head;
}
list *insert(list **head, int _a, char *_b){
    list * temp;
    temp = malloc(sizeof(list));
    temp->a = _a;
    temp->b = _b;
    temp->next = *head;
    *head = temp;
}

void showList(list * head){
    list * temp = head;
    if(temp == NULL)
        printf("Empty \n");
    while (temp != NULL) {
        printf("%d  %s",temp->a, temp->b);
        printf("\n--------------------------------------------------------\n");
        temp = temp->next;
    }
}
void deletelist (list * head){

    list * temp = head;
    if(temp == NULL)
        printf("Empty \n");
    while (temp != NULL) {
        printf("%d  %s",temp->a, temp->b);
        printf("\n-----------------------usuwam---------------------------------\n");
        head = temp->next;
        free((temp->b));
        temp->b=NULL;
        free (temp);
        temp=head;
    }
      if(temp == NULL)
        printf("pusty \n");
}
int main (){
    time_t czas;//start=time(NULL);
    time(&czas);
    srand(time(0));
    int a=0,cnt=0, i=0;
    list *list;
    list = createList();

    while(cnt<10){
        a=( rand()%70+50);
        char *b;
        b=malloc(sizeof(char)*4);
        b[3]=0;
        for(i=0;i<3;i++)
            b[i]=(rand()%26+65);
        cnt++;
        i=0;
        insert(&list,a,b);
        printf("b = %s \n",b);//wyswietla dobrze
    }

    insert(&list,3,"dziala");//w ten sposob dziala
    showList(list);// ostatnia wartosc b jest przypisana do wszystkich elementow listy
    deletelist(list);
    return 0;
}

edit:: po prostu zastanawia mnie czy nie ma wycieków pamięci. Bo działać to działa poprawnie i po drobnych przeróbkach char*b mogą mieć dowolną długość, więc program bardzo zyskuje na funkcjonalności.

Dlatego też zainteresowałem się tym wątkiem (struktura ze wskaźnikiem char* a nie tablicą, tablica char 99% pytań na pasji ), bo najłatwiej zarezerwować pamięć w strukturze, ale zawsze pojawia się problem długości łańcucha, a tu dopiero po wczytaniu do strumienia można ustalić jego długość i zarezerwować odpowiedną pamięć.