在结构体内为字符串分配内存

Question

我知道这个问题已经存在了一段时间，但我发现答案有点模糊、冗长和/或令人困惑；因此，我将特别提到我的代码，以便充分理解重点。

所以我有这个结构：

typedef struct album {
  unsigned int year;
  char *artist;
  char *title;
  char **songs;
  int songs_c;
} album ;

以下是相关功能：

struct album* init_album(char *artist, char *album, unsigned int year){
  struct album *a;
  a= malloc( sizeof(struct album) );
  a->artist = malloc( strlen(artist) + 1);
  strncpy(a->artist, artist, strlen(artist));
  a->title = malloc( strlen(album) +  1);
  strncpy(a->title, album, strlen(album));
  a->year = year;
  return a;
}

void add_song(struct album *a, char *song){
  int index = a->songs_c;
  if (index == 0){
    a->songs = malloc( strlen(song) );
  } else a->songs[index] = malloc( strlen(song)+1 );

  strncpy(a->songs[index], song, strlen(song));
  a->songs_c= a->songs_c+1;
}

还有一个主函数：

int main(void){
  char *name;
  char artist[20] = "The doors";
  char album[20] = "People are strange";
  int year = 1979;

  struct album *a;
  struct album **albums;

  albums = malloc( sizeof(struct album));

  albums[0] = init_album((char *)"hihi", (char *)"hoho", 1988);

  albums[1] = init_album((char *)"hihi1", (char *)"hoho1", 1911);

  printf("%s, %s, %d\n", albums[0]->artist, albums[0]->title, albums[0]->year);
  printf("%s, %s, %d\n", albums[1]->artist, albums[1]->title, albums[1]->year);

  char song[] = "song 1\0";

  add_song(albums[1], song);

  free(albums[0]);
  free(albums[1]);
}

在“add_song()”中使用strncpy添加歌曲时出现分段错误。

我做错了什么？就像听到的那样，C语言没有“正确”的实现方式，只要它能工作且没有bug，就可以了。但对于初学者来说，获取有关内存分配和复杂数据结构的警告反馈或建议是很好的。

非常感谢！ /s

- Taher

---

char song[] = "song 1\0"; 这里添加了两个空字符。只要使用""，编译器就会为您添加一个不可见的空字符，您不必手动添加。"x"与{'x'，'\0'}相同。 - Lundin

@Vlad 这是一个艰难的选择。要么使用C/C++编写并将程序员90%的时间用于内存管理，要么选择另一种语言，并将程序执行时间的90%用于同样的目的。=) - Lundin

@Lundin：并不完全正确。我认为，如果你需要在关键路径上分配/释放内存-这是一个糟糕的设计，并且无论你使用C还是C++都是一个问题，否则从C++使用std::string不会损害你的性能，特别是如果你有一个对象池（甚至是无锁对象池）。 - user405725

Answer 1

问题在于，strncpy()不会自动添加字符串结束符。

a->artist = malloc( strlen(artist) + 1);
strncpy(a->artist, artist, strlen(artist)); // null terminator is not placed

由于您告诉它，缓冲区只有足够字符串的空间。这里的解决方案是只需使用strcpy() - 您可以确定缓冲区足够大。

还有这个：

free(albums[0]);
free(albums[1]);

只释放结构体，而不释放这些结构体指向的字符串，这会导致内存泄漏。

- sharptooth

---

为什么要使用strcpy而不是strncpy与strlen（artist）+1一起使用？ - Taher

@Smokie：它会起作用，但这没有任何意义——你将在字符串上多进行一次扫描和过度设计的代码。strcpy()是你想要的，所以直接使用它即可。 - sharptooth

Answer 2

if (index == 0) {
    a->songs = malloc( strlen(song) );
} else a->songs[index] = malloc( strlen(song)+1 );

这不是一个好主意。你必须通过 a->songs[x] 来播放歌曲x，因此你需要分配 a->songs 作为 (char**)malloc(sizeof(char*)*numsongs)。即使只有一首歌，你也应该将其放入子指针中。

你的程序出现段错误的原因之一是上述代码没有像其他地方一样加上NUL的+1。另一个原因是你没有在 strncpy 的长度中添加+1，因此没有任何内容被终止。

- Borealid

---

我将使用这个函数来动态添加歌曲，每次添加歌曲时malloc会起作用吗？我的先前分配/使用的内存不会丢失吗？ - Taher

Answer 3

在我看来，你关键错误的做法是没有使用适当的工具 :-)

一个问题就是下面这行代码：

a->songs = malloc( strlen(song) );

你分配了与第一首歌曲长度相等的字节数，但你想要一个char指针数组。这可能会因为运气好而起作用，如果第一首歌曲标题的字符数比使用的char指针数量所需的字节数多。

但最好还是这样做：

a->songs = calloc(max_number_of_songs, sizeof(char*));

甚至可以在需要时动态地扩展“songs”数组并使用realloc重新分配空间。

顺便说一句，你从未将songs_c初始化为任何值，这意味着你可能根本没有分配songs的内存。

此外，你使用以下方法分配了albums的内存

albums = malloc( sizeof(struct album));

可能由于两个指针的大小小于 struct album 的大小，这可能会凑巧起作用，但我认为你真正的意思是

albums = calloc(2, sizeof(struct album *));

所有这些问题都应该被静态代码分析或运行时分析工具捕获。

- Christoffer

Answer 4

认真考虑替换这个：

a->artist = malloc(strlen(artist) + 1);
strncpy(a->artist, artist, strlen(artist));

使用这个：

a->artist = my_strdup(artist);

在哪里：

char * my_strdup(const char *s)
{
  char *out = NULL;

  if(s != NULL)
  {
      const size_t len = strlen(s);
      if((out = malloc(len + 1)) != NULL)
         memcpy(out, s, len + 1);
  }
  return out;  
}

我认为显然后者更清晰。从功能上讲，它也更好，因为 strncpy() 的语义很糟糕，我认为应该避免使用。此外，我的解决方案很可能更快。如果您的系统有 strdup()，您可以直接使用它，但它不是100%可移植的，因为它没有很好地标准化。当然，您应该对所有需要将字符串复制到动态分配内存的地方进行替换。

- unwind

Answer 5

在initialbum函数中，album [1]的songs_c变量未初始化。这将具有垃圾值。

在add_song函数中，由于索引未初始化，导致sEGV。

- sunmoon