这个函数有什么问题吗？

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