mehrere mallocs sinnvoll überprüfen



  • Guten Abend.

    Ich habe eine struct student, die 4 Zeiger auf char als member hält:

    typedef struct Student_t
    {
    	char *reg_number;		
    	char *forename;
    	char *surename;
    	char *e_mail;
    	Date birthday;
    } Student;
    

    Ich möchte jetzt für alle 4 chars* speicher mittels malloc holen, und würde das so machen:

    char *reg_number = malloc( strlen( temp_reg_number ) + 1 );
    if( reg_number == NULL )
    {
    //tue was
    }
    char *forename = malloc( strlen( temp_forename ) + 1 );
    if( forename == NULL )
    {
    // gleiches Problem, nur muss ich jetzt aufpassen dass der Speicher von reg_number wieder frei wird
    free( reg_number );
    }
    char *surename = malloc( strlen( temp_surename ) + 1 );
    if( surename == NULL )
    {
    // tue was
    free( reg_number );
    free( forename );
    }
    ...
    

    Meine erste Frage: Wenn malloc fehlschlägt, macht es Sinn das Programm weiterlaufen zu lassen? Ist es vielleicht doch sinnvoll, das Programm zu beenden?

    Meine zweite Frage: Wie kann ich obigen Code eventuell vereinfachen?

    Anmerkung, die informativ sein könnte:
    Es sind schon einige mallocs, die ich machen muss. Laut Aufgabenstellung soll ich eine Funktion schreiben, die 100.000 Studenten anlegt. Das sind dann 100.000 Nodes + 100.000 Studenten + 100.000 * 4 membervariablen. also 600.000 mallocs.

    Vielen Dank schonmal



  • hallihallo schrieb:

    Meine erste Frage: Wenn malloc fehlschlägt, macht es Sinn das Programm weiterlaufen zu lassen? Ist es vielleicht doch sinnvoll, das Programm zu beenden?

    Wenn der Speicher ausgeht, wird das Programm kaum fortlaufen können, schließlich wurde dieser für den weiteren reibungslosen Ablauf benötigt, daher solltest du es beenden, allerdings kannst du vorher noch eine Beschreibung des Fehlers in ein Log schreiben, um das Problem nachzuvollziehbar zu machen, und Daten sichern.

    hallihallo schrieb:

    Meine zweite Frage: Wie kann ich obigen Code eventuell vereinfachen?

    Indem du keinen dynmamischen Speicher verwendest! Registrierungsnummern, Namen, E-Mail - sie alle werden nicht unbestimmt lang sein, also verwende lieber Arrays mit konstanter Größe, welche mehr oder weniger Speicher nicht verwenden, und prüfe die Länge der Eingaben (ein lächerlich langer Name z.B. kann nur ein Scherz sein und nach einer Warnung ignoriert werden). Auch der Student als Knoten-Wert sollte nicht dynamisch reserviert werden.
    Die Zeiger eines Studenten kannst du mit NULL initialisieren und bei einem Fehler eine Funktion zur Freigabe nutzen, die nur jene Zeiger befreit, welche nicht NULL sind.



  • Nur noch 1x calloc für alles wäre:

    enum {MAX_STE=80};
    typedef struct Student_t
    {
        char reg_number[MAX_STE];      
        char forename[MAX_STE];  
        char surename[MAX_STE];  
        char e_mail[MAX_STE];  
        Date birthday; /* ?! */
    } Student;
    ...
    Student *student=calloc(100000,sizeof*student); /* das dürfte auch schneller und speicherschonender sein als 100000x malloc */
    


  • Der OT hat extra jetzt Zeiger angelegt, da ihm von DirkB im Thread http://www.c-plusplus.net/forum/p2376644#2376644 dazu geraten wurde 😉



  • hallihallo schrieb:

    Meine erste Frage: Wenn malloc fehlschlägt, macht es Sinn das Programm weiterlaufen zu lassen? Ist es vielleicht doch sinnvoll, das Programm zu beenden?

    Auf jeden Fall mit sinnvoller Fehlerrückgabe weiterlaufen. Du würdest auch nicht wollen, dass irgendeine Bibliothek einfach dein Programm beendet, nur weil die intern Panik bekommt.

    #include <string.h>
    #include <stdlib.h>
    #include <assert.h>
    #include <stdio.h>
    
    /* Eine Menge von möglichen Ergebnissen von Funktionen, die theoretisch
     * fehlschlagen können. Im Moment reichen uns zwei Werte. */
    typedef enum Error
    {
    	/* Die Funktion konnte ihre Aufgabe vollständig erfüllen. */
    	Success = 0,
    
    	/* Die Funktion konnte ihre Aufgabe nicht erfüllen, weil notwendige
    	 * Ressourcen nicht verfügbar waren. */
    	NoResources = 1
    }
    Error;
    
    typedef struct Student /* kein _t Postfix! */
    {
        char *reg_number;      
        char *forename;
        char *surename;
        char *email;
    }
    Student;
    
    /* Das ist ein Konstruktor. Er stellt sicher, dass das Student-Objekt
     * vollständig bestimmt ist. Er ist jedoch nicht dafür zuständig den
     * Speicher anzufordern. Vorsorglich ist der Rückgabetyp ein Fehlercode
     * für spätere Erweiterungen. */
    Error Student_create(Student *s, char *reg_number,
                         char *forename, char *surename,
                         char *email)
    {
    	assert(reg_number);
    	assert(forename);
    	assert(surename);
    	assert(email);
    	s->reg_number = reg_number;
    	s->forename = forename;
    	s->surename = surename;
    	s->email = email;
    	return Success;
    }
    
    /* Das ist ein Destruktor. Dieser darf auf einem erfolgreich
     * konstruierten Student-Objekt aufgerufen werden. Danach befindet
     * sich das Objekt in einem ungültigen Zustand, kann aber durch Aufruf
     * des Konstruktors auch wieder gültig gemacht werden. Der Destruktor
     * ist dafür zuständig die Ressourcen freizugeben. */
    void Student_destroy(Student *s)
    {
    	free(s->reg_number);
    	free(s->forename);
    	free(s->surename);
    	free(s->email);
    }
    
    /* Gibt ein neues Stück Speicher zurück, das den übergebenen String
     * enthält. Das Ergebnis muss mit free wieder freigegeben werden. */
    static char *clone_string(char const *str)
    {
    	size_t const length = strlen(str);
    	char *cloned = malloc(length + 1);
    	if (cloned)
    	{
    		memcpy(cloned, str, length + 1);
    	}
    	return cloned;
    }
    
    /* Es gibt verschiedene Möglichkeiten die Fehlerbehandlung umzusetzen.
     * Ich stelle ein paar der akzeptablen Möglichkeiten vor: */
    
    static Error create_example_student_A(Student *s)
    {
    	/* Diese Variante ist sehr kurz, weil zwei Eigenschaften der
    	 * Speicherverwaltung ausgenutzt werden:
    	 * - Es ist selten, dass malloc fehlschlägt. Wir können für den
    	 *   Fall optimieren, dass es erfolgreich ist.
    	 * - free(NULL) ist kein Problem, sondern tut einfach nichts.
    	 */
    	Error result;
    	char *reg_number = clone_string("123");
    	char *forename = clone_string("forename");
    	char *surename = clone_string("surename");
    	char *email = clone_string("email@email.com");
    	if (reg_number && forename && surename && email)
    	{
    		/* Wir reichen den Rückgabewert des Konstruktors weiter. */
    		result = Student_create(s, reg_number, forename, surename, email);
    	}
    	else
    	{
    		result = NoResources;
    	}
    	if (result != Success)
    	{
    		/* Im Fehlerfall wird ausgenutzt, dass free(NULL) nichts tut.
    		 * Da der Fehlerfall selten ist, muss er nicht schnell sein.
    		 * Also sind die unnötigen Aufrufe an free nicht von Bedeutung.
    		 * Ein Vorteil dieser Schreibweise ist die gute Lesbarkeit durch
    		 * die Kürze. */
    		free(reg_number);
    		free(forename);
    		free(surename);
    		free(email);
    	}
    	/* Einige betrachten Funktionen als lesbarer, die nur ein return
    	 * haben. Dieser Umstand ist jedoch eher ein zufälliges Nebenprodukt
    	 * dieser Variante. */
    	return result;
    }
    
    static Error create_example_student_B(Student *s)
    {
    	/* Kurz ist auch die Variante mit Verschachtelung. Diese ist
    	 * vielleicht am einfachsten nachvollziehbar. Die verschiedenen
    	 * Abschnitte der Ressourcenbeschaffung sind sofort sichtbar.
    	 * Diese Variante kehrt im Fehlerfall so schnell wie möglich zurück.
    	 * Sie nutzt auch nicht aus, dass free(NULL) nichts tut, weil auch
    	 * der Aufruf alleine Zeit kosten würde. Diese Variante funktioniert
    	 * auch gut mit anderen Ressourcen als Speicher, zum Beispiel mit
    	 * eigenen Konstruktoren und Destruktoren. */
    	Error result = NoResources;
    	char *reg_number = clone_string("123");
    	if (reg_number)
    	{
    		char *forename = clone_string("forename");
    		if (forename)
    		{
    			char *surename = clone_string("surename");
    			if (surename)
    			{
    				char *email = clone_string("email@email.com");
    				if (email)
    				{
    					result = Student_create(s, reg_number, forename, surename, email);
    					if (result == Success)
    					{
    						/* Im Erfolgsfall kann sofort zurückgekehrt werden. Es gibt hier
    						 * also genau zwei mal return. Diese Trennung kann als Vor- oder
    						 * Nachteil wahrgenommen werden. */
    						return Success;
    					}
    					free(email);
    				}
    				free(surename);
    			}
    			free(forename);
    		}
    		free(reg_number);
    	}
    	return result;
    }
    
    static Error create_example_student_C(Student *s)
    {
    	/* Dies entspricht der Variante B, wenn man die Verschachtelung
    	 * herausnimmt und mit goto ersetzt. Das kann dann sinnvoll sein,
    	 * wenn die Verschachtelung sehr tief werden würde. Alternativ
    	 * könnte man die Funktion auch aufteilen, um die Verschachtelung
    	 * jeweils gering zu halten. */
    	Error result = NoResources;
    	char *reg_number;
    	char *forename;
    	char *surename;
    	char *email;
    
    	reg_number = clone_string("123");
    	if (!reg_number)
    	{
    		goto failure_4;
    	}
    
    	forename = clone_string("forename");
    	if (!forename)
    	{
    		goto failure_3;
    	}
    
    	surename = clone_string("surename");
    	if (!surename)
    	{
    		goto failure_2;
    	}
    
    	email = clone_string("email@email.com");
    	if (!email)
    	{
    		goto failure_1;
    	}
    
    	result = Student_create(s, reg_number, forename, surename, email);
    	if (result == Success)
    	{
    		return Success;
    	}
    
    	/* Die Labels sind so mit den Freigabefunktionen verzahnt, dass kein
    	 * unnötiges free aufgerufen wird. */
    	free(email);
    failure_1:
    	free(surename);
    failure_2:
    	free(forename);
    failure_3:
    	free(reg_number);
    failure_4:
    	return result;
    }
    
    static void print_student(FILE *out, Student const *s)
    {
    	fprintf(out, "reg_number: %s\n", s->reg_number);
    	fprintf(out, "forename: %s\n", s->forename);
    	fprintf(out, "surename: %s\n", s->surename);
    	fprintf(out, "email: %s\n", s->email);
    }
    
    int main(void)
    {
    	int rc = 1;
    	Student a;
    	if (create_example_student_A(&a) == Success)
    	{
    		Student b;
    		if (create_example_student_B(&b) == Success)
    		{
    			Student c;
    			if (create_example_student_C(&c) == Success)
    			{
    				/* Mal sehen, ob die Daten auch stimmen. */
    				print_student(stdout, &a);
    				print_student(stdout, &b);
    				print_student(stdout, &c);
    				rc = 0;
    				Student_destroy(&c);
    			}
    			Student_destroy(&b);
    		}
    		Student_destroy(&a);
    	}
    	return rc;
    }
    
    $ valgrind --leak-check=yes ./a.out
    ==6417== Memcheck, a memory error detector
    ==6417== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
    ==6417== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
    ==6417== Command: ./a.out
    ==6417== 
    reg_number: 123
    forename: forename
    surename: surename
    email: email@email.com
    reg_number: 123
    forename: forename
    surename: surename
    email: email@email.com
    reg_number: 123
    forename: forename
    surename: surename
    email: email@email.com
    ==6417== 
    ==6417== HEAP SUMMARY:
    ==6417==     in use at exit: 0 bytes in 0 blocks
    ==6417==   total heap usage: 12 allocs, 12 frees, 114 bytes allocated
    ==6417== 
    ==6417== All heap blocks were freed -- no leaks are possible
    ==6417== 
    ==6417== For counts of detected and suppressed errors, rerun with: -v
    ==6417== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
    

    Scheint zu funktionieren.

    So schreibt man korrekt Programme ohne Leaks in C. Wenn dir das zu umständlich ist und du eine Wahl hast, nimm C++.

    PS: Wenn ich so etwas wie "schnell" schreibe, sind das Unterschiede im Bereich von Nanosekunden, die nur theoretisch interessant sind. In der Praxis spielt so etwas keine Rolle, erst recht nicht für Anfänger. Lesbarer Code ist wichtiger als Mikrooptimierungen. Ganz abgesehen davon, dass man malloc nicht so benutzen würde, wenn es schnell sein müsste.


Anmelden zum Antworten