Modo consigliato per gestire più errori malloc in una singola funzione in C

Question

Qual è il modo consigliato di gestire più errori malloc che potrebbero verificarsi in sequenza come nel codice seguente?

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; 
} 

Il problema di questo approccio è che se il numero di mallocs sono alti, si potrebbe dimenticare di liberare e causare perdite di memoria. Inoltre, se è presente una sorta di registro che deve essere emessa quando si verifica un errore, il codice diventa molto lungo.

Ho visto il codice che li gestisce con un goto, in cui si cancellano tutti i mallocs in un unico posto, solo una volta. Il codice non è lungo, ma non mi piace usare gotos.

C'è un modo migliore di uno di questi due approcci?

Forse il problema è il design in primo luogo. Esiste una regola empirica quando si progettano le funzioni quando si tratta di ridurre al minimo più mallocs?

Modifica: C'è un altro modo che ho visto e utilizzato. Invece di usare goto, si mantiene uno stato del programma e si procede solo se lo stato è OK. Simile al goto ma non all'uso di goto. Ma questo aumenta il numero di istruzioni if ​​che potrebbero far rallentare il codice.

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

Personalmente, preferisco usare goto. Ma dal momento che è sicuro di free(NULL), di solito si può fare tutte le assegnazioni in anticipo:

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; 
} 

Se è necessario la memoria allocata per fare un calcolo per ottenere una dimensione, una nuova attribuzione, la funzione è probabilmente sufficientemente complessa che dovrebbe essere comunque rifatto.

Answer 2

Questa è una possibile soluzione:

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; 
} 

non vorrei consigliare la soluzione secondo ("male" goto evitare). È più complicato che la soluzione goto.

BTW è sicuro di liberare un puntatore nullo, pertanto

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

può essere sostituito da:

free(pBufY); 

Answer 3

Il modo più corretto per fare questo, a mio parere, è quello di spostare il nucleo funzionalità per una funzione separata:

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; 
} 

Ora è solo bisogno di tornare dalla doStuff Upo n errore, senza preoccuparsi di deallocazione. Idealmente, si farebbe il malloc all'interno della funzione wrapper e quindi si separa l'allocazione della memoria dall'algoritmo vero e proprio, ma a volte non è possibile

Si noti che free garantisce che non fa nulla nel caso si passi un puntatore nullo ad esso.

EDIT:

Si noti che se si fanno le allocazioni all'interno doStuff è necessario passare puntatore a puntatori!

Answer 4

Ottima domanda! (Ho pensato che avrei trovato una vittima di esso, ma no, strano, un aspetto così importante in C apparentemente mai chiesto prima)

Ci sono due domande che ho trovato che copre un po 'di questo campo:

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

Questi si concentrano principalmente sulla strada goto. Quindi per prima cosa copriamolo.

Il modo goto

Se si dispone di un codice che dipende da allocare diverse risorse che devono essere rilasciati in seguito, è possibile utilizzare un modello come di seguito:

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; 
} 

Si può leggere di più su questo approccio sull'eccellente blog di Regehr: http://blog.regehr.org/archives/894 anche indicando il kernel Linux stesso che usa frequentemente questo pattern.

codice Freccia

Questo è uno dei possibili modi per farlo altrimenti. L'esempio di cui sopra si presenta così con una "freccia" modello:

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; 
} 

Il problema evidente con cui il modello deve il suo nome è potenzialmente profondo nidificazione (il codice alla ricerca come una freccia), perché questo modello non è stato utile tanto.

Altri modi

Non c'è molto che posso pensare con l'esigenza di allocare e liberare le risorse (fatta eccezione per la variante di segnalazione si dettaglio nella tua domanda). Hai più libertà quando non hai tale vincolo, puoi vedere alcuni buoni approcci descritti nelle altre domande e risposte.

Se le risorse non sono dipendenti l'uno dall'altro, si potrebbe anche utilizzare altri modelli (come rientro anticipato cui il primo esempio di codice fornisce), ma questi due sono quelli che possono affrontare nel modo giusto con depencies risorsa se è necessario (ovvero, resource2 può essere allocata solo su una risorsa valida1) e se la funzione libera della risorsa specificata non gestisce il ritorno di un'allocazione non riuscita.