Valgrind 修复内存泄漏答案

【问题标题】：Valgrind to fix memory leaksValgrind 修复内存泄漏
【发布时间】：2017-07-29 13:47:14
【问题描述】：

我有一个程序，我正在阅读一个包含 30,000 个数字的文本文件，这些数字是空格分隔的（每行 5 个）。当我运行程序时，有时它可以工作，但有时它不能（段错误），我相信这意味着我有内存泄漏。

我有以下几点：

int main(int argc, char const *argv[])
{

    char input[256];
    char buffer[30000];
    char **nums = NULL;
    nums = malloc(sizeof(char) * 30000);

    char *token;
    int counter = 0;
    FILE *fp;
    fp = fopen("textfile.txt","r");

    fgets(input,sizeof(input),stdin);

    while (fgets(buffer,sizeof(buffer),fp) != NULL)
    {
        token = strtok(buffer," ");
        nums[counter] = malloc(sizeof(char) * 50);

        while (token != NULL)
        {
            if (strlen(token) > 1)
            {
                strcpy(nums[counter],token);
                counter++;
            }
            token = strtok(NULL," ");
        } 
    }

    for (int x = 0; x < 30000;x++)
    {
        free(nums[x]);
    }

    free(nums);
    fclose(fp);

    return 0;
}

我正在运行 valgrind 以尝试调试，但在读取输出时遇到问题。谁能告诉我哪里出错了？

==24368== Use of uninitialised value of size 8
==24368==    at 0x4C2588C: strcpy (mc_replace_strmem.c:311)
==24368==    by 0x400820: main (in /home/a.out)
==24368==
==24368== Invalid write of size 1
==24368==    at 0x4C2588C: strcpy (mc_replace_strmem.c:311)
==24368==    by 0x400820: main (in /home/a.out)
==24368==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==24368==

【问题讨论】：

如果你打开调试符号，让编译后的代码用函数名、文件名和行号等注释，valgrind 输出将包括文件和行号。对于大多数编译器，它的 -g.
你甚至没有为counterth 令牌分配内存，除了每行的第一次迭代。另外，因为这些是 numbers，正如你所说，我不确定你为什么要将 strings 存储在数组中？
不清楚您是在阅读 30000 个数字还是 6000 行（每行 5 个数字）。为什么需要存储 6000（30000？）行，而不是 30000 数字？这是一个 X-Y 问题吗？你真的想要这些数字的二维数组[6000][5] 吗？还有为什么要扔掉第一行，是列标题吗？
我正在阅读 6000 行，但我将该行中的每个数字存储到数组中，这给了我一个包含 30,000 个数字的数组@WeatherVane

标签： c parsing dynamic memory-leaks allocation

【解决方案1】：

nums = malloc(sizeof(char) * 30000);

不会在数字上保存 30000 个指针。尺寸太小，应该是：

nums = malloc(sizeof(char*) * 30000);

除此之外，不要做：

token = strtok(buffer," ");
nums[counter] = malloc(sizeof(char) * 50);

你应该这样做：

token = strtok(buffer," ");
nums[counter] = malloc(strlen(token)+1);

因此您为每个令牌分配适当数量的内存（不要太多，但不要太少），并注意 sizeof(char) 始终为 1，因此省略它。

【讨论】：

【解决方案2】：

如果您使用调试符号进行编译，通常使用-g，那么 valgrind 将在其输出中包含文件和行号，使其更容易理解。

@Jean-FrançoisFabre already found one malloc problem，您的 while 循环包含更多内容。

while (fgets(buffer,sizeof(buffer),fp) != NULL)
{
    token = strtok(buffer," ");
    nums[counter] = malloc(sizeof(char) * 50);

你总是为nums[counter]分配内存，但是...

    while (token != NULL)
    {
        if (strlen(token) > 1)
        {
            strcpy(nums[counter],token);
            counter++;

您有时只会增加counter。你会泄漏很多内存。

您还将token（最多可达 29998 个字节）复制到 nums[counter]（只有 50 个字节）中。

        }
        token = strtok(NULL," ");
    } 
}

改为根据需要分配nums[counter]，并分配到合适的大小。

while (fgets(buffer,sizeof(buffer),fp) != NULL)
{
    token = strtok(buffer," ");

    while (token != NULL)
    {
        if (strlen(token) > 1)
        {
            nums[counter] = malloc(sizeof(char) * (strlen(token) + 1));
            strcpy(nums[counter],token);
            counter++;
        }
        token = strtok(NULL," ");
    } 
}

或者，如果您不介意使用 POSIX 函数，请使用 strdup。

nums[counter] = strdup(token);

而且，正如其他人在 cmets 中指出的那样，为什么将数字存储为字符串是值得怀疑的。立即使用strtol 或strtod 将它们转换为数字并存储。它更简单，消耗的内存更少。

long nums[30000];

...

char *end;
nums[counter] = strtol(token, &end, 10);
if( end != '\0' ) {
    fprintf(stderr, "Couldn't understand %s\n", token);
}

end 是指向token 上的点的指针，strtol 停止解析。检查它是否为空字节要求token 必须只包含数字。您对转换的要求有多严格取决于您。

【讨论】：