这个函数有什么问题吗？

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