Suppose I have a function that allocates memory for the caller:
假设我有一个为调用者分配内存的函数:
int func(void **mem1, void **mem2) {
*mem1 = malloc(SIZE);
if (!*mem1) return 1;
*mem2 = malloc(SIZE);
if (!*mem2) {
/* ... */
return 1;
}
return 0;
}
I'd like to hear your feedback on the best way to free() the allocated memory in case the second malloc() fails. You can imagine a more elaborate situation with more error exit points and more allocated memory.
在第二个malloc()失败的情况下,我想听听你对free()分配内存的最佳方法的反馈。你可以想象一个更复杂的情况,有更多的错误退出点和更多的分配内存。
9 个解决方案
#1
25
I know people are loathe to use them, but this is the perfect situation for a goto
in C.
我知道人们不喜欢使用它们,但这是C中goto的完美情况。
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;
}
#2
6
This is where a goto is appropriate, in my opinion. I used to follow the anti-goto dogma, but I changed that when it was pointed out to me that do { ... } while (0); compiles to the same code, but isn't as easy to read. Just follow the some basic rules, like not going backwards with them, keeping them to a minimum, only using them for error conditions, etc...
在我看来,这是goto合适的地方。我过去常常遵循反goto教条,但当我向我指出{...} while(0)时,我改变了这一点。编译成相同的代码,但不容易阅读。只需遵循一些基本规则,例如不要与它们一起倒退,将它们保持在最低限度,仅将它们用于错误条件等等......
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;
}
#3
4
This is a bit controversial, but I think the goto
approach used in Linux kernel actually works pretty well in this situation:
这有点争议,但我认为Linux内核中使用的goto方法在这种情况下实际上运行良好:
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;
}
#4
2
This is a readable alternative:
这是一个可读的替代方案:
int func(void **mem1, void **mem2) {
*mem1 = malloc(SIZE);
*mem2 = malloc(SIZE);
if (!*mem1 || !*mem2) {
free(*mem2);
free(*mem1);
return 1;
}
return 0;
}
#5
1
Does the caller do anything useful with the memory blocks which have been correctly allocated before the failure? If not, the callee should handle the deallocation.
调用者是否对在失败之前正确分配的内存块做了有用的事情?如果不是,被调用者应该处理释放。
One possibility to do the cleanup efficiently is using do..while(0)
, which allows to break
where your example return
s:
有效清理的一种可能性是使用do..while(0),它允许破坏示例返回的位置:
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;
}
If you do a lot of allocations, you might want to use your freeAll()
function to do the cleanup here as well.
如果你做了很多分配,你可能也希望使用你的freeAll()函数来进行清理。
#6
1
Personally; I have a resource tracking library (basically a balanced binary tree) and I have wrappers for all allocation functions.
亲自;我有一个资源跟踪库(基本上是一个平衡的二叉树),我有所有分配函数的包装器。
Resources (such as memory, sockets, file descriptors, semaphores, etc - anything you allocate and deallocate) can belong to a set.
资源(例如内存,套接字,文件描述符,信号量等 - 您分配和解除分配的任何内容)都属于一个集合。
I also have an error handling library, whereby the first argument to each function is an error set and if something goes wrong, the function experiencing an error submits an error into the error set.
我还有一个错误处理库,每个函数的第一个参数是一个错误集,如果出现错误,遇到错误的函数会向错误集提交一个错误。
If an error set contains an error, no functions execute. (I have a macro at the top of every function which causes it to return).
如果错误集包含错误,则不执行任何函数。 (我在每个函数的顶部都有一个宏,导致它返回)。
So multiple mallocs look like this;
所以多个malloc看起来像这样;
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 );
There is no need to check the return value, because if an error occurs, no following functions will execute, e.g. the following mallocs never occur.
不需要检查返回值,因为如果发生错误,则不会执行以下功能,例如,以下mallocs永远不会发生。
So, when the time comes for me to deallocate resources (say at the end of a function, where all the resources used internally by that function have been placed into a set), I deallocate the set. It's just one function call.
因此,当我需要释放资源时(比如在函数结束时,该函数内部使用的所有资源都已放入集合中),我会释放该集合。这只是一个函数调用。
res_delete_set( resource_set );
I don't need to specifically check for errors - there are no if()s in my code checking return values, which makes it maintainable; I find the profliferation of in-line error check destroys readability and so maintainability. I just have a nice plain list of function calls.
我不需要专门检查错误 - 我的代码中没有if()s检查返回值,这使得它可以维护;我发现在线错误检查的前瞻性破坏了可读性和可维护性。我只是有一个很好的函数调用列表。
It's art, man :-)
这是艺术,男人:-)
#7
0
My own inclination is to create a variable argument function that frees all non-NULL pointers. Then the caller can handle the error case:
我自己的倾向是创建一个可变参数函数来释放所有非NULL指针。然后调用者可以处理错误情况:
void *mem1 = NULL;
void *mem2 = NULL;
if (func(&mem1, &mem2)) {
freeAll(2, mem1, mem2);
return 1;
}
#8
0
If the above goto statements horrify you for some reason, you can always do something like this:
如果上面的goto语句由于某种原因让你感到恐惧,你总是可以这样做:
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;
}
I use this method pretty regularly, but only when it's very clear what's going on.
我经常使用这种方法,但只有当它非常清楚发生了什么时才会这样。
#9
-2
I'm a little horrified by all the recommendations for a goto statement!
我对goto声明的所有建议感到有点害怕!
I have found that the use of goto leads to confusing code which is more likely to give rise to programmer errors. My preference now is to avoid its use altogether, except in the most extreme situations. I'd almost never use it. Not because of academic perfectionism, but because a year or more later it always seems more difficult to recall the overall logic than with the alternative I will suggest.
我发现使用goto会导致代码混乱,这更容易引起程序员错误。我现在的偏好是完全避免使用它,除非在最极端的情况下。我几乎从不使用它。不是因为学术上的完美主义,而是因为一年或更长的时间后,总是回想起整体逻辑而不是我所建议的替代方案。
Being one who loves to refactor things to minimize my option to forget stuff (like clearing a pointer), I'd add a few functions first. I'll presume it's likely that I would be reusing these quite a bit in the same program. Function imalloc() would do the malloc operation with the indirect pointer; ifree() would undo this. cifree() would free memory conditionally.
作为一个喜欢重构事物以最大限度地减少忘记内容的选择(比如清除指针)的人,我先添加一些函数。我认为我可能会在同一个程序中重复使用这些。函数imalloc()将使用间接指针执行malloc操作; ifree()会撤消这个。 cifree()会有条件地释放内存。
With that in hand, my version of the code (with a third arg, for sake of demonstration) would be like this:
有了这个,我的代码版本(第三个arg,为了演示)将是这样的:
// 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;
}
I prefer to have only one return from a function, at the end. Jumping out in between is quick (and in my opinion, kind of dirty). But more significantly, allows you to easily bypass associated cleanup code unintentionally.
我最后只想从一个函数返回一个。跳出来之间很快(在我看来,有点脏)。但更重要的是,允许您无意中轻松绕过相关的清理代码。
#1
25
I know people are loathe to use them, but this is the perfect situation for a goto
in C.
我知道人们不喜欢使用它们,但这是C中goto的完美情况。
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;
}
#2
6
This is where a goto is appropriate, in my opinion. I used to follow the anti-goto dogma, but I changed that when it was pointed out to me that do { ... } while (0); compiles to the same code, but isn't as easy to read. Just follow the some basic rules, like not going backwards with them, keeping them to a minimum, only using them for error conditions, etc...
在我看来,这是goto合适的地方。我过去常常遵循反goto教条,但当我向我指出{...} while(0)时,我改变了这一点。编译成相同的代码,但不容易阅读。只需遵循一些基本规则,例如不要与它们一起倒退,将它们保持在最低限度,仅将它们用于错误条件等等......
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;
}
#3
4
This is a bit controversial, but I think the goto
approach used in Linux kernel actually works pretty well in this situation:
这有点争议,但我认为Linux内核中使用的goto方法在这种情况下实际上运行良好:
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;
}
#4
2
This is a readable alternative:
这是一个可读的替代方案:
int func(void **mem1, void **mem2) {
*mem1 = malloc(SIZE);
*mem2 = malloc(SIZE);
if (!*mem1 || !*mem2) {
free(*mem2);
free(*mem1);
return 1;
}
return 0;
}
#5
1
Does the caller do anything useful with the memory blocks which have been correctly allocated before the failure? If not, the callee should handle the deallocation.
调用者是否对在失败之前正确分配的内存块做了有用的事情?如果不是,被调用者应该处理释放。
One possibility to do the cleanup efficiently is using do..while(0)
, which allows to break
where your example return
s:
有效清理的一种可能性是使用do..while(0),它允许破坏示例返回的位置:
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;
}
If you do a lot of allocations, you might want to use your freeAll()
function to do the cleanup here as well.
如果你做了很多分配,你可能也希望使用你的freeAll()函数来进行清理。
#6
1
Personally; I have a resource tracking library (basically a balanced binary tree) and I have wrappers for all allocation functions.
亲自;我有一个资源跟踪库(基本上是一个平衡的二叉树),我有所有分配函数的包装器。
Resources (such as memory, sockets, file descriptors, semaphores, etc - anything you allocate and deallocate) can belong to a set.
资源(例如内存,套接字,文件描述符,信号量等 - 您分配和解除分配的任何内容)都属于一个集合。
I also have an error handling library, whereby the first argument to each function is an error set and if something goes wrong, the function experiencing an error submits an error into the error set.
我还有一个错误处理库,每个函数的第一个参数是一个错误集,如果出现错误,遇到错误的函数会向错误集提交一个错误。
If an error set contains an error, no functions execute. (I have a macro at the top of every function which causes it to return).
如果错误集包含错误,则不执行任何函数。 (我在每个函数的顶部都有一个宏,导致它返回)。
So multiple mallocs look like this;
所以多个malloc看起来像这样;
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 );
There is no need to check the return value, because if an error occurs, no following functions will execute, e.g. the following mallocs never occur.
不需要检查返回值,因为如果发生错误,则不会执行以下功能,例如,以下mallocs永远不会发生。
So, when the time comes for me to deallocate resources (say at the end of a function, where all the resources used internally by that function have been placed into a set), I deallocate the set. It's just one function call.
因此,当我需要释放资源时(比如在函数结束时,该函数内部使用的所有资源都已放入集合中),我会释放该集合。这只是一个函数调用。
res_delete_set( resource_set );
I don't need to specifically check for errors - there are no if()s in my code checking return values, which makes it maintainable; I find the profliferation of in-line error check destroys readability and so maintainability. I just have a nice plain list of function calls.
我不需要专门检查错误 - 我的代码中没有if()s检查返回值,这使得它可以维护;我发现在线错误检查的前瞻性破坏了可读性和可维护性。我只是有一个很好的函数调用列表。
It's art, man :-)
这是艺术,男人:-)
#7
0
My own inclination is to create a variable argument function that frees all non-NULL pointers. Then the caller can handle the error case:
我自己的倾向是创建一个可变参数函数来释放所有非NULL指针。然后调用者可以处理错误情况:
void *mem1 = NULL;
void *mem2 = NULL;
if (func(&mem1, &mem2)) {
freeAll(2, mem1, mem2);
return 1;
}
#8
0
If the above goto statements horrify you for some reason, you can always do something like this:
如果上面的goto语句由于某种原因让你感到恐惧,你总是可以这样做:
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;
}
I use this method pretty regularly, but only when it's very clear what's going on.
我经常使用这种方法,但只有当它非常清楚发生了什么时才会这样。
#9
-2
I'm a little horrified by all the recommendations for a goto statement!
我对goto声明的所有建议感到有点害怕!
I have found that the use of goto leads to confusing code which is more likely to give rise to programmer errors. My preference now is to avoid its use altogether, except in the most extreme situations. I'd almost never use it. Not because of academic perfectionism, but because a year or more later it always seems more difficult to recall the overall logic than with the alternative I will suggest.
我发现使用goto会导致代码混乱,这更容易引起程序员错误。我现在的偏好是完全避免使用它,除非在最极端的情况下。我几乎从不使用它。不是因为学术上的完美主义,而是因为一年或更长的时间后,总是回想起整体逻辑而不是我所建议的替代方案。
Being one who loves to refactor things to minimize my option to forget stuff (like clearing a pointer), I'd add a few functions first. I'll presume it's likely that I would be reusing these quite a bit in the same program. Function imalloc() would do the malloc operation with the indirect pointer; ifree() would undo this. cifree() would free memory conditionally.
作为一个喜欢重构事物以最大限度地减少忘记内容的选择(比如清除指针)的人,我先添加一些函数。我认为我可能会在同一个程序中重复使用这些。函数imalloc()将使用间接指针执行malloc操作; ifree()会撤消这个。 cifree()会有条件地释放内存。
With that in hand, my version of the code (with a third arg, for sake of demonstration) would be like this:
有了这个,我的代码版本(第三个arg,为了演示)将是这样的:
// 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;
}
I prefer to have only one return from a function, at the end. Jumping out in between is quick (and in my opinion, kind of dirty). But more significantly, allows you to easily bypass associated cleanup code unintentionally.
我最后只想从一个函数返回一个。跳出来之间很快(在我看来,有点脏)。但更重要的是,允许您无意中轻松绕过相关的清理代码。