PROBLEMA Segmentation fault in C

Pubblicità

Madg96

Nuovo Utente
Messaggi
141
Reazioni
8
Punteggio
38
Salve ragazzi, qualcuno gentilmente mi aiuterebbe a risolvere questo segmentation fault, nel codice è spiegato tutto quello che il programma fa tramite commenti
Ma in poche parole devo creare una classifica per il gioco del tris e questo codice parte quando la partita termina (il progetto non prevede persistenza dei risultati)

C:
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <sys/stat.h>


typedef struct {
    char* nickname;
    int vittorie;
    int pareggi;
    int sconfitte;
} score;

typedef struct nodo{
  score giocatore;
  struct nodo* next;
} nodo;

void setPunteggio (char* player1, int risultato);
void stampaClassifica(nodo* head);
nodo * getPosizione (score player, nodo* head);
void setLista(score player, nodo* head);
nodo* cercaGiocatore(char* player, nodo* head);

int main () {
  /*
  QUANDO TERMINA LA PARTITA I GIOCATORI CHIAMANO RISPETTIVAMENTE LA FUNCTION setPunteggio
  QUESTA CHIAMA A SUA VOLTA LA FUNCTION cercaGiocatore PER VERIFICARE SE IL GIOCATORE SIA GIÀ PRESENTE IN LISTA
  DOPODICHÈ RIEMPIRÀ O AGGIORNERÀ LA STRUTTURA score E AL TERMINE  CHIAMERÀ LA FUNCTION setLista PASSANDOGLI LO score APPENA
  CREATO, QUESTO VERRA INSERITO NELLA LISTA IN MODO ORDINATO TENENDO CONTO PRIMA DEL NUMERO DI VITTORIE POI DEL NUMERO DI PAREGGI E POI QUELLO DI SCONFITTE, SE
  IL GIOCATORE DOVESSE GIÀ ESISTERE ALL INTERNO DELLA LISTA VERRANNO SOLO AGGIORNATI I DATI DEL SUO PUNTEGGIO
 */
    printf("main");
   setPunteggio("madg96", 1);
   setPunteggio("ciruzz", 3);

}


nodo * getPosizione (score player, nodo* head){
  int flag = 0;

  while(head != NULL && flag == 0){
    if(head -> giocatore.vittorie > player.vittorie ) {
      head = head -> next;
    } else if (head -> giocatore.vittorie == player.vittorie){
        if(head -> giocatore.pareggi > player.pareggi){
          head = head -> next;
        } else if(head -> giocatore.pareggi == player.pareggi) {
            if (head -> giocatore.sconfitte < player.sconfitte){
              head = head -> next;
            }else {
              return head;
              flag = 1;
            }
        } else {
          return head;
          flag = 1;
        }
    } else {
      return head;
      flag = 1;
    }
  }

  return head;

}

void setLista(score player, nodo* head){

  if(head == NULL) {
    head = (nodo*) malloc (sizeof(nodo));
    head -> giocatore = player;
    head -> next = NULL;
  } else {
    nodo* curr = NULL;
    nodo* newNode = NULL;
    curr = getPosizione(player, head);
    newNode =(nodo*) malloc (sizeof(nodo));
    newNode -> giocatore = player;
    newNode -> next = curr; // forse *curr o &curr
    curr = newNode;

  }
}


void setPunteggio(char* player, int risultato){
  nodo * getterHead, head;
  score * p;

printf("entro\n");

  getterHead = cercaGiocatore(player, &head);

  if(getterHead == NULL){//IL GIOCATORE NON È PRESENTE NELLA LISTA QUINDI BISOGNA ALLOCARE SPAZIO NELLA LISTA E NELLA STRUTTURA

    //head= (nodo*) malloc(sizeof(nodo)); //ALLOCO SPAZIO NELLA LISTA
     p = (score*) malloc (sizeof(player)); // ALLOCO SPAZIO PER UNA NUOVA ISTANZA DELLA STRUTTURA

    if(risultato == 1){//IL GIOCATORE HA VINTO E SETTO 1 ALLE VITTORIE

        p -> nickname = player;
        p -> vittorie = 1;
        p -> pareggi = 0;
        p -> sconfitte = 0;
        setLista(*p, &head); //MANDO L ISTANZA DELLA STRUCT A SET LISTA PER INSERIRLO IN MODO ORDINATO
      }else if (risultato == 2) { //IL GIOCATORE HA PAREGGIATO E SETTO 1 AI PAREGGI

        p -> nickname = player;
        p -> vittorie = 0;
        p -> pareggi = 1;
        p -> sconfitte = 0;
        setLista(*p, &head);
      } else if (risultato == 3) { //IL GIOCATORE HA PERSO E SETTO 1 ALLE SCONFITTE

        p -> nickname = player;
        p -> vittorie = 0;
        p -> pareggi = 0;
        p -> sconfitte = 1;
        setLista(*p, &head);
        }
      } else { // NEL CASO IN CUI IL GIOCATORE SIA GIÀ PRESENTE IN LISTA TROVO LA SUA ISTANZA E AGGIUNGO 1 IN BASE AL RISULTATO OTTENUTO

        if(risultato == 1 ){// QUESTI IF ELSE FUNZIONANO ALLO STESSO MODO DI QUELLI SOPRA, IN BASE AL RISULTATO AGGIUNGONO PUNTI AL GIOCATORE

          getterHead -> giocatore.vittorie += 1;
        } else if ( risultato == 2){

          getterHead -> giocatore.pareggi += 1;
        } else if ( risultato == 3){

          getterHead -> giocatore.sconfitte += 1;
        }

      }

    //stampaClassifica(&head);
  }


nodo* cercaGiocatore(char* player, nodo* head){

int trovato = 0;

while(head != NULL && trovato == 0){
  if(strcmp(head -> giocatore.nickname, player) == 0) {
    trovato = 1;
    return head;
  }
  head = head -> next;
}

return NULL;

}
 
Non l'ho nemmeno provato, ma questo è già un errore:

C:
            }else {
              return head;
              flag = 1;
            }
        } else {
          return head;
          flag = 1;
        }
    } else {
      return head;
      flag = 1;
    }

a che ti serve il flag? Non verrà mai raggiunta quell'istruzione.

Un errore che sicuro crea problemi è questo:

C:
  p = (score*) malloc (sizeof(player)); // ALLOCO SPAZIO PER UNA NUOVA ISTANZA DELLA STRUTTURA

player è un puntatore a char. Perchè mai castarlo a score*?

Inoltre:

C:
 nodo * getterHead, head;

head non è un puntatore qui.

Penso di averti dato un buon numero di spunti...
 
Non l'ho nemmeno provato, ma questo è già un errore:

C:
}else {
return head;
flag = 1;
}
} else {
return head;
flag = 1;
}
} else {
return head;
flag = 1;
}

a che ti serve il flag? Non verrà mai raggiunta quell'istruzione.

Un errore che sicuro crea problemi è questo:

C:
p = (score*) malloc (sizeof(player)); // ALLOCO SPAZIO PER UNA NUOVA ISTANZA DELLA STRUTTURA

player è un puntatore a char. Perchè mai castarlo a score*?

Inoltre:

C:
nodo * getterHead, head;

head non è un puntatore qui.

Penso di averti dato un buon numero di spunti...
Grazie mille, per quanto riguarda il primo errore, si sono un po' un coglionazzo, il secondo è stato un errore di distrazione in quanto prima la mia struct si chiamava player poi per non fare confusione con la variabile player ed avere conflitti l ho chiamata score, mentre per l ultimo punto sapresti spiegarmi il perché ? Perché non devo iniziarlo a puntatore ? Credevo che fosse necessario mandare il puntatore alla testa della lista per mandare un "istanza" della lista in un altra funzione. Comunque grazie mille

Inviato da ASUS_I01WD tramite App ufficiale di Tom\'s Hardware Italia Forum
 
Hai provato a metterti in debug per capire il problema? :)
Onestamente no, non ci avevo proprio pensato, ho sempre fatto piccoli programmini in c e non ho mai avuto bisogno del debugger, per fare questo progetto sto usando in vm ubuntu , compilo su terminale e utilizzo l editor "Atom" per usare il debug mi consigli di scaricare un ide tipo "Code:Block" oppure non so , se nello store di Atom ci sia un debug per il c. Grazie della risposta

Inviato da ASUS_I01WD tramite App ufficiale di Tom\'s Hardware Italia Forum
 
Onestamente no, non ci avevo proprio pensato, ho sempre fatto piccoli programmini in c e non ho mai avuto bisogno del debugger, per fare questo progetto sto usando in vm ubuntu , compilo su terminale e utilizzo l editor "Atom" per usare il debug mi consigli di scaricare un ide tipo "Code:Block" oppure non so , se nello store di Atom ci sia un debug per il c. Grazie della risposta

Inviato da ASUS_I01WD tramite App ufficiale di Tom\'s Hardware Italia Forum
Ti ha suggerito già Andretti :)

Il debugger è una cosa molto utile per scovare gli errori. Non sottovalutare i piccoli programmini :D
 
Grazie mille, per quanto riguarda il primo errore, si sono un po' un coglionazzo, il secondo è stato un errore di distrazione in quanto prima la mia struct si chiamava player poi per non fare confusione con la variabile player ed avere conflitti l ho chiamata score, mentre per l ultimo punto sapresti spiegarmi il perché ? Perché non devo iniziarlo a puntatore ? Credevo che fosse necessario mandare il puntatore alla testa della lista per mandare un "istanza" della lista in un altra funzione. Comunque grazie mille

Inviato da ASUS_I01WD tramite App ufficiale di Tom\'s Hardware Italia Forum

Ti riferisci a questo immagino:
C:
nodo * getterHead, head;

in questo caso stai dichiarando getterHead come puntatore al tipo nodo, ma head è solo di tipo nodo, non un puntatore, come invece secondo me ti aspetti tu.

Come semplice esempio guarda questo codice:
C:
#include <stdio.h>

int main() {
    int *n, n1;
    int b = 100;
    
    n1 = &b;
    n  = &b;
    
    printf("%d, %d", n1, n);
    
    return 0;
}

qui n è un puntatore a int, ma n1 è solo di tipo int.
Compilando viene dato questo warning sulla riga che assegna l'indirizzo di b alla variabile n1:

Codice:
test.c: In function 'main':
test.c:7:5: warning: assignment to 'int' from 'int *' makes integer from pointer without a cast [-Wint-conversion]
    7 |  n1 = &b;
      |     ^

Anyway, concordo con Ibernato e Andretti.
Il debugger è molto importante. In ambiente Linux c'è anche Radare2. In ambiente Windows hai l'imbarazzo della scelta, ma i più comodo da usare sono probabilmente OllyDbg, ImmunityDebugger e x64dbg.

Poi se usi un IDE hai il debugger integrato dove ti è sufficiente settare i breakpoint nell'editor, e non vedrai nemmeno del codice assembly.
 
Pubblica nuovamente tutto il codice dopo le correzioni apportate (quello sopra non dovrebbe più essere coerente con quello che hai ora).
 
Pubblica nuovamente tutto il codice dopo le correzioni apportate (quello sopra non dovrebbe più essere coerente con quello che hai ora).
Certo , ecco qui, l' unico problema ora è che quando entro in cerca giocatore head è inizializzato ma non è uguale a NULL anche se la lista è vuota, infatti alla riga 92 lo inizializzo io a Null, ovviamente non è una soluzione definitiva ma evita il segmentation fault, posterò di nuovo il codice quando avrò finito e l avrò testato
C:
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <sys/stat.h>


typedef struct {
    char* nickname;
    int vittorie;
    int pareggi;
    int sconfitte;
} score;

typedef struct nodo{
  score giocatore;
  struct nodo* next;
} nodo;

void setPunteggio (char* player1, int risultato);
void stampaClassifica(nodo* head);
nodo * getPosizione (score player, nodo* head);
void setLista(score player, nodo* head);
nodo* cercaGiocatore(char* player, nodo* head);

int main () {
  /*
  QUANDO TERMINA LA PARTITA I GIOCATORI CHIAMANO RISPETTIVAMENTE LA FUNCTION setPunteggio
  QUESTA CHIAMA A SUA VOLTA LA FUNCTION cercaGiocatore PER VERIFICARE SE IL GIOCATORE SIA GIÀ PRESENTE IN LISTA
  DOPODICHÈ RIEMPIRÀ O AGGIORNERÀ LA STRUTTURA score E AL TERMINE  CHIAMERÀ LA FUNCTION setLista PASSANDOGLI LO score APPENA
  CREATO, QUESTO VERRA INSERITO NELLA LISTA IN MODO ORDINATO TENENDO CONTO PRIMA DEL NUMERO DI VITTORIE POI DEL NUMERO DI PAREGGI E POI QUELLO DI SCONFITTE, SE
  IL GIOCATORE DOVESSE GIÀ ESISTERE ALL INTERNO DELLA LISTA VERRANNO SOLO AGGIORNATI I DATI DEL SUO PUNTEGGIO
 */
 int a = 1;
    printf("main\n");
   setPunteggio("madg96", 1);
   setPunteggio("ciruzz", 3);

}


nodo * getPosizione (score player, nodo* head){
  int flag = 0;

  while(head != NULL && flag == 0){
    if(head -> giocatore.vittorie > player.vittorie ) {
      head = head -> next;
    } else if (head -> giocatore.vittorie == player.vittorie){
        if(head -> giocatore.pareggi > player.pareggi){
          head = head -> next;
        } else if(head -> giocatore.pareggi == player.pareggi) {
            if (head -> giocatore.sconfitte < player.sconfitte){
              head = head -> next;
            }else {
                flag = 1;
              return head;
            }
        } else {
            flag = 1;
          return head;
        }
    } else {
        flag = 1;
      return head;
    }
  }

  return head;

}

void setLista(score player, nodo* head){

  if(head == NULL) {
    head = (nodo*) malloc (sizeof(nodo));
    head -> giocatore = player;
    head -> next = NULL;
  } else {
    nodo* curr = NULL;
    nodo* newNode = NULL;
    curr = getPosizione(player, head);
    newNode =(nodo*) malloc (sizeof(nodo));
    newNode -> giocatore = player;
    newNode -> next = curr; // forse *curr o &curr
    curr = newNode;

  }
}


void setPunteggio(char* player, int risultato){
  nodo* getterHead = NULL;
  nodo* head = NULL;
  score* p;



  getterHead = cercaGiocatore(player, head);

  if(getterHead == NULL){//IL GIOCATORE NON È PRESENTE NELLA LISTA QUINDI BISOGNA ALLOCARE SPAZIO NELLA LISTA E NELLA STRUTTURA

     p = (score*) malloc (sizeof(score)); // ALLOCO SPAZIO PER UNA NUOVA ISTANZA DELLA STRUTTURA

    if(risultato == 1){//IL GIOCATORE HA VINTO E SETTO 1 ALLE VITTORIE

        p -> nickname = player;
        p -> vittorie = 1;
        p -> pareggi = 0;
        p -> sconfitte = 0;
        setLista(*p, head); //MANDO L ISTANZA DELLA STRUCT A SET LISTA PER INSERIRLO IN MODO ORDINATO
      }else if (risultato == 2) { //IL GIOCATORE HA PAREGGIATO E SETTO 1 AI PAREGGI

        p -> nickname = player;
        p -> vittorie = 0;
        p -> pareggi = 1;
        p -> sconfitte = 0;
        setLista(*p, head);
      } else if (risultato == 3) { //IL GIOCATORE HA PERSO E SETTO 1 ALLE SCONFITTE

        p -> nickname = player;
        p -> vittorie = 0;
        p -> pareggi = 0;
        p -> sconfitte = 1;
        setLista(*p, head);
        }
      } else { // NEL CASO IN CUI IL GIOCATORE SIA GIÀ PRESENTE IN LISTA TROVO LA SUA ISTANZA E AGGIUNGO 1 IN BASE AL RISULTATO OTTENUTO

        if(risultato == 1 ){// QUESTI IF ELSE FUNZIONANO ALLO STESSO MODO DI QUELLI SOPRA, IN BASE AL RISULTATO AGGIUNGONO PUNTI AL GIOCATORE

          getterHead -> giocatore.vittorie += 1;
        } else if ( risultato == 2){

          getterHead -> giocatore.pareggi += 1;
        } else if ( risultato == 3){

          getterHead -> giocatore.sconfitte += 1;
        }

      }

    stampaClassifica(head);
  }


nodo* cercaGiocatore(char* player, nodo* head){

int trovato = 0;
int result;

  if(head != NULL){
    while(head != NULL && trovato == 0){
      char * string = head -> giocatore.nickname;
      result = strcmp(string , player);
      if(result == 0) {
        trovato = 1;
        return head;
      }
      head = head -> next;
    }
  }else {
    return NULL;
  }
}

void stampaClassifica(nodo* head){
  int i=1;
  if (head != NULL) {
    while(head != NULL) {
      printf("\n Posizione = %d Nickname = %s Vittorie %d Pareggi %d Sconfitte %d\n ", i, head -> giocatore.nickname, head -> giocatore.vittorie, head -> giocatore.pareggi, head -> giocatore.sconfitte );
      head = head -> next;
    }
  } else{
    printf("\nClassifica vuota\n");
  }
}
 
Ci sono molteplici problemi, anche legati alla logica di ciò che stai facendo. Non riesco a correggerteli ora.

In primis, per copiare il nome del giocatore, devi utilizzare memcpy e non assegnare come stai facendo.
L'errore anche logico è mantenere il puntatore alla head dentro alla funzione setPunteggio(). Questo oltre ad essere logicamente sbaglato causa anche non pochi problemi...

Ogni volta che chiami la funzione head sarà NULL. In buona sostanza ad ogni chiamata a setPunteggio() hai un memory leak che è dato dal fatto che allochi memoria per head, ma non la deallochi mai (e ti perdi il puntatore alla base, avendo sempre una lista vuota). Non è il modo corretto di implementare una linked list.

Ti mostro un esempio veloce:
C:
#include <stdio.h>
#include <stdlib.h>

struct Nodo
{
  struct Nodo   *next;
  size_t         number;
};


void list_add(struct Nodo **nodo, const size_t item)
{
    struct Nodo *new = (struct Nodo*) malloc(sizeof(struct Nodo));
    
    new->next = NULL;
    new->number = item;
    
    // Lista vuota
    if(!*nodo) {
        *nodo = new;
        return;
    }
    
    struct Nodo *pHead = *nodo;
    // Aggiunta in coda
    while(pHead->next != NULL)
        pHead = pHead->next;
    
    pHead->next = new;
}

void print_list(struct Nodo *nodo)
{
    while(nodo) {
        printf("%d\n", nodo->number);
        nodo = nodo->next;
    }
}

int main()
{
  struct Nodo *nodo;
  list_add(&nodo, 10);
  list_add(&nodo, 12);
  list_add(&nodo, 14);
  list_add(&nodo, 16);
  
  print_list(nodo);
  
  // FREE 
}

Il mio è un pò più semplice del tuo, in quanto inserisco sempre in coda, ma l'aspetto importante è il modo in cui tratto il puntatore iniziale alla lista.

Poi come ultima cosa puoi anche migliorare un pò il codice, una volta che ti funziona. Ad esempio, quegli else-if a cascata puoi evitarli utilizzando uno switch.
Ancora meglio se cambi l'algoritmo che usi, e invece di usare 3 variabili per lo stato, usi un array con 3 elementi (i tuoi stati). Quindi un codice come questo:

C:
if(risultato == 1 ){// QUESTI IF ELSE FUNZIONANO ALLO STESSO MODO DI QUELLI SOPRA, IN BASE AL RISULTATO AGGIUNGONO PUNTI AL GIOCATORE

          getterHead -> giocatore.vittorie += 1;
        } else if ( risultato == 2){

          getterHead -> giocatore.pareggi += 1;
        } else if ( risultato == 3){

          getterHead -> giocatore.sconfitte += 1;
        }

puoi trasformarlo in un più semplice:

C:
getterHead->giocatore.stato[risultato]++;

se "risultato" ha come primo valore 1 e non 0, dovrai ovviamente fare getterHead->giocatore.stato[risultato-1]++;.
 
Ci sono molteplici problemi, anche legati alla logica di ciò che stai facendo. Non riesco a correggerteli ora.

In primis, per copiare il nome del giocatore, devi utilizzare memcpy e non assegnare come stai facendo.
L'errore anche logico è mantenere il puntatore alla head dentro alla funzione setPunteggio(). Questo oltre ad essere logicamente sbaglato causa anche non pochi problemi...

Ogni volta che chiami la funzione head sarà NULL. In buona sostanza ad ogni chiamata a setPunteggio() hai un memory leak che è dato dal fatto che allochi memoria per head, ma non la deallochi mai (e ti perdi il puntatore alla base, avendo sempre una lista vuota). Non è il modo corretto di implementare una linked list.

Ti mostro un esempio veloce:
C:
#include 
#include 

struct Nodo
{
struct Nodo *next;
size_t number;
};


void list_add(struct Nodo **nodo, const size_t item)
{
struct Nodo *new = (struct Nodo*) malloc(sizeof(struct Nodo));

new->next = NULL;
new->number = item;

// Lista vuota
if(!*nodo) {
*nodo = new;
return;
}

struct Nodo *pHead = *nodo;
// Aggiunta in coda
while(pHead->next != NULL)
pHead = pHead->next;

pHead->next = new;
}

void print_list(struct Nodo *nodo)
{
while(nodo) {
printf("%d\n", nodo->number);
nodo = nodo->next;
}
}

int main()
{
struct Nodo *nodo;
list_add(&nodo, 10);
list_add(&nodo, 12);
list_add(&nodo, 14);
list_add(&nodo, 16);

print_list(nodo);

// FREE 
}

Il mio è un pò più semplice del tuo, in quanto inserisco sempre in coda, ma l'aspetto importante è il modo in cui tratto il puntatore iniziale alla lista.

Poi come ultima cosa puoi anche migliorare un pò il codice, una volta che ti funziona. Ad esempio, quegli else-if a cascata puoi evitarli utilizzando uno switch.
Ancora meglio se cambi l'algoritmo che usi, e invece di usare 3 variabili per lo stato, usi un array con 3 elementi (i tuoi stati). Quindi un codice come questo:

C:
if(risultato == 1 ){// QUESTI IF ELSE FUNZIONANO ALLO STESSO MODO DI QUELLI SOPRA, IN BASE AL RISULTATO AGGIUNGONO PUNTI AL GIOCATORE

getterHead -> giocatore.vittorie += 1;
} else if ( risultato == 2){

getterHead -> giocatore.pareggi += 1;
} else if ( risultato == 3){

getterHead -> giocatore.sconfitte += 1;
}

puoi trasformarlo in un più semplice:

C:
getterHead->giocatore.stato[risultato]++;

se "risultato" ha come primo valore 1 e non 0, dovrai ovviamente fare getterHead->giocatore.stato[risultato-1]++;.
Mammamia, non so come ringraziarti, sono anni che non usavo le linked list in c, e a quanto vedi stavo avendo non pochi problemi, in più seguirò il tuo consiglio, ma comunque bisogna pensare che questo programma diventerà una libreria per un progetto più grande in ogni caso grazie tante ancora, mi metto subito a lavoro per apportare le correzioni.

Inviato da ASUS_I01WD tramite App ufficiale di Tom\'s Hardware Italia Forum
 
… usi un array con 3 elementi (i tuoi stati). Quindi un codice come questo:
Qui ci sono varie scuole di pensiero. Che il codice sia più compatto non significa necessariamente che sia “migliore” (uso le virgolette perché è un termine relativo). Il problema nell’usare il vettore è che bisogna o ricordarsi cosa corrispondano gli indici o usare un gruppo di #define (o una enumerazione) in modo da evitare di scrivere direttamente il valore numerico dell’indice (il cosiddetto “hardcode” che occorre evitare come la peste). Io sono della scuola che tre variabili con i loro nomi ben definiti siano molto più chiari che un vettore, che preferisco usare solo per quantità omogenee. Ma, ripeto, è una scelta personale.
 
Certo, davo per scontato l'utilizzo di indici utilizzando delle enum o delle #define.
Capisco ciò che vuoi dire comunque; ho consigliato questa strada perchè mi sembra più pulito e semplice da leggere rispetto a un else-if come quello sopra (fosse uno switch, sarebbe più o meno la stessa cosa, anche se "più efficiente" di un else-if). Vedendo quell'istruzione capisci subito cosa fa, senza seguire la logica - seppur semplice ovviamente - di quelle 20 righe circa (parlando anche di quelle sotto nell'else).
Ma come dici tu, si tratta di scelte personali; anzi, talvolta si tratta di scelte fatte per mantenere "uniformità" in un progetto (specie se conosci quelli che ci metteranno mano).
 
Pubblicità
Pubblicità
Indietro
Top