C/C++ - C programma liste doppiamente concatenate

salve a tutti, ho un problema con questo programma e non riesco a trovare una soluzione: quando lo lancio dopo aver completato il ciclo for per la seconda scanf va in loop e non mi da più nulla, cosa ho sbagliato?

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

struct list{
    int value;
    struct list *next;
    struct list *prec;
};
    
void print (struct list *li){
    while(li != NULL){
        printf("%i ", li->value);
        li = li->next;
    }
}

void destroy (struct list *l){
    if (l != NULL){
        struct list *c = l;
        struct list *p = NULL;
        while(c!= NULL){
            if ( p!= NULL)
                free(p);
        };
    };
}
    

int main (void){
    int n = 10;
    int i;
    struct list *p = (struct list * ) malloc(sizeof(struct list));
    struct list *tail;
    scanf("%d", &p->value);
    tail = p;
    while ( n != 1){
        tail->next = (struct list *) malloc(sizeof(struct list));
        tail = tail->next;
        scanf("%d", &tail->value);
        tail->prec = p;
        p = p->next;
        n--;
    };
    tail->next = NULL;
    print (p);
    destroy(p);
}

    
        

Guarda questa funzione:



Il ciclo continua finchè C non è NULL (e non lo sarà mai, visto che le assegni un valore presumibilmente non NULL che non cambia), e la funzione free() si attiva se P non è NULL (ma P è sempre NULL, perchè le assegni il valore NULL e poi non la modifichi più.


Tra parentesi, avresti potuto trovare tu stesso questi errori con un minimo di debug... ti consiglierei di concentrarti molto su quest'aspetto, perchè un buon debug svolto da chi è stato coinvolto nella scrittura del codice spesso è risolutivo ed utile (soprattutto in ambito didattico) più di qualsiasi "consiglio esterno"

grazie per la correzione, proprio non lo avevo notato; comunque adesso il programma si chiude ma la funzione print non mi stampa la lista, c'è qualche errore nell'elaborazione della lista? magari ho sbagliato la parte di codice dove associo i puntatori a il valore precedente?

Il problema sta nel while subito prima della chiamata di print(): tu scrivi

 while ( n != 1){
        tail->next = (struct list *) malloc(sizeof(struct list));
        tail = tail->next;
        scanf("%d", &tail->value);
        tail->prec = p;
        p = p->next;
        n--;
    }; 

Ma quello spostamento di P (assolutamente inutile comunque, visto che per puntare alla fine della lista usi già TAIL) fa sì che, alla fine del ciclo, esso punti all'ultimo elemento della lista: praticamente, hai perso il tuo puntatore alla testa.

Una forma corretta del codice è:

while (n != 1){
		tail->next = (struct list *) malloc(sizeof(struct list));
		tail = tail->next;
		scanf("%d", &tail->value);
		tail->prec = p;
		n--;
	};

Detto questo, rileggendo il tuo codice ho notato alcune cose cui dovresti fare attenzione:

1 - Tu utilizzi, in tutte le funzioni, i parametri come variabili "modificabili": per esempio, qui

void print(struct list *li){
	while (li != NULL){
		printf("%i ", li->value);
		li = li->next;
	}
}

Usi il puntatore facendolo "scorrere" lungo la lista. Questo non è strettamente sbagliato, ma senz'altro non aiuta la leggibilità e favorisce una condotta potenzialmente rischiosa: in questo caso non succede nulla, perchè il puntatore è passato by value e dunque ad essere oggetto di modifica è solo il valore della sua "copia" inserita nello stack frame della funzione print(), ma se per caso andassi a modificare il valore di ciò a cui esso punta produrresti dei cambiamenti anche nella variabile originale (perchè andresti a modificare il dato effettivamente puntato), e non è detto che tu voglia questo... fidati, è un errore che può capitare, soprattutto quando si scrive tanto codice
Sarebbe meglio, a mio avviso, definire una variabile puntatore locale cui assegnare il valore del parametro e da far "scorrere" al suo posto

2 - La funzione main() è di tipo int, dunque deve ritornare un valore.

3 - Tutte le scanf che hai inserito non sono precedute da alcuna stampa su console... praticamente, un ipotetico utente del tuo programma deve immaginare che tu stia aspettando un suo cenno di vita, e deve anche immaginare che tu ti aspetti che lui inserisca un intero!

Dovresti aggiungere un controllo errori : malloc può ritornare NULL!
E con quel codice sarebbe un disastro!

Riferito al punto 3: no, scherzi, io adoro scoprire da me come funzionano i programmi

grazie per l'aiuto, comunque non ho messo printf prima degli scanf perchè è un programma di esercizio e quindi sapevo cosa dovevo inserire; ovviamente se fosse un programma "per il pubblico" le avrei messe specificando anche; seconda cosa: potreste spiegarmi perchè il puntatore che fa scorrere la testa è sbagliato? Togliendolo poi tutti i valori sono comunque legati doppiamente anche al parametro prec o si collegano solo al primo? poi potresti scrivermi un esempio del creare un puntatore locale come hai detto al posto di usare quello passato? (giusto per avere un idea di come si fa in modo giusto)
Per il resto ora il programma funziona (credo ) quindi grazie mille per gli aiuti dati

In senso meramente "grammaticale" non è scorretto (il C permette quell'istruzione)... lo è concettualmente: di fatto, tu effettui gli inserimenti aggiungendo i nodi alla fine della lista, quindi il nodo di testa non cambia mai, e non ha senso che tu faccia scorrere il suo puntatore, perchè di fatto lo mandi a puntare un nodo intermedio, che non corrisponde a quello di testa.
Ti faccio un esempio: se hai questa lista

|1|->|2|->|3|

Il tuo puntatore alla testa P punta al valore 1, giusto? Bene, se tu inserisci in coda un altro nodo (come fai nel tuo programma), hai la lista:

|1|->|2|->|3|->|4|

Dove, fai attenzione, il nodo di testa non è cambiato (e quindi, non ha senso far cambiare il "bersaglio" di P): è cambiato solo il nodo di coda.



Analizza il codice dentro il while:

Punto 1: crei un nuovo nodo successivo alla coda attuale

		tail->next = (struct list *) malloc(sizeof(struct list));

Punto 2: sposti il puntatore alla coda in modo che punti al nuovo "ultimo nodo"

		tail = tail->next;

Punto 3 (che risponde alla tua domanda): imposti il campo PREC del tuo ultimo nodo in modo che punti al suo predecessore (il "vecchio" ultimo nodo).

		tail->prec = p;

Quindi, di fatto, vai a creare una lista del tipo (la rappresento in verticale "stile albero" e con soli 4 nodi, per comodità:

NODO 1: |value = 1; prec = NULL; succ = NODO 2| <- p
NODO 2: |value = 2; prec = NODO 1; succ = NODO 3|
NODO 3: |value = 3; prec = NODO 2; succ = NODO 4|
NODO 4: |value = 4; prec = NODO 3; succ = NULL| <- tail

Che è una lista 2-linkata, proprio come volevi che fosse: ogni elemento punta sia al suo predecessore che al suo successore




Attenzione: quello che ti ho proposto non è "IL MODO GIUSTO" in senso assoluto, perchè anche quello che hai fatto tu è ammissibile: semplicemente, io ti ho proposto un modo in generale meno rischioso, più lineare e più leggibile (soprattutto in casi in cui ti trovi a dover gestire molti dati - penso, per esempio, ad implementazioni dell'algoritmo di Er o di certi altri algoritmi ricorsivi) di gestire il rapporto "variabili/parametri"
Comunque, a titolo di esempio cito la tua funzione di stampa: tu l'hai scritta così:

    void print(struct list *li){
            while (li != NULL){
                    printf("%i ", li->value);
                    li = li->next;
            }
    }

Ecco, io ti propongo di riscriverla nella forma:

    void print(struct list *li)
   {
            struct list *temp = li;

            while (temp != NULL)
           {
                    printf("%i ", temp->value);
                    temp = temp->next;
            }
    }

In questo modo, l'output non è cambiato affatto (ottieni comunque la stampa di tutti i valori), ma hai ottenuto una versione più chiara e leggibile del codice, dove:

- I parametri non cambiano significato in corso d'opera: LI puntava alla testa della lista all'inizio, e continua a puntarvi alla fine
- Una variabile locale appositamente definita si occupa di lavorare sul valore passato dal parametro

È molto più ordinata e leggibile di prima, non credi?

grazie mille per la risposta, ora penso di aver capito molto di più