[C] Programma crasha se aggiungo più di un elemento al DB

[stichtom]

Ciao a tutti, sto facendo un esercizio per la gestione di un registro scolastico. L'ho finito e all'inizio sembrava funzionare correttamente. Poi mi sono accorto che se aggiungo più di uno studente al database, il programma crasha. In particolare ho notato che il problema è collegato in qualche modo al salvataggio del nome (forse qualcosa con la realloc), infatti se lavoro solo con i numeri delle matricole non c'è nessun problema.


Qualcuno potrebbe darmi una mano? Il codice completo (in particolare le funzioni add e print sono quelle interessate):

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


#ifdef _WIN32
#define CLEAR "cls"
#else // Negli altri OS
#define CLEAR "clear"
#endif


#define CLEAR_BUFFER do { c = getchar(); } while (c != '\n' && c != EOF); // Macro per pulire il buffer in input
#define MAX 100 // Definisco il numero massimo di caratteri per il nome, cognome e indirizzo
#define TRUE 1
#define FALSE 0


typedef struct {
    int day;
    int month;
    int year;
} DATE_T;


typedef struct {
    int code;
    char *name;
    char *surname;
    char *address;
    DATE_T date;
    int *exams;
} STUDENT_T;


STUDENT_T *student;
char courses[][15] = {"Programmazione", "Inglese", "Analisi"}; // SISTEMARE POI
int myIndex = 0; // Contatore che conta il numero degli studenti


int checkValue(int value, int check, int min, int max);
void checkAllocation(STUDENT_T *student);
void menu();
void add(STUDENT_T *student);
void printDatabase(STUDENT_T *student);
char* readString();
void exitmenu();




int main()
{


    student = (STUDENT_T*) malloc(sizeof(STUDENT_T));
    checkAllocation(student);


    menu();


    free(student);


    return 0;
}


void menu()
{
    int menu, check;


    system(CLEAR);


    printf("\t\t ---- MENU GESTIONE SEGRETERIA ---- \n\n");
    printf("1 - Inserisci un nuovo studente\n");
    printf("2 - Stampa il registro\n");
    printf("3 - Elimina uno studente\n");
    printf("4 - Ricerca uno studente\n");
    printf("5 - Ricerca studenti per esami da sostenere\n");
    printf("6 - Esci dal programma\n\n");
    printf("Scegli un opzione dal menu: ");
    check = scanf("%d", &menu);
    menu = checkValue(menu, check, 1, 6);


    switch (menu) {
    case 1 :
        add(student);
        break;
    case 2 :
        printDatabase(student);
        break;
    case 3 :
        //cancel(student);
        break;
    case 4 :
        //search(student);
        break;
    case 5 :
        //searchSubject(student);
        break;
    case 6 :
        exit(EXIT_SUCCESS);
    default :
        printf("Errore, scelta non valida");
        exit(EXIT_FAILURE);
    }


    return;
}




int checkValue(int value, int check, int min, int max)
{
    int c;


    CLEAR_BUFFER
    while (value < min || value > max || check != 1) {
        printf("Errore, devi inserire un valore valido compreso tra %d e %d: ", min, max);
        check = scanf("%d", &value);
        CLEAR_BUFFER
    }


    return value;
}


void checkAllocation(STUDENT_T *student)
{
    if (student == NULL) {
        printf("Errore durante l'allocazione dinamica della memoria");
        exit(EXIT_FAILURE);
    }


    return;
}


void exitmenu()
{
    int option, check;


    printf("\n\nPremi 0 per tornare al main o 1 per uscire: ");
    check = scanf("%d", &option);
    option = checkValue(option, check, 0, 1);


    if (option == 0)
        menu();
    else if (option == 1)
        exit(EXIT_SUCCESS);
}


void add(STUDENT_T *student)
{


    char c;
    int check, j;
    int i = myIndex;
    /*enum subjects {Programmazione, Inglese, Analisi, Algoritmi, Architettura_Elaboratori, Algebra};
    enum subjects courses;*/
    system(CLEAR);


    student = (STUDENT_T*) realloc(student, (myIndex + 1) * sizeof(STUDENT_T));
    checkAllocation(student);


    printf("I: %d, INDEX = %d", i, myIndex);
    printf("\t\t ---- MENU GESTIONE SEGRETERIA ---- \n\n");
    printf("Inserisci il numero di matricola: ");
    check = scanf("%d", &student[i].code);
    student[i].code = checkValue(student[i].code, check, 0, INT_MAX);
    for (j = 0; j < myIndex; j++)
        if (student[i].code == student[j].code) {
            printf("Il numero di matricola inserito e\' gia presente nel database");
            exitmenu();
        }


    printf("Inserisci il nome: ");
    for (j = 0; j < MAX && (c = getchar()) != '\n'; j++) {
        student[i].name = (char*)realloc(student[i].name , (j+1) * sizeof(char));
        if (student[i].name  == NULL) {
            printf("Errore durante l'allocazione dinamica della memoria");
            exit(EXIT_FAILURE);
        }
        student[i].name[j] = c;
    }
    student[i].name[j] = '\0';


    // altri input 


    myIndex++;


    exitmenu();


    return;


}


void printDatabase(STUDENT_T *student)
{
    int i, j;


    system(CLEAR);


    printf("\t\t ---- VISUALIZZA CONTENUTO DATABASE ---- \n\n");
    printf("INDEX: %d\n\n", myIndex);
    if (myIndex == 0)
        printf("Non ci sono elementi da visualizzare nel database");
    else {
        for (i = 0; i < myIndex; i++) {
            printf("INDEX: %d, i: %d\n\n", myIndex, i);
            printf("Numero matricola: %d\n", student[i].code);
            printf("\tNome e Cognome: %s\n", student[i].name/*, student[i].surname*/);
           // Altre stampe


        }


    }


    exitmenu();


    return;


}

[Samuele_70]

Inizializza

student[i].name = NULL;

prima della realloc, e dovresti risolvere.

[stichtom]

Ciao, uno dei problemi era quello, ma il più importante era quello per cui passavo il puntatore student come argomento delle varie funzioni, ma poi dopo averci lavorato localmente, mi dimenticavo di salvarlo da qualche parte!!

Grazie mille

[M.A.W. 1968]

Mi corre l'obbligo di sottolineare che il codice (peraltro presentato su vari forum in contemporanea) contiene molti altri gravissimi errori di progettazione, di stile e di implementazione. Sfortunatamente non c'è tempo di analizzarli nel dettaglio, quindi procediamo con alcuni esempi in ordine sparso.

#define CLEAR_BUFFER do { c = getchar(); } while (c != '\n' && c != EOF);

è un tipico errore che, secondo il livello dell'esame, può comportare la bocciatura o nel migliore dei casi un drastico abbassamento della valutazione. In contesti di programmazione normali, non si deve assolutamente e in alcun caso "nascondere" il terminatore di statement del C entro una macro, né tantomeno includervi variabili che saranno poi, forse, chissà come e chissà dove dichiarate. Non siamo all'IOCCC, e il purging del buffer di tastiera può e deve avvenire entro una normalissima funzione C.

Il modo in cui viene usata la realloc() è poi inaccettabile. Premesso che l'array di strutture è la struttura dati meno indicata per la gestione di una simile implementazione (ma questa scelta può dipendere da vincoli e imposizioni didattiche, quindi non approfondiamo), l'acquisizione delle stringhe da tastiera un carattere per volta con chiamate reiterate alla realloc() è davvero oltre i confini della fantascienza.
In casi del genere, l'idioma più solido e fondamentale prevede di:

1) Allocare staticamente nello heap un buffer di adeguate dimensioni (vulgo, prima del main());

2) Accettare l'input utente in tale buffer, usando esclusivamente una fgets() su stdin (la scanf è deprecatissima per l'input di stringhe);

3) Allocare dinamicamente, se proprio se ne ha voglia, un buffer di dimensione pari alla lunghezza della stringa appena immessa (i.e. strlen()). Tale puntatore, una volta verificatane la corretta allocazione (i.e. non NULL) sarà associato al puntatore di campo nella struttura corrente, corrispondente al nuovo record.

4) Copiare il contenuto del buffer di lavoro nel buffer di campo appena allocato.


Vi sono poi molti altri aspetti secondari (es. le chiamate alla printf() sono molto costose computazionalmente, e conviene sempre aggregarle o evitarne l'uso in favore di puts ovunque possibile), ma per il momento potremmo fermarci qui.

[stichtom]

Mi corre l'obbligo di sottolineare che il codice (peraltro presentato su vari forum in contemporanea) contiene molti altri gravissimi errori di progettazione, di stile e di implementazione. Sfortunatamente non c'è tempo di analizzarli nel dettaglio, quindi procediamo con alcuni esempi in ordine sparso.

#define CLEAR_BUFFER do { c = getchar(); } while (c != '\n' && c != EOF);

è un tipico errore che, secondo il livello dell'esame, può comportare la bocciatura o nel migliore dei casi un drastico abbassamento della valutazione. In contesti di programmazione normali, non si deve assolutamente e in alcun caso "nascondere" il terminatore di statement del C entro una macro, né tantomeno includervi variabili che saranno poi, forse, chissà come e chissà dove dichiarate. Non siamo all'IOCCC, e il purging del buffer di tastiera può e deve avvenire entro una normalissima funzione C.

Il modo in cui viene usata la realloc() è poi inaccettabile. Premesso che l'array di strutture è la struttura dati meno indicata per la gestione di una simile implementazione (ma questa scelta può dipendere da vincoli e imposizioni didattiche, quindi non approfondiamo), l'acquisizione delle stringhe da tastiera un carattere per volta con chiamate reiterate alla realloc() è davvero oltre i confini della fantascienza.
In casi del genere, l'idioma più solido e fondamentale prevede di:

1) Allocare staticamente nello heap un buffer di adeguate dimensioni (vulgo, prima del main());

2) Accettare l'input utente in tale buffer, usando esclusivamente una fgets() su stdin (la scanf è deprecatissima per l'input di stringhe);

3) Allocare dinamicamente, se proprio se ne ha voglia, un buffer di dimensione pari alla lunghezza della stringa appena immessa (i.e. strlen()). Tale puntatore, una volta verificatane la corretta allocazione (i.e. non NULL) sarà associato al puntatore di campo nella struttura corrente, corrispondente al nuovo record.

4) Copiare il contenuto del buffer di lavoro nel buffer di campo appena allocato.


Vi sono poi molti altri aspetti secondari (es. le chiamate alla printf() sono molto costose computazionalmente, e conviene sempre aggregarle o evitarne l'uso in favore di puts ovunque possibile), ma per il momento potremmo fermarci qui.

Ciao e prima di tutto grazie per tutte le dritte, ne farò tesoro.
Per quanto riguarda la pulizia del buffer in input non capisco come posso farlo con una funzione. Certo, potrei usare fflush(stdin) ma è ancora più sbagliato dato che il comportamento non è definito e non è assolutamente portabile. Poi non capisco, perchè dici di non usare il terminatore di statement in una macro?

Grazie mille ancora per gli altri consigli

[M.A.W. 1968]

Per quanto riguarda la pulizia del buffer in input non capisco come posso farlo con una funzione. Certo, potrei usare fflush(stdin) ma è ancora pià sbagliato dato che il comportamento non è definito e non è assolutamente portabile. Poi non capisco, perchè dici di non usare il terminatore di statement in una macro?

Ecco un'ottima funzione, altamente portatile, per il purging del keyboard buffer. Ingegneristicamente molto migliore di una macro, anche se non è possibile renderla inline con la maggioranza dei compilatori C:

void clearKeyboardBuffer() 
{
    char ch;
    do 
    {
        ch = getchar();
    } while (('\n' != ch) && (EOF != ch));
    /* 
    ** Alternativa sintetica: 
    ** while ((ch = getchar() != '\n') && (ch != EOF)); 
    */
}

L'uso delle macro è soggetto a vincoli fortemente restrittivi in tutte le norme di stile, in quanto prono ad abusi potenzialmente senza limiti e quindi foriero di errori di difficilissima tracciabilità. Inoltre, tali abusi rendono difficile l'uso di tools di analisi statica e dinamica del codice (MISRA checker, Lint e derivati, interpretazione astratta), che sono obbligatori in molti mercati. La letteratura concorda in modo univoco sulla restrizione dell'uso degli idiomi di preprocessore secondo regole di stile come quelle qui richiamate, in particolare è proibito inglobare elementi sintattici significativi (vedi ';' finale), la cui assenza nel sorgente finale possa poi confondere quei parser automatici che non richiamano preliminarmente il preprocessore.