wskaźnik do struktury w strukturze

0

Mam 2 struktury. Jedna jest zawarta w drugiej. Chcę zdefiniować tablicę struktur "team", z tablicą struktur "informacje_o_przeciwniku" w środku każdej struktury "team".
Chcę przydzielać pamięć do tych struktur dynamicznie.
Napisałem kod ,ale mi nie działa. Tzn, działa jak odpalam poprzez run, ale jak śledzę przebieg debuggerem to wyskakuję błąd:
"projekt_3_3 2179 cygwin_exception: Dumping stack trace to projekt_3_3.exe.stackdump"
Czy to co napisałem ma sens? Czy to co chcę zrobić jest możliwe? Czy jest inny sposób by osiągnać zamierzony efekt?

Potrzebuję trzymać informacje o drużynie sportowej i o meczach które rozegrała z innymi drużynami w jakiś sposób.

struct team{
    char nazwa[51];
    struct informacje_o_przeciwniku *rywal;
    int punkty;
    int roznica_bramek;
    int bramki_zdobyte_w_sumie;
};

struct informacje_o_przeciwniku{
    char przeciwnik[51];
    int bramki_zdobyte;
    int bramki_stracone;
};
int main(int argc, char** argv){
    char temp_ch[51];
    char temp_ch2[51];
    char temp_ch3[51];
    char temp_ch4[51];
    int liczba_druzyn = 0;
    int liczba_druzyn_w_sumie = 0;
    struct team *druzyna;
    struct team *temp;
    druzyna = (struct team *) malloc(2 * sizeof (struct team));
    druzyna->rywal = (struct informacje_o_przeciwniku *) malloc(2 * sizeof (struct team));

    while (1){
        printf(">");
        if (strcmp(temp_ch, "q") == 0){
            break;
        }
        scanf("%s", temp_ch);
        if (strcmp(temp_ch, "new") == 0){
            printf("Podaj liczbe druzyn: ");
            scanf("%d", &liczba_druzyn);
            liczba_druzyn_w_sumie += liczba_druzyn;

            if ((temp = (struct team *) realloc(druzyna, liczba_druzyn_w_sumie * sizeof (struct team))) == NULL){
                printf("Nie udało sie zaalokowac pamieci  1\n");
                exit(EXIT_SUCCESS);
            } else{
                druzyna = temp;
            }

            for (int i = 0; i < liczba_druzyn_w_sumie; i++){
                if (((temp + i)->rywal = (struct informacje_o_przeciwniku *) realloc(((druzyna + i)->rywal), (liczba_druzyn_w_sumie - 1) * sizeof (struct informacje_o_przeciwniku))) == NULL){
                    printf("Nie udało sie zaalokowac pamieci  2\n");
                    exit(EXIT_SUCCESS);
                } else{
                    (druzyna + i)->rywal = (temp + i)->rywal;
                }
            }

            for (int i = liczba_druzyn_w_sumie - liczba_druzyn; i < liczba_druzyn_w_sumie; i++){
                printf("Druzyna %d: ", i - (liczba_druzyn_w_sumie - liczba_druzyn) + 1);
                scanf("%s", &druzyna[i].nazwa);
            }
            goto koniec_petli;
        }
}
2

Zacząć należy od tego, że ten kod nie jest zdatny do debugowania, bo najprawdopodobniej nie wkleiłeś tutaj całego programu. Zmodyfikowałem go lekko i sobie jakoś poradziłem, ale na przyszłość jeśli chcesz uzyskać pomoc to wrzuć tutaj cały kod. No to tak:

  1. Jeśli obsługujesz błędy to gdy błąd wystąpi nie wychodź z programu poprzez exit(EXIT_SUCCESS) tylko exit(EXIT_FAILURE).

  2. Zobacz ten fragment kodu:

druzyna = (struct team *) malloc(2 * sizeof (struct team));
    druzyna->rywal = (struct informacje_o_przeciwniku *) malloc(2 * sizeof (struct team));

Po co tutaj ta liczba 2? Przecież na początku alokujesz tylko po jednym obiekcie.

  1. Jeśli chcesz stworzyć tablicę struktur używając do tego sterty to najlepiej jest to zrobić po prostu w ten sposób w przypadku Twojego kodu:
if ((druzyna = (struct team*)realloc(druzyna, liczba_druzyn * sizeof(struct team))) == NULL) {
                fprintf(stderr, "%s\n", "Nie udalo sie zaalokowac pamieci");
                exit(EXIT_FAILURE);
 }

Nie musisz tworzyć bez sensownych zmiennych tymczasowych żeby obsłużyć błędy. Dodatkowo kiedy wypisujesz informację o błędzie to wypisuj na stderr zamiast stdout.

  1. Jeśli chodzi o pętlę, która zajmuje się strukturami rywal to tam realloc jest raczej złym pomysłem. Wystarczy trzymać gdzieś jakąś zmienną oznaczającą indeks przeciwnika i zwiększać ją z każdą iteracją pętli o wartość liczba_drużyn, żeby nie wpływać na obiekty zaalokowane wcześniej. (Ty przechodziłeś po wszystkich obiektach od początku i zwiększałeś ich pamięć niepotrzebnie co doprowadziło do strasznego jej marnowania) Jak już mówiłem realloc jest w tym przypadku funkcją niepotrzebną, ale malloc już tak, bo rezerwujemy pamięć dla pojedynczego obiektu w pamięci, a nie zwiększamy te obiekty, które już istnieją. (może nie obiekty, a konkretniej struktury)
for (int i = rywal_idx; i < liczba_druzyn_w_sumie; i++) {
                if (((druzyna + i)->rywal = (struct informacje_o_przeciwniku *)malloc(sizeof(struct informacje_o_przeciwniku))) == NULL) {
                    fprintf(stderr, "%s\n", "Nie udało sie zaalokowac pamieci  2");
                    exit(EXIT_FAILURE);
                }
}

rywal_idx += liczba_druzyn;

  1. Zmienna liczba_druzyn_w_sumie musi wynosić 1 podczas jej definiowania ze względu na wcześniejszą alokację obiektu.

  2. Nadpisujesz bufor zaalokowany na stosie używając do tego funkcji scanf("%s", temp_ch); i to jest klasyczne Buffer based Overflow->https://en.wikipedia.org/wiki/Buffer_overflow. Nadpisujesz też obiekty zaalokowane na stercie tą samą funkcją, a to prowadzi do Heap based Overflow->https://en.wikipedia.org/wiki/Heap_overflow

  3. Nie używaj goto, bo prowadzi do chaosu w kodzie. https://stackoverflow.com/questions/3517726/what-is-wrong-with-using-goto

  4. Zmienne najlepiej jest nazywać po angielsku. Jeśli już koniecznie chcesz po polsku to przynajmniej nie mieszaj ich z angielskimi odpowiednikami. Dobieraj też lepiej ich nazwy, bo druzyna to słaba nazwa dla zbioru struktur/"obiektów".

Nie wkleiłeś całego kodu, ale mam nadzieję, że gdzieś zwalniasz tę pamięć. ;)

1

do tego co napisano wcześniej:

  1. nie zwalniasz pamięci
  2. zamiast
druzyna->rywal = (struct informacje_o_przeciwniku *) malloc(2 * sizeof (struct team));

winno chyba być:

druzyna->rywal = (struct informacje_o_przeciwniku *) malloc(2 * sizeof (struct informacje_o_przeciwniku));
0

@Shizzer:
.3. Wzorowałem się na tej stronie Realloc

.4. Twój sposób stworzy dynamicznie 1(jedną) strukturę informacje_o_przeciwniku w każdej strukturze team, a po co mi tworzyć dynamicznie strukturę jeśli jej rozmiar jest zawsze 1?
Ja potrzebuję zmieniać rozmiar informacje_o_przeciwniku dynamicznie i to zmieniać dla tych struktur zaaloakowanych wcześniej.
Jak wprowadzę do systemu 3 drużyny to każda będzie mogła rozegrać mecz z 2 innymi drużynami, czyli będzie:
3 elementowa tablica struktur team, a w środku każdego elementu będzie 2 elementowa tablica struktur informacje_o_przeciwniku.
Następnie jak dodam 2 kolejne drużyny to będę Potrzebował:
5 elementowa tablica struktur team, a w środku każdego elementu będzie 4 elementowa tablica struktur informacje_o_przeciwniku i dane które wprowadziłem wcześniej powinny być nienaruszone.

.6. Nie jestem pewien czy dobrze rozumiem. Funkcja scanf("%s", temp_ch) może nadpisać dane, tylko jeśli podam łańcuch dłuższy niż rozmiar temp_ch tak? W jaki sposób w takim razie powinienem pobierać stringi?

.7. goto używam tylko do wychodzenia z wielokrotnie zagnieżdżonych pętli i do przechodzenia na sam koniec głównej pętli programu. To chyba nie jest zła praktyka(?).

1

.4. Twój sposób stworzy dynamicznie 1(jedną) strukturę informacje_o_przeciwniku w każdej strukturze team, a po co mi tworzyć dynamicznie strukturę jeśli jej rozmiar jest zawsze 1?

Rozmiar struktury nie jest równy 1 tylko jest równy sizeof(struct informacje_o_przeciwniku), czyli 59 bajtów + wyrównanie. Obiekt/struktura jest jedna.

3 elementowa tablica struktur team, a w środku każdego elementu będzie 2 elementowa tablica struktur informacje_o_przeciwniku.
Następnie jak dodam 2 kolejne drużyny to będę Potrzebował:
5 elementowa tablica struktur team, a w środku każdego elementu będzie 4 elementowa tablica struktur informacje_o_przeciwniku i dane które wprowadziłem wcześniej powinny być nienaruszone.

W takim razie stosowanie funkcji realloc jest uzasadnione i dobrze zaimplementowane, ale wywal te zmienne tymczasowe, bo są niepotrzebne.

.6. Nie jestem pewien czy dobrze rozumiem. Funkcja scanf("%s", temp_ch) może nadpisać dane, tylko jeśli podam łańcuch dłuższy niż rozmiar temp_ch tak? W jaki sposób w takim razie powinienem pobierać stringi?

Bufory alokowane są na stosie, a funkcja scanf pozwala pisać do tych buforów, czyli piszesz w efekcie po pamięci procesu. scanf("%s") nie dba o to ile danych dokładnie wprowadził użytkownik, czyli użytkownik może w efekcie wprowadzić tyle danych ile chce. W efekcie dzieje się coś takiego:

STOS:

# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #

Bufor [51 bajtów]

# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #

Zapisany adres ramki stosu dla funkcji [8 bajtów]

# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #

Adres powrotu [8 bajtów]

# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #

Przypuśćmy, że użytkownik wpisze 70 liter A jako input.

Stos będzie wyglądał w ten sposób:

# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #

AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA

# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #

AAAAAAAA

# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #

AAAAAAAA

# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #

i tak dalej

# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #

Skutek jest taki, że atakujący nadpisuje adres powrotu funkcji main w Twoim przypadku. Skoro atakujący ma możliwość nadpisania adresu powrotu to może "skoczyć" w dowolne miejsce w pamięci i na przykład wykonać złośliwy kod. Jakbyś chciał się więcej o tym dowiedzieć to sobie poczytaj o Buffer Overflow. Bezpieczne pobieranie ciągów znaków oferuje funkcja fgets->https://www.tutorialspoint.com/c_standard_library/c_function_fgets.htm i to ją powinieneś używać. Adresy będą miały rozmiar 8 bajtów jeśli Twój plik binarny jest 64-bitowy, w przypadku 32-bitowych plików binarnych adresy będą miały rozmiar 4 bajtów.

.7. goto używam tylko do wychodzenia z wielokrotnie zagnieżdżonych pętli i do przechodzenia na sam koniec głównej pętli programu. To chyba nie jest zła praktyka(?).

To jest chyba jedyny przypadek gdzie goto nie jest złą praktyką. Ale z Twojego kodu wynika, że break byłby lepszym rozwiązaniem jeśli wstawiłbyś break w miejsce goto, bo nie wychodzisz tam z zagnieżdżonych pętli.

0

Uzywanie goto do wychodzenia z zagniezdzonych petli jest bardzo zla praktyka. Bo to oznacza ich skomplikowanie. Przy rozwijaniu kodu - spotkana autentyczna przygoda z kodem, ktory byl wlasnie wielokrotnie skomplikowany przez iles osob, pozniej okazalo sie, ze fragment musi byc chroniony semaforem, ktos ladnie zrobil w okreslonym miejscu taki blok, a tu okazalo sie, ze w srodku w jakims tam warunku bylo nieladne wyjscie z zagniezdzonych petli/warunkow i omijala zwolnienie semafora...

Pech chcial, ze warunek ten zachodzil rzadko, ale zachodzil (wiec nie wywalalo sie na dziendobry). Urzadzenia z tym kodem z nieznanych przyczyn sie wieszaly od czasu do czasu i watchdog musial reagowac resetem urzaadzenia.

Byly tu 2 zle rzeczy:

  1. doprowadzenie do takiego zagniezdzenia - jesli dochodzi - znaczy sie jest potrzebna refaktoryzacja tego fragmentu kodu
  2. jesli w okreslonej chwili nie ma na to czasu, jednak nie stosowac wyjsc typu return, goto itp w miejscach, gdzie mozna to przeoczyc.

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