[C] Crash su gets()

[danisrcnl]

Ciao a tutti, ho un piccolo problema con un programma che gestisce un database di studenti universitari implementando funzioni di ordinamento per nome o matricola, ricerca normale o dicotomica a seconda dei casi, cancellazione dal DB, etc nonchè l'aggiunta di uno studente al DB stesso, qui ho un crash fisso sulla prima gets() del codice che vi posto di seguito, è molto probabile che io abbia combinato qualche pasticcio con il passaggio by reference del vettore di struct, visto che sono un poco alle strette una mano nell'individuare in fretta il problema la gradirei molto :) se risolvo intanto vi faccio sapere grazie!

int aggiungi(studente **database, int n, int i)
{
studente db;
int raddoppio=0;

if(n==i)
{
*database=(struct Item *)realloc(database, sizeof(struct Item)*2*n);
raddoppio=1;
}

printf("Inserisci la MATRICOLA (SXXXXXX): ");
gets(db.matricola);
printf("\nInserisci il NOME: ");
gets(db.nome);
printf("\nInserisci il COGNOME: ");
gets(db.cognome);
printf("\nInserisci la DATA di NASCITA (gg/mm/aaaa) : ");
scanf("%d/%d/%d", &db.data.giorno, &db.data.mese, &db.data.anno);
printf("\nInserisci il SESSO (M o F) : ");
while(getchar() != '\n');
scanf("%c", &db.sesso);

*database[i]=db;

return(raddoppio);

}

[sottovento]

E' necessario sapere la definizione di studente; altrimenti si tratta di divinazione :D

[danisrcnl]

E' necessario sapere la definizione di studente; altrimenti si tratta di divinazione :D

Hai ragione :asd:

typedef struct Item
{
char *matricola;
char *nome;
char *cognome;
struct
{
int giorno;
int mese;
int anno;
}data;
char sesso;
}studente;

[sottovento]

Non hai allocato memoria alcuna per matricola, nome e cognome. Sono solo puntatori al nulla.

Potresti decidere di assegnare loro una dimensione fissa e cambiare la definizione, per esempio, con
char matricola[100];
cosi' avrai 100 caratteri a disposizione.
Altrimenti lo puoi fare anche dinamicamente.

Ci sono altri problemi nel tuo codice, ma e' meglio vederli dopo per evitare confusione

[danisrcnl]

Non hai allocato memoria alcuna per matricola, nome e cognome. Sono solo puntatori al nulla.

Potresti decidere di assegnare loro una dimensione fissa e cambiare la definizione, per esempio, con
char matricola[100];
cosi' avrai 100 caratteri a disposizione.
Altrimenti lo puoi fare anche dinamicamente.

Ci sono altri problemi nel tuo codice, ma e' meglio vederli dopo per evitare confusione

Ci sono altre duecento righe che non ti ho mostrato, le memorie per ogni stringa o qualsiasi cosa sia sono allocate tutte, altrimenti non funzionerebbero le altre dieci funzioni che ho scritto :asd: il problema deve essere nella scrittura sul database passato by reference, credo, visto che è una cosa che ho visto da poco e sicuramente ho fatto un po' di confusione.

[sottovento]

Ci sono altre duecento righe che non ti ho mostrato, le memorie per ogni stringa o qualsiasi cosa sia sono allocate tutte, altrimenti non funzionerebbero le altre dieci funzioni che ho scritto :asd: il problema deve essere nella scrittura sul database passato by reference, credo, visto che è una cosa che ho visto da poco e sicuramente ho fatto un po' di confusione.


Le 200 righe sono nella funzione che hai pubblicato? Se si, e' necessario vederle.
Se no, il problema e' ancora quello perche' dichiari db di tipo studente e lo usi subito...

[danisrcnl]

Le 200 righe sono nella funzione che hai pubblicato? Se si, e' necessario vederle.
Se no, il problema e' ancora quello perche' dichiari db di tipo studente e lo usi subito...

La cosa a cui mi fai pensare ora è che forse non ho aggiustato le allocazioni dopo aver fatto passare la funzione dalla ricezione by value del DB alla ricezione by reference, visto che stupidamente avevo passato tutto come nelle altre funzioni. Ora vedo se aggiustando questa cosa funzione :) l'importante è che quello che ho scritto sopra sia giusto!

[danisrcnl]

Le 200 righe sono nella funzione che hai pubblicato? Se si, e' necessario vederle.
Se no, il problema e' ancora quello perche' dichiari db di tipo studente e lo usi subito...

Ok ora l'assegnazione locale a db infatti funziona :p , solo credo che ci sia qualche errore nel'ultimo passaggio visto che ora ho un crash dopo tutte le gets ahah

[sottovento]

Ok ora l'assegnazione locale a db infatti funziona :p , solo credo che ci sia qualche errore nel'ultimo passaggio visto che ora ho un crash dopo tutte le gets ahah
Bene.
Se hai ancora un crash, prova a postare la nuova versione...

[danisrcnl]

Bene.
Se hai ancora un crash, prova a postare la nuova versione...

Il problema l'ho capito: non riallocavo anche nel vettore vero e proprio, ossia allocavo solo per la funzione locale, ho messo una bozza dell'allocazione in database, ma l'ho scritta male visto che qualche errore qui sopra, sicuramente ho gestito male i puntatori a database, ora te la mostro tutta la funzione:

int aggiungi(studente **database, int n, int i)
{
studente db;
int raddoppio=0;

if(n==i)
{
*database=(struct Item *)realloc(database, sizeof(struct Item)*2*n);
raddoppio=1;
}

db.matricola=(char*)malloc(sizeof(char)*N);
db.nome=(char*)malloc(sizeof(char)*MAX);
db.cognome=(char*)malloc(sizeof(char)*MAX);

printf("Inserisci la MATRICOLA (SXXXXXX): ");while((getchar() != '\n') && (getchar() != ' '));
gets(db.matricola);
printf("\nInserisci il NOME: ");while((getchar() != '\n') && (getchar() != ' '));
gets(db.nome);
printf("\nInserisci il COGNOME: ");while((getchar() != '\n') && (getchar() != ' '));
gets(db.cognome);
printf("\nInserisci la DATA di NASCITA (gg/mm/aaaa) : ");
scanf("%d/%d/%d", &db.data.giorno, &db.data.mese, &db.data.anno);
printf("\nInserisci il SESSO (M o F) : ");
while(getchar() != '\n');
scanf("%c", &db.sesso);

/*
*database[i].matricola=(char*)malloc(sizeof(char)*N);
*database[i].nome=(char*)malloc(sizeof(char)*MAX);
*database[i].cognome=(char*)malloc(sizeof(char)*MAX);*/

*database[i]=db;

free(db.matricola);free(db.nome);free(db.cognome);

return(raddoppio);

}

[sottovento]

*database[i]=db;
}

Purtroppo la copia non puo' essere fatta cosi': all'interno della tua struttura le stringhe sono puntatori ad altra locazione (che fra l'altro vai a deallocare).
Dovresti farti una copia a mano ed eventualmente decidere cosa fare delle stringhe? Vuoi copiare l'intero contenuto? Allora devi allocare spazio in db e copiare. Vuoi copiare solo l'indirizzo, visto che e' valido? Allora non devi deallocare alla fine.

[danisrcnl]

*database[i]=db;
}

Purtroppo la copia non puo' essere fatta cosi': all'interno della tua struttura le stringhe sono puntatori ad altra locazione (che fra l'altro vai a deallocare).
Dovresti farti una copia a mano ed eventualmente decidere cosa fare delle stringhe? Vuoi copiare l'intero contenuto? Allora devi allocare spazio in db e copiare. Vuoi copiare solo l'indirizzo, visto che e' valido? Allora non devi deallocare alla fine.

Non saprei, in realtà l'operazione non mi sembra del tutto erronea, nel senso che copio in una struct un'altra struct dello stesso tipo, come ho fatto, ad esempio nelle funzioni di ordinamento. Al di la di questo, ignorando questo discorso, se volessi allocare correttamente nuova memoria per database[i] dalla funzione e non dal main come dovrei fare? Visto che ho un errore in fase di build su quelle righe?

[sottovento]

Non saprei, in realtà l'operazione non mi sembra del tutto erronea, nel senso che copio in una struct un'altra struct dello stesso tipo, come ho fatto, ad esempio nelle funzioni di ordinamento. Al di la di questo, ignorando questo discorso, se volessi allocare correttamente nuova memoria per database[i] dalla funzione e non dal main come dovrei fare? Visto che ho un errore in fase di build su quelle righe?

No, in realta' quando copi la struttura lo fai copiando campo per campo, non allocando nuova memoria per le stringhe e copiandone il contenuto: copi semplicemente l'indirizzo, quindi la struttura di partenza e di arrivo puntano alla stessa copia delle stringhe allocate. Quando vai a deallocarle fai in modo che entrambe le strutture puntino ad un valore invalido e vai in crash.

Che errore hai in fase di build?

[danisrcnl]

No, in realta' quando copi la struttura lo fai copiando campo per campo, non allocando nuova memoria per le stringhe e copiandone il contenuto: copi semplicemente l'indirizzo, quindi la struttura di partenza e di arrivo puntano alla stessa copia delle stringhe allocate. Quando vai a deallocarle fai in modo che entrambe le strutture puntino ad un valore invalido e vai in crash.

Che errore hai in fase di build?

Oggi dopo lezione provo a modificarla in modo da scrivere direttamente sul mio database[i] senza struct d'appoggio.

[danisrcnl]

No, in realta' quando copi la struttura lo fai copiando campo per campo, non allocando nuova memoria per le stringhe e copiandone il contenuto: copi semplicemente l'indirizzo, quindi la struttura di partenza e di arrivo puntano alla stessa copia delle stringhe allocate. Quando vai a deallocarle fai in modo che entrambe le strutture puntino ad un valore invalido e vai in crash.

Che errore hai in fase di build?

Ecco qui la funzione un po' più raffinata, nessun errore in fase di build ed esecuzione, almeno apparentemente, sintatticamente corretta, ma nello stampare il database dopo l'aggiunta, nell'elenco ho, al suo indice, un (null) (null) (null) 0/0/0, praticamente come se non avesse memorizzato nulla.

void aggiungi(studente** p_database, int* p_allocati, int* p_usati)
{
if(*p_allocati==*p_usati)
{
*p_database=(struct Item*)realloc(*p_database, sizeof(struct Item)*2*(*p_allocati));
*p_allocati=2*(*p_allocati);
}

p_database[*p_usati]->nome=(char*)malloc(sizeof(char)*MAX);
p_database[*p_usati]->cognome=(char*)malloc(sizeof(char)*MAX);
p_database[*p_usati]->matricola=(char*)malloc(sizeof(char)*MAX);

getchar();
printf("\nInserisci il NOME: ");
gets(p_database[*p_usati]->nome);

printf("\nInserisci il COGNOME: ");
gets(p_database[*p_usati]->cognome);

printf("\nInserisci la MATRICOLA: ");
gets(p_database[*p_usati]->matricola);

printf("\nInserisci la data di nascita (gg/mm/aaaa): ");
scanf("%d/%d/%d", &(p_database[*p_usati]->data.giorno), &(p_database[*p_usati]->data.mese), &(p_database[*p_usati]->data.anno)); getchar();

printf("\nInserisci il SESSO: ");
scanf("%c", &(p_database[*p_usati]->sesso));

(*p_usati)++;
}

[danisrcnl]

Anche se in realtà il vettore database, anche se di struct, dovrebbe essere passato by reference alla funzione, proprio in quanto vettore, quindi dovrei aver fatto un casino inutile :doh:

[fano]

In ogni caso anche se riuscirai ad allocare la memoria per le tue stringhe ti sconsiglio vivamente di usare gets() per leggere da STDIN! Tu alla fine gli darai una dimensione fissa (poniamo 100 quindi allocare memoria non servirebbe a nulla falli array di 100 e via!) e cosa accade se l'utente mette 150 caratteri? Che nella tua stringa finiscono 150 caratteri senza alcun tappo ovvero una stringa sbordante e non valida :eek:

Quindi meglio usare fgets() a cui passi la dimensione massima dell'array e se l'utente passa più caratteri la stringa viene, semplicemente, troncata!