我的记忆管理出了什么问题？

Question

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

我正在用C做一个小应用程序，它应该连接到ecobee。暂时忽略这是愚蠢/可怕的/上帝为什么要使用C作为REST客户端，这是一个个人的项目，因为它的乐趣。

我目前在内存管理方面遇到了问题。我用注释注释了提供的要点，并摘录了一些我不认为相关的代码。基本上，在我到达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)，它杀死了应用程序，因为显然身体已经被释放了。我想我的realloc中有一个坏了还是怎么了？

最让我困惑的是，当我两天前处理坏数据时(对api进行无效的调用，并从错误中提取信息以填充未来的请求--当时我还没有设置登录)，一切都很好。一旦我开始获得真正的数据，应用程序就开始“堆栈崩溃”。这就是我昨晚所能做到的。

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

Answer 1

正如@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);
}

Answer 2

好吧，你这里有个大问题：

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

这可能会改变s1的值，一旦退出该函数，它就会立即丢失。您不会将这个新指针值返回给调用方，因此它使用的是旧的、现在无效的指针值。因此造成损失。

您需要使调用方可以使用更改后的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);