从错误返回后，释放内存的最佳方法是什么？

Question

假设我有一个为调用方分配内存的函数：

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

    *mem2 = malloc(SIZE);
    if (!*mem2) {
        /* ... */
        return 1;
    }

    return 0;
}

如果第二个malloc()失败，我想听听您对释放分配的内存的最佳方式的反馈。您可以想象一个更复杂的情况，具有更多的错误退出点和更多的已分配内存。

- FunkyMo

Answer 1

我知道人们不太愿意使用它们，但在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;
}      

- FreeMemory

---

在某些情况下，goto语句是可以接受的。我们在Linux内核中经常看到这种清理方式。它运行良好且易于理解。 - Steve Lazaridis

@Steve Lazaridis：没错。和Linux内核一起工作让我明白有时候使用goto语句是可以的。 :) - FreeMemory

除非你预先将mem1和mem2清零，否则它不能可靠地工作。 - Jonathan Leffler

@Jonathtan Leffler：我假设调用者是一个“好的C公民”。除此之外，你需要将它们清零。 - FreeMemory

如果您预先将*mems清零，则无需测试if(*mem)。 - Dour High Arch

Answer 2

在我看来，这里使用goto是合适的。我曾经遵循反对goto的教条，但当有人指出do {...} 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;
}

- Ryan Graham

Answer 3

这可能有点争议，但我认为在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;
}

- jpalecek

---

你完全可以在那里使用if-else。只需编写以下代码：if（mem！= null）{code ...} free（mem）; 您可以更快地阅读代码，编译器也确切知道您想要做什么。 - zoran404

Answer 4

这是一种易读的替代方案：

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

- user97370

Answer 5

个人而言，我有一个资源跟踪库（基本上是一棵平衡二叉树），并且我对所有分配函数进行了包装。

资源（例如内存、套接字、文件描述符、信号量等——任何你分配和释放的东西）可以属于一个集合。

我还有一个错误处理库，其中每个函数的第一个参数都是一个错误集，如果出现问题，遇到错误的函数会将错误提交到错误集中。

如果错误集包含一个错误，则不执行任何函数。（我在每个函数的顶部设置了一个宏，使其返回）。

因此，多个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 );

不需要检查返回值，因为如果发生错误，后续的函数都不会执行，例如下面的malloc永远不会发生。

所以，当我需要释放资源时（比如在一个函数的结尾处，该函数内部使用的所有资源都已经被放入一个集合中），我只需要调用一个函数即可。

res_delete_set( resource_set );

我不需要专门检查错误——我的代码中没有if()语句检查返回值，这使得它易于维护；我发现内联错误检查的增多破坏了可读性和可维护性。我只有一个漂亮简洁的函数调用列表。

这就是艺术，伙计 :-)

- user82238

Answer 6

调用者是否对在失败之前正确分配的内存块执行任何有用的操作？如果没有，被调用方应该处理释放。

一种有效清理的可能性是使用do..while(0)，它允许在您的示例return处break：

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

如果您进行了大量的分配，您可能希望在此处使用您的freeAll()函数来进行清理。

- Christoph

---

不，调用者只是在必要时释放并返回。这段代码难道不是一个掩饰的goto吗？尽管我不反对为此目的使用goto... - Andrew Keeton

@FunkyMo：是的，它基本上就是一个goto，但有一个限制，你只能跳过块的其余部分，也就是不可能做出邪恶的事情[tm]；我个人认为这比使用goto更清晰，我只在需要跳出嵌套循环时使用goto。 - Christoph

@FunkyMo：我经常使用这个模式：它允许一些巧妙的宏魔法：https://dev59.com/q0bRa4cB1Zd3GeqP2KGY#569659 - Christoph

Answer 7

如果以上的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;
}

我经常使用这种方法，但只有在非常清楚发生了什么的情况下才会使用。

- Andrew

Answer 8

我的个人倾向是创建一个可变参数函数，该函数释放所有非NULL指针。然后调用者可以处理错误情况：

void *mem1 = NULL;
void *mem2 = NULL;

if (func(&mem1, &mem2)) {
    freeAll(2, mem1, mem2);
    return 1;
}

- FunkyMo

---

实际上，在空指针上调用free没有惩罚。您不必检查NULL。但是，将其初始化为NULL很重要。 - ypnos

@ypnos：你在想C++的delete。将NULL传递给free()是无效的。 - Captain Segfault

我查了K＆R和全能的Word，它说free（NULL）绝对什么也不做。 - Andrew Keeton

如果你想使用可变参数，你会遇到两个问题。1. 变量参数列表之前的最后一个参数是什么？2. 你怎么知道可变参数列表的结尾在哪里？（像execl()一样停止在NULL上不会让你传递多个NULL指针。） - bk1e

@bk1e 这实际上是我第一次使用可变参数函数，所以当我写它时（在我的帖子之后），我必须包括要释放的元素数量。 - Andrew Keeton

Answer 9

我对所有关于使用goto语句的建议都感到有些恐惧！

我发现使用goto会导致混乱的代码，更容易引起程序员错误。我的偏好是尽可能避免使用它，除非在极端情况下。我几乎从不使用它。这不是因为追求学术完美，而是因为一年或更长时间后，与我将要提出的替代方案相比，总体逻辑似乎更难以回忆。

作为一个喜欢重构事物以最小化遗忘内容（如清除指针）的人，我首先会添加一些函数。我假设我很可能会在同一个程序中多次重复使用它们。函数imalloc()将使用间接指针执行malloc操作；ifree()将撤消此操作。cifree()将有条件地释放内存。

有了这个，我的代码版本（带有第三个参数，以便演示）将像这样：

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

我更喜欢在函数末尾只有一个返回。在中途跳出来虽然快（而且在我看来有点不规范），但更重要的是，这样可以轻松无意中绕过相关的清理代码。

- digijock

---

如果该函数返回FALSE，则会泄漏内存，并且在返回TRUE时释放内存，这不是提问者所询问的。 - Dour High Arch

更正了打字错误 - cond比较应该是(!cond)。 - digijock

Dour High Arch - 如果您能撤回该评论，这会很好，因为该评论是不正确的。我所提出的基本方案确实非常有效，不会泄漏，也不会返回无效指针。 - digijock

我还要补充的是，这直接而准确地回答了原问题提出者所问的问题。 - digijock