liste dinamiche - Segmentation fault - C

[NaTEn]

Ciao a tutti.
Prima di tutto mi presento, dato che sono nuovo del forum, mi chiamo Fabio e spero di riuscire a trovare una soluzione ad un mio problema.
Ho scritto il sorgente di un programma che:
Crea una lista dinamica semplice (contenente sono una variabile int e il puntatore al valore seguente).
Il programma chiede all'utente quanti numeri vuole inserire e alloca memoria per la lista.
Dopo aver fatto ciò ordina la lista in ordine crescente. L'ordinamento l'ho effettuato per parametro, quindi spostando i puntatori al elemento successivo della lista.

La parte dell'inserimento (insCoda) funziona.
L'altro sottoprogramma no, mi da segmentation fault precisamente al primo if.

Ho provato a rileggerlo qualche volta, ma non riesco a trovare l'errore.
Posto di seguito il sorgente, se qualcuno può aiutarmi grazie mille.

Fabio.

[NaTEn]

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


//struttura (lista)
typedef struct _arrivo{
int nm;
struct _arrivo *next;
}arrivo;

//prototipi di funzione
arrivo *ordTappaM(arrivo **);
arrivo *insCoda(arrivo **, int);


//main
int main(int argc, char *argv[]){
arrivo *a, *tmp;
int n, tmpnm, i;

//inizializzo la testa a NULL
a = NULL;

//chiede il numero dei partecipanti
printf("\nQuanti partecipanti? ");
scanf("%d", &n);

//alloco memoria alla lista chiamando la funzione
for(i = 0; i < n; i++){
printf("\nInserisci numero maglia: ");
scanf("%d", &tmpnm);
insCoda(&a, tmpnm);
}
//controllo che nella lista ci sia almeno un elemento
if(n > 1)
ordTappaM(&a);
for(tmp = a; tmp != NULL; tmp = tmp->next)
printf("\n%d", tmp->nm);

scanf("%d", n);

return 0;
}


//ordinamento lista
arrivo *ordTappaM(arrivo **t){
arrivo *tmp, *tmp2;

tmp = *t;

/*controlla se è necessario cambiare la testa della lista oppure no
(se il nm della testa è maggiore di uno degli altri nella lista*/
if(tmp->nm > tmp->next->nm){
tmp2 = tmp;
tmp = tmp->next;
tmp->next = tmp2;
}

/*confronta ogni elemento della lista con tutti gli altri e se ne trova
due non in ordine inverte i piuntatori all'elemento successivo*/
for(tmp = (*t)->next; tmp->next != NULL; tmp = tmp->next){
for(tmp2 = tmp->next; tmp2 != NULL; tmp2 = tmp2->next){
if(tmp2->nm > tmp2->next->nm){
tmp = tmp2->next;
tmp2->next = tmp2;
tmp2 = tmp2->next->next;
}
}
}
return *t;
}

//Inserisce i valori in coda alla lista
arrivo *insCoda(arrivo **t, int tmpnm){
arrivo *nuovo, *tmp;
int i;

//controllo che la malloc sia andata a buon fine
if(nuovo = (arrivo *)malloc(sizeof(arrivo))){
//memorizzo il valore nella lista
nuovo->nm = tmpnm;
nuovo->next = NULL;
if(!(*t))
return nuovo;
for(tmp = *t; tmp->next != NULL; tmp = tmp->next);
tmp->next = nuovo;
}

//se la malloc fallisce
else
printf("\nError1!Malloc fallita.");
return *t;
}

[tia86]

Semplice, stai passando un null pointer (a=NULL).

[MrX87]

bhè non so cosa voglia dire che stai passando un NULL pointer, perchè inizialmente quando si dichiara una lista, per forza il puntatore alla testa è NULL, il problema piuttosto è nella lettura, che magari sarebbe più corretta così:

//Inserisce i valori in coda alla lista
arrivo *insCoda(arrivo *t, int tmpnm)
{
    arrivo *nuovo, *tmp;
    int i;

    //controllo che la malloc sia andata a buon fine
    if(nuovo = (arrivo *)malloc(sizeof(arrivo))){
    
    //memorizzo il valore nella lista
    nuovo->nm = tmpnm;
    if( !t ) {
        nuovo->next = NULL;
        t = nuovo;
    }
    else {
         nuovo->next = t;
         t = nuovo;
    }
    return nuovo;
}

in questo modo controlli il caso di quando inserisci in testa per la prima volta, e quando inserisci dopo il primo elemento: inoltre la funzione se la fai di tipo arrivo* è inutile passare il puntatore alla testa "a" per riferimento, ma basta ritornarlo con il return; allo stesso modo nella funzione di ordinamento, che non ho ancora visto...ma sembra che c'è qualche errore!
se ci sono altri problemi, magari li vediamo domani che sn anche le 2,42...ahah..ciao ragazzi

[NaTEn]

Ciao..
Ho letto ora la tua risposta, ma ho un dubbio. Se io passo la testa della lista e non il puntatore alla testa della lista, quindo ritorno il valore, nel caso io cambiassi la testa, questo cambiamento non sarà visibile al mio main poiché la testa della lista la passo per valore...
Probabilmente sbaglio io, ma vorrei capire :master:
Comunque in che senso cosa vuol dire passare un NULL pointer??
Al primo inserimento sarà NULL perché la lista è vuota, ma ai successivi no...(questa è una parte di un programma, non è tutto, l'ho adattato io per renderlo funzionante a se, ma perché appunto non capivo questo pezzo perché fosse sbagliato..)

[Samuele_70]

Il problema è quello già evidenziato

// Inserisce i valori in coda alla lista
arrivo *insCoda( arrivo **t, int tmpnm)
{
	arrivo *nuovo, *tmp;

	// controllo che la malloc sia andata a buon fine
	nuovo = (arrivo *)malloc(sizeof(arrivo));
	if(nuovo)
	{
		// memorizzo il valore nella lista
		nuovo->nm   = tmpnm;
		nuovo->next = NULL;
		if(NULL==*t)
		{
			*t = nuovo; // <--- ! ;)
			return nuovo;
		}
		for( tmp = *t; tmp->next != NULL; tmp = tmp->next);
		tmp->next = nuovo;
	}

	// se la malloc fallisce
	else
		printf("\nError1!Malloc fallita.");
	return *t;
}

Se passi alla funzione il puntatore della variabile testa, di fatto restituirne poi anche il valore con la return è superfluo.

[tia86]

Originariamente inviato da NaTEn
Ciao..
Ho letto ora la tua risposta, ma ho un dubbio. Se io passo la testa della lista e non il puntatore alla testa della lista, quindo ritorno il valore, nel caso io cambiassi la testa, questo cambiamento non sarà visibile al mio main poiché la testa della lista la passo per valore...
Probabilmente sbaglio io, ma vorrei capire :master:
Comunque in che senso cosa vuol dire passare un NULL pointer??
Al primo inserimento sarà NULL perché la lista è vuota, ma ai successivi no...(questa è una parte di un programma, non è tutto, l'ho adattato io per renderlo funzionante a se, ma perché appunto non capivo questo pezzo perché fosse sbagliato..)

Semplicemente che nel tuo programma la veriabile a non viene mai modificata e quindi vai a usare un null pointer nelle tue funzioni, ordMappa non tiene conto di questo e quindi fai una access violation.

[MrX87]

bhè, allora dire che il problema è sia nella funzione di lettura, nel senso che :

Se passi alla funzione il puntatore della variabile testa, di fatto restituirne poi anche il valore con la return è superfluo.

quindi secondo me si può modificare o come avevo proposto prima io, ovvero passando la testa per valore, e poi effettuare in return della testa al main, oppure puoi dichiarare la funzione void e passi la testa per riferimento;
poi secondo problema, che nella funzione che ordina la lista c'è sicuro un accesso a un puntatore NULL che causa il segmentation fault, come dice proprio tia86:

Semplicemente che nel tuo programma la veriabile a non viene mai modificata e quindi vai a usare un null pointer nelle tue funzioni, ordMappa non tiene conto di questo e quindi fai una access violation.

[NaTEn]

Stamattina, con più lucidità, ho modificato il codice e ho trovato parecchi errori abbastanza banali (alcuni li avete detti anche voi).
Solo che ora ho fatto un po' un macello con l'ordinamento, nel senso che non fa più una violazione di accesso, ma perdo da qualche parte il puntatore all'elemento successivo della lista.
Ora provo a modificarlo e vedere se riesco a farlo andare, in caso posto il codice modificato.

Intanto vi ringrazio per le risposte..