c - 释放内存时堆损坏

Question

我正在努力处理这段代码。顾名思义，函数应该返回字符串数组，表示作为参数给出的字符串的所有旋转。

char **str_all_rotations(const char *data) 
{
    int i = 0; /* Loop counter */ 
    len = strlen(data); /* Len of input */ 

    /******************/
    /*  malloc memory */

    char **all_rotations = (char**)malloc(sizeof(char*)* len);
    char *double_data = (char*)malloc(len * 2 * sizeof(char));

    for (i = 0; i < len; i++) 
    {
         all_rotations[i] = (char*)malloc(sizeof(char)* len);
    }

    /*******************/
    /*  Rotations part */

    strcpy(double_data, data);
    strcpy(double_data + len, data);

    for (i = 0; i < len; i++) 
    {
        strncpy(all_rotations[i], double_data + i, len);
        all_rotations[i][len] = '\0';
    }

    free(double_data); /* Release memory */

    return all_rotations;
}

从算法的角度来看它工作得很好，但是这个函数的简单调用

char *str = "omgillsetyouonfire";
char **asdf = str_all_rotations(str);

for (int i = 0; i < strlen(str); i++) 
{
    free(asdf[i]);
}

free(asdf);

由于堆损坏而失败。我看不出有什么问题。如何调试这种错误？

Answer 1

您的代码存在一些问题

1. 当您使用时

   strcpy(double_data + len, data);
   

   您将一个额外的字节复制到 double_data，即您没有为其分配空间的 nul 终止符，因此您应该像这样分配空间

   char *double_data = malloc(2 * len + 1));
   

2. 这同样适用于for循环中的分配，即

   all_rotations[i] = (char*)malloc(sizeof(char)* len);
   

   当然修复是

   all_rotations[i] = malloc(1 + len);
   

3. 您从不检查 malloc() 是否返回 NULL，这是不好的做法。

4. Do not cast the return value of malloc()

5. 不要使用 strlen() 作为循环的条件，除非字符串的长度在循环内发生变化，因为 strlen() 会计算长度每次调用时的字符串，因此您正在创建一个 O(n) 算法 O(n²)。

6. 标准要求 sizeof(char) == 1，因此它只会让您的代码变得困惑。

这是您自己的代码，旨在解决上述问题

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

char **
str_all_rotations(const char *const data)
 {
    int    index;
    char **all_rotations;
    char  *double_data;
    int    length;

    if (data == NULL)
        return NULL;
    length        = strlen(data);
    index         = 0;
    all_rotations = malloc(length * sizeof(*all_rotations));
    if (all_rotations == NULL)
        return NULL;
    double_data = malloc(2 * length + 1);
    if (double_data == NULL)
        goto cleanup;
    for (index = 0 ; index < length ; index++)
     {
        all_rotations[index] = malloc(1 + length);
        if (all_rotations[index] != NULL && index < 4)
            continue;
        goto cleanup;
     }
    memcpy(double_data, data, length);
    memcpy(double_data + length, data, length);

    double_data[2 * length] = '\0';
    for (index = 0 ; index < length ; index++)
     {
        memcpy(all_rotations[index], double_data + index, length);
        all_rotations[index][length] = '\0';
     }
    free(double_data);

    return all_rotations;

cleanup:
    while (index >= 0)
        free(all_rotations[index--]);
    free(all_rotations);
    free(double_data);

    return NULL;
 }

int
main(void)
 {
    char  *str  = "omgillsetyouonfire";
    char **asdf = str_all_rotations(str);

    if (asdf != NULL)
     {
        for (int i = 0 ; str[i] != '\0' ; i++)
         {
            printf("%s\n", asdf[i]);
            free(asdf[i]);
         }
        free(asdf);
     }

    return 0;
 }