0

I want to delete the first Node of a list (I call it "head") but output give me an error "pointer being freed was not allocated". Do you know where is the issue? Sorry for Italian words if you didn't understand something I can edit my question. I think that the issue is in void cancellaTriangolo. Maybe I saved wrong nodes and they don't save themselves.

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

/* STRUTTURE */
typedef struct punto {
    int x;
    int y;
} PUNTO;

typedef struct triangolo {
    PUNTO v;
    int lato;
} TRIANGOLO;

typedef struct nodo {
    TRIANGOLO t;
    struct nodo *next;
} NODO;

/* STAMPA LISTA */
int perimetro(TRIANGOLO t) {
    return t.lato * 3;
}

void stampaTriangolo(TRIANGOLO t) {
    printf("Il triangolo ha il lato uguale a: %d con un perimetro pari a %d, il vertice in alto ha coordinate (%d,%d)\n",
           t.lato, perimetro(t), t.v.x, t.v.y);
}

void stampaLista(NODO *head) {
    if (head->next == NULL) {
        printf("Lista vuota!\n");
    } else {
        while (head->next != NULL) {
            head = head->next;
            stampaTriangolo(head->t);
        }
    }
}

/* INSERIMENTO */
TRIANGOLO creaTriangolo() {
    TRIANGOLO nuovo;
    printf("Inserisci il lato del nuovo triangolo: ");
    scanf("%d", &nuovo.lato);
    printf("\n");
    printf("Inserisci le coordinate del vertice con y maggiore:\n");
    printf("x: ");
    scanf("%d", &nuovo.v.x);
    printf("\n");
    printf("y: ");
    scanf("%d", &nuovo.v.y);
    printf("\n");
    
    return nuovo;
}

void inserisciPerPerimetro(NODO *head) {

    NODO *nuovo = malloc(sizeof(NODO));
    nuovo->t = creaTriangolo();
    nuovo->next = NULL;

    if (head == NULL) {
        head = nuovo;
    } else {
        if (perimetro(nuovo->t) < perimetro(head->t)) {
            nuovo->next = head;
            head = nuovo;
        } else {
            while (head->next != NULL && perimetro(head->next->t) < perimetro(nuovo->t)) {
                head = head->next;
            }
            nuovo->next = head -> next;
            head->next = nuovo;
        }
    }
    printf("Inserimento effettuato!\n\n");
}

/* CANCELLAZIONE IN TESTA */
void cancellaTriangolo(NODO *head) {
    NODO *primoNodo = NULL;

    /* lista vuota? */
    if (head == NULL) {
        primoNodo = NULL;
        printf("Lista vuota!\n\n");
    } else {
        primoNodo = head;
        head = head->next;
        free(primoNodo);
        printf("Cancellazione effettuata!\n");
    }
}

int main() {
    /* inizializza la lista */
    NODO *head = malloc(sizeof(NODO));
    head->next = NULL;
    
    int risposta = -1;          // per interazione con utente

    while (risposta != 0) {
        /* richiedi un'operazione all'utente */
        printf("Che operazione vuoi svolgere?\n");
        printf("1 -> Inserisci un triangolo nella lista ordinata secondo il perimetro crescente\n");
        printf("2 -> Cancella il triangolo in testa alla lista\n");
        printf("3 -> Visualizza la lista di triangoli\n");
        printf("0 -> Termina il programma\n");
        scanf("%d", &risposta);

        /* gestisci le operazioni dell'utente */
        if (risposta == 1) {
            inserisciPerPerimetro(head);
        } else
        if (risposta == 2) {
            cancellaTriangolo(head);
        } else
        if (risposta == 3) {
            stampaLista(head);
        } else
        if (risposta == 0) {
            printf("Finito!\n\n");
        } else {
            printf("Selezione non valida!\n\n");
        }
    }
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • 1
    When you do `head = nuovo;` in `inserisciPerPerimetro()` be aware that you are only modifying the variable `head` that is _local to inserisciPerPerimetro()_, not the `head` that is local to `main`. – mmixLinus Feb 03 '22 at 15:04
  • Before worrying about _deleting_ you should worry about _creating_ the list in the first place. `inserisciPerPerimetro(head);` does not modify `head`. Parameters are passed by value. – Jabberwocky Feb 03 '22 at 15:05
  • There are many issues I this code. – Jabberwocky Feb 03 '22 at 15:18
  • @mmixLinus ok so what have I to write in inserisciPerPerimetro() to save it?However in real if I try to create and save a new node it works! – Daniele Sadun Feb 03 '22 at 15:33
  • @Jabberwocky so what have I to do?lol – Daniele Sadun Feb 03 '22 at 15:34

2 Answers2

1

(Sorry for not getting the question right)

Edit: The change of paramater head in cancellaTriangolo won't apply to the head in main by simply doing head = head->next inside that function. To solve this issue, code below might help:

void cancellaTriangolo(NODO** head) {
    if (*head) {
        NODO* p = *head;
        *head = (*head)->next;
        free(p);
    }
}

Call it with cancellaTriangolo(&head).

For other issues like when inserting new node... If you found that new nodes are not being inserted, it might be the same reason. Use NODO** head to send the pointer to head in order to access and change head in functions.

MossHawk
  • 21
  • 1
  • 4
1
  1. Your approach of having a head that actually contains no data is wrong and makes your code overly complicated. The first node of a list is by definition the head of the list. If the head is NULL, the list is empty.
  2. In C parameters are passed by value, so if you want to modify a pointer that has been passed into a function, you need pointers to pointers. The details are explained in this SO question: How do I modify a pointer that has been passed into a function in C?.

You basically want this:

It's not the whole code but only the function that were wrong.


void stampaLista(NODO* head) {
  NODO* current = head;

  if (current == NULL) {
    printf("Lista vuota!\n");
  }
  else {
    while (current != NULL) {
      stampaTriangolo(current->t);
      current = current->next;
    }
  }

  printf("\n");
}

void inserisciPerPerimetro(NODO** head) {

  NODO* nuovo = malloc(sizeof(NODO));
  nuovo->t = creaTriangolo();
  nuovo->next = NULL;

  if (*head == NULL) {
    *head = nuovo;
  }
  else {
    if (perimetro(nuovo->t) < perimetro((*head)->t)) {
      nuovo->next = *head;
      *head = nuovo;
    }
    else {
      while ((*head)->next != NULL && perimetro((*head)->next->t) < perimetro(nuovo->t)) {
        *head = (*head)->next;
      }
      nuovo->next = (*head)->next;
      *head->next = nuovo;    
    }
  }

  printf("Inserimento effettuato!\n\n");
}

void cancellaTriangolo(NODO** head) {
  NODO* primoNodo = NULL;

  /* lista vuota? */
  if (*head == NULL) {
    primoNodo = NULL;
    printf("Lista vuota!\n\n");
  }
  else {
    primoNodo = *head;
    *head = (*head)->next;
    free(primoNodo);
    printf("Cancellazione effettuata!\n");
  }
}

int main() {
  /* inizializza la lista */
  NODO* head = NULL;
  ...
    
    if (risposta == 1) {
      inserisciPerPerimetro(&head);  // use &head
    }
    else if (risposta == 2) {
      cancellaTriangolo(&head);      // use &head
    }
    ...
Jabberwocky
  • 48,281
  • 17
  • 65
  • 115
  • thank you very very much!Only one thing: for "inserisciPerPerimetro" and "cancellaTriangolo" are another way using only a *? Because I'm not sure if I can use it at school – Daniele Sadun Feb 03 '22 at 15:58
  • Why should you not be able to use it at school? It's perfectly legal and clean. But anyway, you could drop the `*` I added at various places and return the new head via `return head`; And call it like this: `head = cancellaTriangolo(head);`. Same for `inserisciPerPerimetro`. – Jabberwocky Feb 03 '22 at 16:06
  • yeah I know but we have to use only a *(don't ask me why lol).However I have to remove all * or only where there are two?and the func will became NODO* in this case? – Daniele Sadun Feb 03 '22 at 16:10
  • @DanieleSadun, no only remove the `*` where I added one. Basically your functions will be identical to your original functions (except the first node mess I removed), but at the end you just add `return head;` – Jabberwocky Feb 03 '22 at 16:12
  • @DanieleSadun the function will become `NODO *cancellaTriangolo(...` – Jabberwocky Feb 03 '22 at 16:15
  • I tried but I have to change void func with "NODO* cancellaTriangolo(NODO *head)" and NODO* primoNodo = NULL.(also for inserisciPerPerimetro).and for both return head.I tried to execute and work all perfectly! Is it ok in this way? – Daniele Sadun Feb 03 '22 at 16:19
  • @DanieleSadun if it works as expected, it's probably OK. – Jabberwocky Feb 03 '22 at 16:27