这是在C语言中释放内存的好方法吗?

The function for freeing an instance of struct Foo is given below:

释放struct Foo实例的函数如下:

void DestroyFoo(Foo* foo)
{
    if (foo) free(foo);
}

A colleague of mine suggested the following instead:

我的一个同事建议这样做:

void DestroyFoo(Foo** foo)
{
    if (!(*foo)) return;
    Foo *tmpFoo = *foo;
    *foo = NULL; // prevents future concurrency problems
    memset(tmpFoo, 0, sizeof(Foo));  // problems show up immediately if referred to free memory
    free(tmpFoo);
}

I see that setting the pointer to NULL after freeing is better, but I'm not sure about the following:

我看到在释放后将指针设置为NULL更好，但是我不确定以下内容:

Do we really need to assign the pointer to a temporary one? Does it help in terms of concurrency and shared memory?
我们真的需要将指针赋值给一个临时指针吗?它在并发性和共享内存方面有帮助吗?
Is it really a good idea to set the whole block to 0 to force the program to crash or at least to output results with significant discrepancy?
将整个block设置为0，以迫使程序崩溃，或者至少输出有显著差异的结果，这真的是个好主意吗?

Thanks in advance!

提前谢谢!

4 个解决方案

#1

Do we really need to assign the pointer to a temporary one? Does it help in terms of concurrency and shared memory?

我们真的需要将指针赋值给一个临时指针吗?它在并发性和共享内存方面有帮助吗?

It has nothing to do concurrency or shared memory. It's pointless.

它与并发或共享内存无关。这是毫无意义的。

Is it really a good idea to set the whole block to 0 to force the program to crash or at least to output results with significant discrepancy?

将整个block设置为0，以迫使程序崩溃，或者至少输出有显著差异的结果，这真的是个好主意吗?

No. Not at all.

不。不客气。

The solution suggested by your colleague is terrible. Here's why:

你同事提出的解决办法很糟糕。原因如下:

Setting whole block to 0 achieves nothing either. Because someone is using a free()'ed block accidentally, they wouldn't know that based on the values at the block. That's the kind of block calloc() returns. So it's impossible to know whether it's freshly allocated memory (calloc() or malloc()+memset()) or the one that's been free()'ed by your code earlier. If anything it's extra work for your program to zero out every block of memory that's being free()'ed.

将整个块设置为0也没有任何效果。因为某人意外地使用了一个free() ed块，他们不会根据块上的值知道这个值。这就是块calloc()返回的类型。因此，不可能知道它是新分配的内存(calloc()或malloc()+memset())，或者是您的代码之前已经免费使用的()。如果有任何问题的话，您的程序需要做额外的工作来清除所有空闲的内存块()。
free(NULL); is well-defined and is a no-op, so the if condition in if(ptr) {free(ptr);} achieves nothing.

免费(空);定义良好且是一个无op，因此if(ptr) {free(ptr);}中的if条件没有任何作用。
Since free(NULL); is no-op, setting the pointer to NULL would actually hide that bug, because if some function is actually calling free() on an already free()'ed pointer, then they wouldn't know that.

自*(空);是no-op，将指针设置为NULL实际上会隐藏那个错误，因为如果某个函数实际上在一个已经空闲的()ed指针上调用free()，那么它们就不会知道这个错误。
most user functions would have a NULL check at the start and may not consider passing NULL to it as error condition:

大多数用户函数在开始时将有一个空检查，并且可能不考虑将NULL传递为错误条件:

void do_some_work(void *ptr) {
    if (!ptr) {
        return; 
    }

   /*Do something with ptr here */
}

So the all those extra checks and zero'ing out gives a fake sense of "robustness" while it didn't really improve anything. It just replaced one problem with another the additional cost of performance and code bloat.

所以所有这些额外的检查和零输出给人一种虚假的“健壮性”的感觉，而它并没有真正改善什么。它只是替换了一个问题，另一个问题是性能和代码膨胀的额外成本。

So just calling free(ptr); without any wrapper function is both simple and robust (most malloc() implementations would crash immediately on double free, which is a good thing).

所以就叫*(ptr);没有任何包装函数都是简单和健壮的(大多数malloc()实现会立即在double free上崩溃，这是一件好事)。

There's no easy way around "accidentally" calling free() twice or more. It's the programmer's responsibility to keep track of all memory allocated and free() it appropriately. If someone find this hard to handle then C is probably not the right language for them.

“意外”调用free()两次或更多不是一件容易的事情。程序员的职责是跟踪分配的所有内存并适当地释放()它。如果有人觉得这很难处理，那么C语言可能不是适合他们的语言。

#2

What your collegue suggests will make the code "safer" in case the function is called twice (see sleske comment...as "safer" may not mean the same for everybody...;-).

您的同事的建议将使代码“更安全”，以防函数被调用两次。因为“更安全”对每个人来说可能都不一样。

With your code, this will most likely crash:

你的代码很可能会崩溃:

Foo* foo = malloc( sizeof(Foo) );
DestroyFoo(foo);
DestroyFoo(foo); // will call free on memory already freed

With your collegues's version of the code, this will not crash:

你的同事版本的代码，这不会崩溃:

Foo* foo = malloc( sizeof(Foo) );
DestroyFoo(&foo);
DestroyFoo(&foo); // will have no effect

Now, for this specific scenario, doing tmpFoo = 0; (within DestroyFoo) is enough. memset(tmpFoo, 0, sizeof(Foo)); will prevent crash if Foo has extra attributes that could be wrongly accessed after memory is released.

对于这个特定的场景，执行tmpFoo = 0;(DestroyFoo内)就足够了。memset(tmpFoo 0 sizeof(Foo));如果Foo有额外的属性，在内存释放后可能被错误访问，则将防止崩溃。

So I would say yes, it may be a good practice to do so....but it's only a kind of security against developers who have bad practices (because there's definitely no reason to call DestroyFoo twice without reallocating it)...in the end, you make DestroyFoo "safer" but slower (it does more stuff to prevent bad usage of it).

所以我想说的是的,这可能是一个很好的实践....但这只是针对有不良实践的开发人员的一种安全措施(因为如果不重新分配的话，绝对没有理由两次调用DestroyFoo)……最后，你会让“驱逐舰喷”更“安全”，但速度会更慢(它会做更多的事情来防止错误的使用)。

#3

The second solution seems to be over engineered. Of course in some situation it might be safer but the overhead and the complexity is just too big.

第二种解决方案似乎设计过度了。当然，在某些情况下，它可能会更安全，但它的开销和复杂性实在是太大了。

What you should do if you want to be on a safe side is setting the pointer to NULL after freeing memory. This is always a good practice.

如果希望安全，应该在释放内存后将指针设置为NULL。这是一个很好的实践。

Foo* foo = malloc( sizeof(Foo) );
DestroyFoo(foo);
foo = NULL;

What is more, I don't know why people are checking if the pointer is NULL before calling free(). This is not needed as free() will do the job for you.

此外，我不知道为什么人们在调用free()之前要检查指针是否为空。这是不需要的，因为free()将为您完成这项工作。

Setting memory to 0 (or something else) is only in some cases a good practice as free() will not clear memory. It will just mark a region of memory to be free so that it can be reused. If you want to clear the memory, so that no one will be able to read it you need to clean it manually. But this is quite heavy operation and that's why this shouldn't be used to free all the memory. In most cases freeing without clearing is just enough and you don't have to sacrifice performance to do unnecessary operation.

将内存设置为0(或其他)仅在某些情况下作为free()的良好实践不会清除内存。它只是将一个内存区域标记为空闲，以便可以重用它。如果您想要清除内存，那么没有人能够阅读它，您需要手动清理它。但这是一个非常繁重的操作，这就是为什么不应该用它来释放所有内存。在大多数情况下，在不清理的情况下释放是足够的，您不必牺牲性能来进行不必要的操作。

#4

void destroyFoo(Foo** foo)
{
    if (!(*foo)) return;
    Foo *tmpFoo = *foo;
    *foo = NULL;
    memset(tmpFoo, 0, sizeof(Foo));
    free(tmpFoo);
}

Your colleague code is bad because

你的同事代码不好是因为

it will crash if foo is NULL
如果foo是空的，它就会崩溃
there's no point in creating additional variable
创建额外的变量是没有意义的
there's no point in setting the values to zeros
将值设置为0是没有意义的
freeing a struct directly doesn't work if it contain things that has to be freed
如果一个结构体包含必须释放的内容，那么直接释放它将不起作用

I think what your colleague might have in mind is this use-case

我认为你的同事可能会想到这个用例

Foo* a = NULL;
Foo* b = createFoo();

destroyFoo(NULL);
destroyFoo(&a);
destroyFoo(&b);

In that case, it should be like this. try here

在这种情况下，应该是这样的。尝试在这里

void destroyFoo(Foo** foo)
{
    if (!foo || !(*foo)) return;
    free(*foo);
    *foo = NULL;
}

First we need to take a look at Foo, let's assume that it looks like this

首先我们需要看一下Foo，假设它是这样的

struct Foo
{
    // variables
    int number;
    char character;

    // array of float
    int arrSize;
    float* arr;

    // pointer to another instance
    Foo* myTwin;
};

Now to define how it should be destroyed, let's first define how it should be created

现在要定义如何销毁它，让我们首先定义如何创建它

Foo* createFoo (int arrSize, Foo* twin)
{
    Foo* t = (Foo*) malloc(sizeof(Foo));

    // initialize with default values
    t->number = 1;
    t->character = '1';

    // initialize the array
    t->arrSize = (arrSize>0?arrSize:10);
    t->arr = (float*) malloc(sizeof(float) * t->arrSize);

    // a Foo is a twin with only one other Foo
    t->myTwin = twin;
    if(twin) twin->myTwin = t;

    return t;
}

Now we can write a destroy function oppose to the create function

现在我们可以写出一个与create函数相反的销毁函数

Foo* destroyFoo (Foo* foo)
{
    if (foo)
    {
        // we allocated the array, so we have to free it
        free(t->arr);

        // to avoid broken pointer, we need to nullify the twin pointer
        if(t->myTwin) t->myTwin->myTwin = NULL;
    }

    free(foo);

    return NULL;
}

Test try here

测试试

int main ()
{
    Foo* a = createFoo (2, NULL);
    Foo* b = createFoo (4, a);

    a = destroyFoo(a);
    b = destroyFoo(b);

    printf("success");
    return 0;
}

#1