我的内存管理哪里出了问题？

Question

https://gist.github.com/macarr/b49c0097666a613f639c4eab931f31d4

我正在用 C 编写一个小应用程序，它应该连接到 ecobee API。暂时忽略这是 dumb/terrible/why 天啊，你把 C 用作 REST API 客户端吗，这是一个个人项目，只是为了好玩。

我目前运行遇到内存管理问题。我已经用注释对提供的要点进行了注释，并删除了我认为不相关的代码。基本上，在我到达 getTokens 之前，该应用程序按预期完全工作。然后下面的代码吓坏了：

struct authorizations getTokens(char* apiKey, char* authCode) {
  char* body = "grant_type=ecobeePin&code=";
  body = concat(body, authCode);
  printf("%s!!!!!!!!!!\n", body); //as expected
  concat2(body, "&client_id=");
  printf("%s!!!!!!!!!!\n", body); //as expected
  concat2(body, apiKey);
  printf("%s!!!!!!!!!!\n", body); //garbage
  ...

concat 和 concat2 的功能见要点。

char* concat(const char *s1, const char *s2)
{
    char* result;
    result = malloc(strlen(s1)+strlen(s2)+1);//+1 for the zero-terminator
    if(!result) {
        exit(1);
    }
    strcpy(result, s1);
    strcat(result, s2);
    printf("CONCAT1: %s\n", result);
    return result;
}

void concat2(char *s1, const char *s2) {
    char temp[strlen(s1)];
    strcpy(temp, s1);
    printf("%s:%s\n", temp, s2);
    s1 = realloc(s1, strlen(temp) + strlen(s2) + 1);//+1 for the null terminator
    if(!s1) {
        exit(1);
    }
    strcpy(s1, temp);
    strcat(s1, s2);
    printf("CONCAT2: %s\n", s1);
}

在我的函数结束时，我 free(body) 终止了应用程序，因为显然 body 已经被释放了。我想我的 realloc 之一不工作了？

最让我困惑的是，当我在两天前处理错误数据时（对 api 进行无效调用，只是从错误中提取信息以填充未来的请求 - 我没有然后设置登录），一切正常。一旦我开始获取真实数据，应用程序就会启动 "stack smashing"。这是我昨晚所能推动的。

此外，欢迎就字符串连接和指针操作方面的错误提出任何一般性建议。假设我已经听说为什么我不应该使用 C 作为 REST API 客户端

Answer 1

好的，您这里有一个主要问题：

s1 = realloc(s1, strlen(temp) + strlen(s2) + 1);

这可能会改变 s1 的值，一旦您退出函数 ，它就会立即丢失。您没有 return 这个新的指针值返回给调用者，所以它使用旧的，现在 无效的 指针值。因此损失。

您需要使 s1 的更改值可供调用者使用，这意味着您必须传递指向它的指针：

void concat2(char **s1, const char *s2) {
    /**
     * You do not need to preserve the contents of s1 here - if successful,
     * realloc will copy the contents of s1 to the new memory; if not, 
     * it will leave the existing contents in place.  
     * 
     * Because realloc can return NULL on failure, you should *not*
     * assign the result back to the original pointer, but instead
     * assign it to a temporary; that way, if realloc does fail, you
     * don't lose the reference to the previously allocated memory
     */
    char *tmp = realloc(*s1, strlen(*s1) + strlen(s2) + 1);//+1 for the null terminator
    if(!tmp) {
        // handle realloc error
    }
    *s1 = tmp;
    strcat(*s1, s2);
    printf("CONCAT2: %s\n", *s1);
}

然后您将其称为

concat2(&body, "&client_id=");
...
concat2(&body, apiKey);

Answer 2

正如@n.m指出的那样。和@BLUEPIXY，concat2 存在严重问题。假设您不想再花时间自己弄清楚，剧透如下...

主要问题是 concat2 尽职尽责地获取 s1 指向的缓冲区，重新分配它以确保它足够大以进行串联，将指向全新调整大小的缓冲区的指针存储在 s1。然后它将字符串连接到那个新缓冲区中，然后——当函数结束时——它抛出 s1 中最重要的新指针值。在第一次调用 concat2 后，您的 getTokens 函数被误认为缓冲区仍位于 body，但该位置很可能已被更改！

（取决于分配器在您的特定平台上的工作方式，它可能 实际上 可能会或很多不会更改，具体取决于新旧大小和分配器详细信息，但您需要假设它可能已经改变。）因此，如果您将 concat2 重写为：

char* concat2(char *s1, const char *s2) {
    char temp[strlen(s1)];
    strcpy(temp, s1);
    s1 = realloc(s1, strlen(temp) + strlen(s2) + 1);//+1 for the null terminator
    if(!s1) {
        exit(1);
    }
    strcpy(s1, temp);
    strcat(s1, s2);
    return(s1);  /* IMPORTANT: new buffer location! */ 
}

并修改您的 concat2 调用，使其看起来像：

body = concat2(body, "&client_id=");
...
body = concat2(body, apiKey);

你会发现事情做得更好。

还有一点需要注意：realloc 已经将缓冲区的先前内容复制到新缓冲区（直到其最初的 malloced 大小；任何添加的内存都将未初始化），因此您根本不需要使用 temp 缓冲区和额外的复制，并且 concat2 可以简化为：

char* concat2(char *s1, const char *s2) {
    s1 = realloc(s1, strlen(s1) + strlen(s2) + 1);//+1 for the null terminator
    if(!s1) {
        exit(1);
    }
    /* now s1 points to the original string at the beginning
     * of a sufficiently large buffer
     */
    strcat(s1, s2);
    return(s1);
}

我的内存管理哪里出了问题？

Where am I going wrong with my memory management?

c

memory-management

ecobee