为什么这种使用 strcpy 被认为是不好的？

Question

我发现了以下 C 代码，标记为 BAD（也称为缓冲区溢出错误）。 问题是我不太明白为什么？在分配等之前捕获输入字符串长度。

char *my_strdup(const char *s)
{
    size_t len = strlen(s) + 1;
    char *c = malloc(len);
    if (c) {
        strcpy(c, s);  // BAD
    }
    return c;
}

来自 cmets 的更新：

• “BAD”标记不准确，代码还不错，效率不高是的，有风险（下）是的，
• 为什么要冒险？ +1 在 strlen() 调用之后需要安全地分配堆上的空间，这也将保留字符串终止符 ('\0')

Answer 1

在使用strcpy时，我看不出代码有任何问题

但是你应该知道它要求s 是一个有效的C 字符串。这是一个合理的要求，但应该指定。

如果你愿意，你可以对 NULL 做一个简单的检查，但我想说没有它也没关系。如果您要复制一个由空指针指向的“字符串”，那么您可能应该检查参数或结果。但是，如果您愿意，只需将其添加为第一行：

if(!s) return NULL;

但正如我所说，它并没有增加太多。它只是让改变成为可能

if(!str) {
    // Handle error
} else {
    new_str = my_strdup(str);
}

到：

new_str = my_strdup(str);
if(!new_str) {
    // Handle error
}

不是很大的收获

Answer 2

这段代码没有问题。

虽然strcpy 可能会导致undefined behavior 如果目标缓冲区不够大以容纳有问题的字符串，但分配的缓冲区大小是正确的。这意味着没有溢出缓冲区的风险。

您可能会看到一些指南建议使用strncpy，它允许您指定要复制的最大字符数，但这有其自身的问题。如果源字符串太长，只会复制指定数量的字符，但这也意味着字符串不是以空结尾的，这需要用户手动完成。例如：

char src[] = "test data";
char dest[5];

strncpy(dest, src, sizeof dest);  // dest holds "test " with no null terminator
dest[sizeof(dest) - 1] = 0;       // manually null terminate, dest holds "test"

如果我知道源字符串合适，我倾向于使用strcpy，否则我将使用strncpy 并手动空终止。

Answer 3

您的示例函数中没有错误。

然而，为了让未来的读者（人类和机械）显而易见没有错误，您应该将 strcpy 调用替换为 memcpy：

char *my_strdup(const char *s)
{
    size_t len = strlen(s) + 1;
    char *c = malloc(len);
    if (c) {
        memcpy(c, s, len);
    }
    return c;
}

无论哪种方式，len 字节被分配，len 字节被复制，但对于memcpy，这一事实对读者来说更加明显。