这个函数有什么问题？答案

【问题标题】：What is wrong in this function?这个函数有什么问题？
【发布时间】：2012-02-25 21:16:18
【问题描述】：

我猜malloc和goto的关系有问题。或者，我猜这里发生了一些内存浪费或内存损坏。希望，有人可以指出我的确切错误。当我编译它并没有给我任何错误，但是我的前辈坚持认为我有错误。

#define FINISH() goto fini;

BOOL Do()
{

    BOOL stat;
    UINT32 ptr;
    int err;

    ptr = (UINT32)malloc(1000);


    free((void*)ptr);

fini:
    return stat;
}

【问题讨论】：

#define FINISH() goto fini; 上帝保佑
请不要使用goto。请不要将goto 隐藏在宏中。
这段代码能编译吗？我认为不会。
为什么给它一个-1，对于不知道如何使用指针类型转换的人或初学者来说，这是一个非常好的点。
这是IOCCC的条目吗？

标签： c pointers compiler-construction malloc goto

【解决方案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;
}

【讨论】：

我认为stat 也没有被初始化，以防if 评估为真。
你不需要ptr != 0 检查：0 转换为空指针，free() 是空指针安全的
@Christoph 如果Fun2(ptr) 需要一个指向有效对象的指针，则需要ptr != NULL，无论如何我建议保留检查。
@ouah Christoph 所指的支票是针对free 电话的。不过，您正在错误地写到Fun2。我更新了我的答案
@JaredPar ah ok ;) 是的，我正在考虑 malloc 返回值。

【解决方案2】：

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

对象stat 也可以在您的函数中使用未初始化。如果没有显式初始化，对象stat 在其声明后具有不确定的值。

【讨论】：

这是一个正确的答案。 Downvoter 会关心解释吗？
@ErnestFriedman-Hill，你认为(UINT32)malloc(1000) 在做什么？
我的不好，反对票被推翻，评论被删除。还有很多其他的问题我没有想到。

【解决方案3】：

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

【讨论】：

【解决方案4】：

您正在将指针转换为 uint32_t 并再次返回。这会擦除指针值的上半部分。

【讨论】：

【解决方案5】：

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

if(foo)
  bar;;
else
  baz

检查您的构建系统。

【讨论】：

【解决方案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);

【讨论】：