Empfohlene Methode zur Behandlung mehrerer malloc-Fehler in einer einzigen Funktion in C

Question

Wie wird empfohlen, mehrere Malloc-Fehler zu behandeln, die wie im folgenden Code nacheinander auftreten können?

bool myFunc(int x, int y) 
{ 
    int *pBufX = null; 
    int *pBufY = null; 

    if((x <= 0) || (y <= 0)) 
    { 
     return false; 
    } 

    pBufX = (int*)malloc(sizeof(int) * x); 
    if(pBufX == null) 
    { 
     return false; 
    } 

    pBufY = (int*)malloc(sizeof(int) * y); 
    if(pBufY == null) 
    { 
     free(pBufX) //free the previously allocated pBufX 
     return false; 
    } 

    //do something useful 

    free(pBufX); 
    free(pBufY); 

    return true; 
} 

Das Problem bei diesem Ansatz ist, dass, wenn die Anzahl der mallocs hoch sind, könnten Sie etwas zu befreien vergessen und Speicherlecks verursachen. Wenn es eine Art von Protokoll gibt, das bei Auftreten eines Fehlers ausgegeben werden muss, wird der Code sehr lang.

Ich habe Code gesehen, der diese mit einem Goto behandelt, wo Sie alle Mallocs an einem Ort löschen, nur einmal. Der Code ist nicht lang, aber ich mag es nicht, gotos zu benutzen.

Gibt es einen besseren Weg als beide dieser Ansätze?

Vielleicht ist das Problem mit dem Design in erster Linie. Gibt es eine Faustregel beim Entwurf von Funktionen, wenn es darum geht, mehrere mallocs zu minimieren?

Edit: Es gibt einen anderen Weg, den ich gesehen und benutzt habe. Anstatt goto zu verwenden, behalten Sie den Status des Programms und fahren nur fort, wenn der Status OK ist. Ähnlich wie goto, aber nicht mit goto. Aber das erhöht die Anzahl der if-Anweisungen, die den Code langsamer laufen lassen könnten.

bool myFunc(int x, int y) 
{ 
    int *pBufX = null; 
    int *pBufY = null; 
    bool bRet = true; 

    if((x <= 0) || (y <= 0)) 
    { 
     return false; 
    } 

    pBufX = (int*)malloc(sizeof(int) * x); 
    if(pBufX == null) 
    { 
     bRet = false; 
    } 

    if(bRet == true) 
    { 
     pBufY = (int*)malloc(sizeof(int) * y); 
     if(pBufY == null) 
     { 
      bRet = false; 
     } 
    } 

    //do something useful 

    if(pBufX != null) 
     free(pBufX); 

    if(pBufY != null)  
     free(pBufY); 

    return bRet; 
} 

Answer 1

Persönlich bevorzuge ich die Verwendung von goto. Aber da es zu free(NULL) sicher ist, können Sie in der Regel alles tun, um die Zuweisungen vorne:

int *a = NULL; 
int *b = NULL; 
a = malloc(sizeof *a * x); 
b = malloc(sizeof *b * y); 
if(a == NULL || b == NULL) { 
    free(a); 
    free(b); 
    return false; 
} 

Wenn Sie die zugewiesenen Speicher benötigen eine Berechnung zu tun, um eine Größe für eine weitere Zuteilung zu bekommen, Ihre Funktion ist wahrscheinlich ausreichend komplex dass es sowieso refaktorisiert werden sollte.

Answer 2

Dies ist eine mögliche Lösung:

bool myFunc(int x, int y) 
{ 
    int returnvalue = false; 

    int *pBufX = NULL; // << null -> NULL 
    int *pBufY = NULL; 

    if((x <= 0) || (y <= 0)) 
    { 
     goto fail; 
    } 

    pBufX = (int*)malloc(sizeof(int) * x); 
    if(pBufX == null) 
    { 
     goto fail; 
    } 

    pBufY = (int*)malloc(sizeof(int) * y); 
    if(pBufY == null) 
    { 
     goto fail; 
    } 

    //do something useful 

    returnvalue = true;  

fail: 
    free(pBufX); // <<< free(x) -> free(pBufX) 
    free(pBufY); // <<< free(y) -> free(pBufY) 

    return returnvalue; 
} 

Ich würde nicht Ihre zweite ("böse" gehe zu vermeiden) Lösung recommed. Es ist komplizierter, dass die Goto-Lösung.

BTW ist es sicher, einen Null-Zeiger zu befreien, damit

if(pBufY != NULL)  // NULL and not null 
    free(pBufY);  // not y but pBufY 

kann ersetzt werden durch:

free(pBufY); 

Answer 3

Die richtige Art und Weise dies, meiner Meinung nach zu tun ist, um den Kern zu bewegen Funktionalität in eine separate Funktion:

inline bool doStuff (int x, int y, int* pBufX, int* pBufy) 

bool myFunc (int x, int y) 
{ 
    bool result; 
    int *pBufX = NULL; 
    int *pBufY = NULL; 

    /* do allocations here if possible */ 

    result = doStuff(x, y, pBufX, pBufY); // actual algorithm 

    free(pBufX); 
    free(pBufY); 

    return result; 
} 

Jetzt nur von doStuff zurückgeben müssen upo n Fehler, ohne sich um die Freigabe kümmern zu müssen. Idealerweise würden Sie das malloc innerhalb der Wrapper-Funktion ausführen und dadurch die Speicherzuweisung vom aktuellen Algorithmus trennen, aber manchmal ist das nicht möglich.

Beachten Sie, dass free garantiert, dass es nichts tut, wenn Sie einen Null-Zeiger darauf übergeben.

EDIT:

Beachten Sie, dass, wenn Sie die Zuordnungen innerhalb doStuff tun Sie Zeiger-to-Zeiger übergeben müssen!

Answer 4

Große Frage! (Ich dachte, ich würde einen Narren es finden, aber nein, seltsam, wie ein wichtiger Aspekt in C scheinbar nie wirklich vor gefragt)

Es gibt zwei Fragen, die ich zu verdecken einen Teil dieses Feld gefunden:

• Valid use of goto for error management in C?
• Any good idioms for error handling in straight C programs?

konzentrieren sich diese vor allem auf die Art und Weise gehe. Lassen Sie uns zuerst darüber sprechen.

Die goto Weg

Wenn Sie einen Code haben, die auf die Zuweisung mehrerer Ressourcen abhängt, die später veröffentlicht werden müssen, können Sie ein Muster wie unten verwenden könnte:

int function(void){ 
    res_type_1 *resource1; 
    res_type_2 *resource2; 

    resource1 = allocate_res_type_1(); 
    if (resource1 == NULL){ goto fail1; } 
    resource2 = allocate_res_type_2(); 
    if (resource2 == NULL){ goto fail2; } 

    /* Main logic, may have failure exits to fail3 */ 

    return SUCCESS; 

    fail3: 
    free_res_type_2(resource2); 
    fail2: 
    free_res_type_1(resource1); 
    fail1: 
    return FAIL; 
} 

können Sie mehr lesen zu diesem Ansatz auf dem ausgezeichneten Regehr-Blog: http://blog.regehr.org/archives/894 verweist auch auf den Linux-Kernel selbst, der häufig dieses Muster verwendet.

Pfeil Code

Dies ist eine der Möglichkeiten, es anders zu machen. Das obige Beispiel sieht wie folgt mit einem „Pfeil“ Muster:

int function(void){ 
    res_type_1 *resource1; 
    res_type_2 *resource2; 
    int  ret = FAIL; 

    resource1 = allocate_res_type_1(); 
    if (resource1 != NULL){ 
     resource2 = allocate_res_type_2(); 
     if (resource2 != NULL){ 

      /* Main logic, should set ret = SUCCESS; if succeeds */ 

      free_res_type_2(resource2); 
     } 
     free_res_type_1(resource1); 
    } 

    return ret; 
} 

Das offensichtliche Problem, mit dem das Muster erhielt seinen Namen der potenziell tief verschachteln (der Code wie ein Pfeil suchen) ist, warum dieses Muster nicht gemocht so viel.

Andere Möglichkeiten

Es gibt keine viel ich von den Anforderungen an die Zuteilung und Freigabe von Ressourcen denken kann (mit Ausnahme der Beflaggung Variante Sie ausführlich in Ihrer Frage). Sie haben mehr Freiheit, wenn Sie keine solche Einschränkung haben, Sie können einige gute Ansätze sehen, die in den anderen Fragen und Antworten beschrieben sind.

Wenn die Ressourcen nicht voneinander abhängig sind, können Sie auch andere Muster verwendet werden könnten (wie frühe Rückkehr die ersten Codebeispiel liefert), aber diese beiden sind diejenigen, die den richtigen Weg mit Ressourcen Depencies umgehen können, wenn Sie müssen das (dh Ressource2 kann nur über einer gültigen Ressource1 zugewiesen werden), und wenn die freie Funktion der gegebenen Ressource nicht die Rückgabe einer fehlgeschlagenen Zuweisung behandelt.