使用strcpy出现valgrind错误

Question

这段代码的作用是从用户输入的以 null 结尾的字符串中提取路径。

  /* begin createPath */
    static inline char* createPath(char * userInput)
    {/* This function retuns the path from the userInput */
            int pathStringLength = 0;
            char *buf = userInput;
            while(*(buf++) != ' ')
                    pathStringLength++;
            char *path = malloc(pathStringLength+1);
            strncpy(path, userInput, pathStringLength);
    //      memcpy(path, userInput, pathStringLength);
            path[pathStringLength+1] = '\0';        
            return path;
    }
    /* end createPath */

根据valgrind的报告，这段代码存在错误：

> ==2919== Conditional jump or move depends on uninitialised value(s)
> ==2919==    at 0x400A87: createPath (in /home/aral/learn/myShell/myShell)
> ==2919==    by 0x400A4C: parseInput (in /home/aral/learn/myShell/myShell)
> ==2919==    by 0x4009C3: main (in /home/aral/learn/myShell/myShell)
> ==2919== 
> ==2919== Invalid write of size 1
> ==2919==    at 0x400AC3: createPath (in /home/aral/learn/myShell/myShell)
> ==2919==    by 0x400A4C: parseInput (in /home/aral/learn/myShell/myShell)
> ==2919==    by 0x4009C3: main (in /home/aral/learn/myShell/myShell)

在stackoverflow上搜索类似的问题时，有些人谈到了添加空终止符，而其他人则提到使用memcpy代替strcpy; 我已经添加了一个空终止符，尝试使用memcpy，但是没有改进，valgrind仍然在抱怨。

我到底做错了什么？如何解决？

- Fingolfin

Answer 1

path[pathStringLength+1] = '\0';

有误。这相当于末尾少了一个字节。你的意思应该是：

path[pathStringLength] = '\0';

如果输入字符串中没有空格，你也会遇到缓冲区溢出。在循环中检查空终止符，并在遇到时终止。我会这样写：

while (*buf != ' ' && *buf != '\0')
{
    pathStringLength++;
    buff++;
}

就我而言，我认为在这里使用memcpy可能是更好的选择。一旦你确定了需要复制的文本量，就可以直接将其传输过去。不需要使用寻找空终止符的字符串函数。您在修复代码时已经检查过它们了。

并且您应该检查malloc的返回值。

- David Heffernan

---

我只是为了清晰起见省略了malloc检查；至于+1，我有点羞愧错过了它lol。 无效写入错误已经消失了；但第一个错误呢？ - Fingolfin

为什么这里使用memcpy更好呢？ - Fingolfin

至于另一个错误，我不确定。你修复了我提到的所有错误吗？包括空终止符检查？ - David Heffernan

@AdelQodmani：memcpy更好，因为它只会复制所请求的字节数。使用strcpy或strncpy会在每个复制的字节上包含一个空值检查。既然您已经确定了要复制多少字节，就没有必要在复制过程中再包含另一个空值检查。 - tomlogic

@DavidHeffernan 和 tomlogic： 非常感谢。 - Fingolfin

Answer 2

你应该写到path[pathStringLength]而不是path[pathStringLength+1]——索引从0开始。

- sions

Answer 3

strncpy 并不是一种比 strcpy 更安全的方法。man strncpy 中写道：

strncpy() 函数类似于 strcpy，但最多只复制 src 的 n 个字节。警告：如果 src 的前 n 个字节中没有 null 字节，则存放在 dest 中的字符串将不会以 null 结尾。

因此，为了防止出现非空终止的情况，下面的做法可以帮助避免问题：

strncpy(des, src, len)
des[len -1] = '\0';

- Sunil Bojanapally

---

这和被问到的问题有什么关联？ - David Heffernan

NULL 被定义为 ((void *) 0)，而 des[len-1] 是 char 类型，因此最好写成 des[len -1] = 0; 或者 des[len -1] = '\0';。 - alk