c-这个函数出了什么问题

Question

我想malloc和goto的关系有问题。或者，我想这里正在发生一些记忆的浪费或记忆的腐败。希望有人能给我指出确切的错误。当我编译的时候，它并没有给我任何错误，但是，我的学长坚持认为我有错误。

#define FINISH() goto fini;
BOOL Do()
{
    BOOL stat;
    UINT32 ptr;
    int err;
    ptr = (UINT32)malloc(1000);

    free((void*)ptr);
fini:
    return stat;
}

Answer 1

以下是我在代码中发现的问题

• 当err != ERROR_SUCCESS时，此函数将泄漏内存。它将跳过free调用
• 您正在将返回的malloc存储到一个32位的位置。这不是一个可移植的解决方案。在64位平台上，这会对你的程序造成严重破坏，因为你会截断地址。如果您必须在这里使用非指针类型，请使用size_t（尽管我会将指针放在整型上）
• 本地stat在这里没有明确分配。如果err != ERROR_SUCCESS，则返回垃圾。需要始终为其指定一个值。最简单的方法是提供默认值
• 您不检查malloc的返回值，并可能将隐藏的NULL指针传递到Fun2

这是我建议的编辑功能

BOOL Do()
{
    BOOL stat = FALSE;
    size_t ptr = 0;
    int err;
    ptr = (UINT32)malloc(1000);
    err = Fun1();
    if (err != ERROR_SUCCESS || ptr == 0)
        FINISH();
    else
        stat = Fun2(ptr);
fini:
    free((void*)ptr);
    return stat;
}

Answer 2

malloc返回一个指针。您正在将指针强制转换为整数，但指针和整数不需要具有相同的表示形式。例如，指针大小可能是64位，不适合您的整数。

此外，对象stat可以在函数中使用而不是初始化。在没有显式初始化的情况下，对象stat在其声明之后具有不确定的值。

Answer 3

我们不知道这应该做什么，但如果Fun1()不返回ERROR_SUCCESS，那么ptr永远不会被释放。据推测，这就是你老板所说的错误。

Answer 4

您正在将指针转换为uint32_t，然后再转换回来。这会擦除指针值的上半部分。

Answer 5

无论您做什么，都不是在编译该代码。它有语法错误。

if(foo)
  bar;;
else
  baz

检查您的生成系统。

Answer 6

我的总体评论，以及在C中工作的一般经验…如果你必须进行指针转换，问问自己：你真的必须这样做吗？老实说，您需要进行指针强制转换的情况极为罕见。更常见的是，当人们使用指针强制转换时，是因为他们在理解上有一些差距，不太清楚他们试图做什么或应该做什么，并且试图让编译器警告静音。

ptr = (UINT32)malloc(1000);

非常糟糕！如果你用这个"指针"做任何事情，如果它能在64位平台上工作，你将非常幸运。将指针保留为指针类型。如果您绝对必须将它们存储在一个整数中，请使用保证足够大的uintptr_t。

我想说你可能一直在尝试：

// Allocate 1,000 32-bit integers
UINT32 *ptr = (UINT32*)malloc(1000 * sizeof(UINT32));

然而，对于C代码来说，这也是一种糟糕的形式，它是一种奇怪的C和C++的混合体。与C++不同的是，在C中，您可以只使用void *并隐式地将其带到任何指针类型：

// Allocate 1,000 32-bit integers
UINT32 *ptr = malloc(1000 * sizeof(UINT32));

最后，

free((void*)ptr);

选择void*是另一个大的危险信号，通常是作者不知道他们在做什么的迹象。一旦您将ptr更改为实际的指针类型，只需执行以下操作：

free(ptr);