Quel est le meilleur moyen de libérer de la mémoire après le retour d’une erreur?

Supposons que j’ai une fonction qui alloue de la mémoire à l’appelant:

int func(void **mem1, void **mem2) { *mem1 = malloc(SIZE); if (!*mem1) return 1; *mem2 = malloc(SIZE); if (!*mem2) { /* ... */ return 1; } return 0; } 

J’aimerais connaître votre avis sur le meilleur moyen de libérer () la mémoire allouée au cas où le second malloc () échouerait. Vous pouvez imaginer une situation plus élaborée avec plus de points de sortie d’erreur et plus de mémoire allouée.

Je sais que les gens répugnent à les utiliser, mais c’est la situation idéale pour un goto en C.

 int func( void** mem1, void** mem2 ) { int retval = 0; *mem1 = malloc(SIZE); if (!*mem1) { retval = 1; goto err; } *mem2 = malloc(SIZE); if (!*mem2) { retval = 1; goto err; } // ... goto out; // ... err: if( *mem1 ) free( *mem1 ); if( *mem2 ) free( *mem2 ); out: return retval; } 

C’est là qu’un goto est approprié, à mon avis. Je suivais le dogme anti-goto, mais j’ai changé cela quand on m’a fait remarquer que {…} while (0); comstack dans le même code, mais n’est pas aussi facile à lire. Il suffit de suivre quelques règles de base, comme ne pas revenir en arrière avec elles, les garder au minimum, les utiliser uniquement pour les conditions d’erreur, etc.

 int func(void **mem1, void **mem2) { *mem1 = NULL; *mem2 = NULL; *mem1 = malloc(SIZE); if(!*mem1) goto err; *mem2 = malloc(SIZE); if(!*mem2) goto err; return 0; err: if(*mem1) free(*mem1); if(*mem2) free(*mem2); *mem1 = *mem2 = NULL; return 1; } 

Ceci est un peu controversé, mais je pense que l’approche goto utilisée dans le kernel Linux fonctionne plutôt bien dans cette situation:

 int get_item(item_t* item) { void *mem1, *mem2; int ret=-ENOMEM; /* allocate memory */ mem1=malloc(...); if(mem1==NULL) goto mem1_failed; mem2=malloc(...); if(mem2==NULL) goto mem2_failed; /* take a lock */ if(!mutex_lock_interruptible(...)) { /* failed */ ret=-EINTR; goto lock_failed; } /* now, do the useful work */ do_stuff_to_acquire_item(item); ret=0; /* cleanup */ mutex_unlock(...); lock_failed: free(mem2); mem2_failed: free(mem1); mem1_failed: return ret; } 

C’est une alternative lisible:

 int func(void **mem1, void **mem2) { *mem1 = malloc(SIZE); *mem2 = malloc(SIZE); if (!*mem1 || !*mem2) { free(*mem2); free(*mem1); return 1; } return 0; } 

L’appelant fait-il quelque chose d’utile avec les blocs de mémoire alloués correctement avant la panne? Sinon, l’appelé devrait gérer la désallocation.

Une possibilité d’effectuer le nettoyage de manière efficace consiste à utiliser do..while(0) , ce qui permet d’ do..while(0) où votre exemple return s:

 int func(void **mem1, void **mem2) { *mem1 = NULL; *mem2 = NULL; do { *mem1 = malloc(SIZE); if(!*mem1) break; *mem2 = malloc(SIZE); if(!*mem2) break; return 0; } while(0); // free is NULL-safe free(*mem1); free(*mem2); return 1; } 

Si vous effectuez beaucoup d’allocations, vous pouvez également utiliser votre fonction freeAll() pour effectuer le nettoyage ici.

Personnellement; J’ai une bibliothèque de suivi des ressources (essentiellement une arborescence binary équilibrée) et des wrappers pour toutes les fonctions d’allocation.

Les ressources (telles que la mémoire, les sockets, les descripteurs de fichier, les sémaphores, etc. – tout ce que vous allouez et désallouez) peuvent appartenir à un ensemble.

J’ai également une bibliothèque de traitement des erreurs, dans laquelle le premier argument de chaque fonction est un ensemble d’erreurs et en cas de problème, la fonction rencontrant une erreur envoie une erreur dans l’ensemble d’erreurs.

Si un ensemble d’erreurs contient une erreur, aucune fonction n’est exécutée . (J’ai une macro en haut de chaque fonction qui la fait revenir).

Donc, plusieurs mallocs ressemblent à ceci;

 mem[0] = malloc_wrapper( error_set, resource_set, 100 ); mem[1] = malloc_wrapper( error_set, resource_set, 50 ); mem[2] = malloc_wrapper( error_set, resource_set, 20 ); 

Il n’est pas nécessaire de vérifier la valeur de retour car, en cas d’erreur, les fonctions suivantes ne seront pas exécutées. Par exemple, les mallocs suivants ne se produisent jamais.

Ainsi, lorsque le moment est venu pour moi de désallouer des ressources (par exemple, à la fin d’une fonction, où toutes les ressources utilisées en interne par cette fonction ont été placées dans un ensemble), je désalloue cet ensemble. C’est juste un appel de fonction.

 res_delete_set( resource_set ); 

Je n’ai pas besoin de vérifier spécifiquement les erreurs – il n’y a pas de if () dans mon code vérifiant les valeurs de retour, ce qui le rend maintenable; Je trouve que l’abondance de la vérification des erreurs en ligne détruit la lisibilité et donc la maintenabilité. J’ai juste une belle liste d’appels de fonction.

C’est de l’ art , mec 🙂

Ma propre inclination est de créer une fonction d’argument variable qui libère tous les pointeurs non NULL. Ensuite, l’appelant peut gérer le cas d’erreur:

 void *mem1 = NULL; void *mem2 = NULL; if (func(&mem1, &mem2)) { freeAll(2, mem1, mem2); return 1; } 

Si les déclarations de goto ci-dessus vous horrifient pour une raison quelconque, vous pouvez toujours faire quelque chose comme ceci:

 int func(void **mem1, void **mem2) { *mem1 = malloc(SIZE); if (!*mem1) return 1; *mem2 = malloc(SIZE); if (!*mem2) { /* Insert free statement here */ free(*mem1); return 1; } return 0; } 

J’utilise cette méthode assez régulièrement, mais seulement quand c’est très clair ce qui se passe.

Je suis un peu horrifié par toutes les recommandations pour une déclaration goto!

J’ai constaté que l’utilisation de goto conduit à un code source de confusion qui risque davantage de générer des erreurs de programmation. Ma préférence est maintenant d’éviter son utilisation, sauf dans les situations les plus extrêmes. Je ne l’aurais presque jamais utilisé. Pas à cause du perfectionnisme académique, mais parce qu’un an ou plus plus tard, il semble toujours plus difficile de rappeler la logique globale qu’avec l’alternative que je proposerai.

Étant quelqu’un qui aime refactoriser des choses pour minimiser mon option d’oublier des choses (comme effacer un pointeur), j’appendais d’abord quelques fonctions. Je présume qu’il est probable que je les réutiliserais un peu dans le même programme. La fonction imalloc () effectuerait l’opération malloc avec le pointeur indirect; ifree () annulerait cela. cifree () libérerait de la mémoire sous condition.

Avec cela en main, ma version du code (avec un troisième argument, à des fins de démonstration) serait la suivante:

 // indirect pointer malloc int imalloc(void **mem, size_t size) { return (*mem = malloc(size)); } // indirect pointer free void ifree(void **mem) { if(*mem) { free(*mem); *mem = NULL; } } // conditional indirect pointer free void cifree(int cond, void **mem) { if(!cond) { ifree(mem); } } int func(void **mem1, void **mem2, void **mem3) { int result = FALSE; *mem1 = NULL; *mem2 = NULL; *mem3 = NULL; if(imalloc(mem1, SIZE)) { if(imalloc(mem2, SIZE)) { if(imalloc(mem3, SIZE)) { result = TRUE; } cifree(result, mem2); } cifree(result, mem1); } return result; } 

Je préfère n’avoir qu’un retour d’une fonction, à la fin. Sauter entre les deux est rapide (et à mon avis, un peu sale). Mais plus important encore, vous permet de contourner facilement le code de nettoyage associé par inadvertance.