C内存管理初学者问题答案

【问题标题】：C memory management beginner questionC内存管理初学者问题
【发布时间】：2009-08-23 16:01:55
【问题描述】：

好的，我有这个code：

typedef struct faux_crit
{
  char dna[DNALEN+1]; //#define'd to 16
  int x, y;
  int age;
  int p;
  int dir;
} crit;

crit *makeguy(int x, int y)
{
  crit *guy;
  guy = (crit *) malloc(sizeof(crit));
  strcpy(guy->dna, makedna());
  guy->x = x;
  guy->y = y;
  guy->age = guy->p = guy->dir = 0;
  return guy;
}

char *makedna()
{
  char *dna;
  int i;
  dna = (char *) malloc(sizeof(char) * DNALEN+1);
  for(i = 0; i < DNALEN; i++)
    dna[i] = randchar();
  return dna;
}

int main()
{
  int i;
  crit *newguy;
  srand((unsigned) time(0));

  newguy = makeguy(0, 0);
  /*[..]
   just printing things here
   */
  free(newguy);

  return 0;
}

我只想知道我在管理内存时做错了什么，因为 valgrind 报告了内存错误。我认为它是 makedna 中的 dna var，但我应该什么时候释放它？退出函数后无法访问，需要返回，所以在此之前无法释放。

编辑：好的，谢谢大家。

【问题讨论】：

请不要发布指向您的代码的链接。相反，发布实际的、最小的、可编译的代码来说明您的问题，并与您的问题保持一致。

标签： c memory-management

【解决方案1】：

最简单的解决方法是像这样更改 makeguy()：

char* dna = makedna();
strcpy(guy->dna, dna);
free(dna);

但这不是一个好的解决方案，因为您在一个位置分配内存并在另一个位置释放它。最好在同一个地方做 malloc 和 free 。所以我建议将 makedna() 更改为：

void* makedna(char* dna, int dna_len)
{
  int i;
  for(i = 0; i < dna_len; i++)
    dna[i] = randchar();
}

你可以这样调用makedna()：

char* dna = (char*)malloc(DNALEN+1);
makedna(dna, DNALEN);
dna[DNALEN] = 0;
strcpy(guy->dna, dna);
free(dna);

现在 makedna() 只做它应该做的事情：制作一个 dna 序列。内存管理应该由调用者负责。此外，如果在不同的调用站点需要，此解决方案提供了使用静态 char 数组的灵活性。

【讨论】：

+1 提出了关于消除冗余分配的出色观点。
请注意，即使是调用者也不需要分配内存，因为 guy->dna 已经分配了。而且你仍然有 strcpy 和 null 终止的问题。
是的，调用者不需要分配内存。这就是我所说的“此外，此解决方案提供了使用静态 char 数组的灵活性，如果在不同的呼叫站点需要的话”。我修复了空终止错误。感谢您指出这一点。

【解决方案2】：

你应该这样做：

char *tempdna = makedna();
strcpy(guy->dna, tempdna);
free(tempdna);

但要使strcpy 工作，您的makedna 函数需要以零结尾的字符串。最后，在返回之前，有：

dna[DNALEN] = 0;

【讨论】：

strcpy 还是有问题，除非 makedna 被固定为 null 终止它创建的字符串。
嗯？这正是他的dna[DNALEN] = 0; 线路正在做的事情

【解决方案3】：

您应该在将makedna() 的指针传递给strcpy 之前存储它，这样您就可以在完成后释放它。

char* dna = makedna();
strcpy(guy->dna, dna);
free(dna);

【讨论】：

strcpy 还是有问题，除非 makedna 被固定为 null 终止它创建的字符串。
确实，我确实忘了提及这一点。看看克里斯的回答。

【解决方案4】：

您应该更改 makedna() 以获取参数：

void makedna(char* dna)
{
  int i;
  for(i = 0; i < DNALEN; i++)
  {
    dna[i] = randchar();
  }
}

为了清楚起见，请注意额外的大括号

第 14 行变为：

makedna(guy->dna);

这避免了至少一组混淆 malloc 和 free。
编辑：
此解决方案还避免了 strcpy 的任何空终止问题。

【讨论】：

-1 表示“样式的额外大括号”；否则你的帖子就不错了。
@Neil：是的，编码风格的圣战！ :-P
@anyone：它不是编码风格，而是将来别人在 for 下添加另一行时的安全性。没有大括号，它并不总是很明显，它没有执行多次。使用大括号时，您必须考虑您是指要由 'for' 执行的额外行，还是之后执行。
@Chris Downvoting 仅仅因为你不同意他们的编程风格显然是错误的，特别是当（正如你指出的）答案是正确的时候。
WRT 在for 之后添加另一行：我来自认为您的缩进应该与代码的意图相匹配的学派，是的，当添加第二条语句时，然后大括号应同时添加。然后，一个人在精神上学会阅读“没有大括号的多个缩进行”，就像“switch case not after break (or return, continue, throw, etc.)”一样多的WTF。在单行语句中添加大括号会大大增加它们的体积。

【解决方案5】：

fbereton 是对的 - 你不需要用于 DNA 的 malloc。

但是，如果您在结构中存储了 malloc 的内存，关键是您不能只释放结构并期望释放其他内存分配。您希望在释放结构本身之前释放结构内的所有分配。

【讨论】：

发帖者没有 malloc'ing 结构中的 dna 变量，他正在 malloc'ing 一个同名的局部变量。此外，我认为他意识到为了使free(guy) 成为一个简单的调用，他需要dna 成员是一个静态分配的数组。

【解决方案6】：

你猜对了。该问题与makedna 函数有关。但是，该功能没有任何问题。更多的是关于你如何使用它。

makedna 返回一个指向已分配内存的指针是完全可以的。但是，由调用函数在完成后释放该内存。

因此我建议你改变：

crit *guy;
guy = (crit *) malloc(sizeof(crit));
strcpy(guy->dna, makedna());

到：

crit *guy;
guy = (crit *) malloc(sizeof(crit));
char * dna = makedna();
strcpy(guy->dna, dna);
// Now that we copied the content of the dna string
// we can free it.
free(dna);

编辑：作为Chris Jester-Young points out，您的makedna 函数也存在一个小问题。您需要对从该函数返回的值进行空终止，以便strcpy 可以正常工作。在返回结果之前将其添加到您的 makedna 函数中：

dna[DNALEN] = 0;

【讨论】：

strcpy 还是有问题，除非 makedna 被固定为 null 终止它创建的字符串。
是的，没错。我本来就错过了。将编辑帖子以反映这一点。

【解决方案7】：

第 3 行 : dna[DNALEN+1] // 包含 [DNALEN+1] 元素的静态声明数组 - 内存已分配第 25 行：不需要 malloc，因为数组已经存在

【讨论】：

Line 25 指的是一个名为dna 的局部变量，而不是您所指的结构的成员。