Выделение памяти для строки внутри структуры

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]);
}

Ошибка сегментации при вводе strncpy для добавления песни в "add_song ()".

Что я делаю критически неправильно?как слышали несколько раз, «правильного» способа реализации в c не существует, если он работает и не глючит, это нормально, но для новичка было бы замечательно получить некоторую осторожную обратную связь или советы по использованию распределения памятинаряду со сложными структурами данных.

Спасибо большое!/ С

Answer 1

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

Это не очень хорошая идея.Вы должны петь x через a->songs[x], поэтому вам нужно выделить a->songs как (char**)malloc(sizeof(char*)*numsongs).Когда есть только одна песня, вы все равно должны поместить ее в под-указатель.

Одна из причин, по которой вы используете segfaulting, заключается в том, что в приведенном выше нет +1 для NUL, как у вас везде... Во-вторых, вы не добавили +1 к длине strncpy, поэтому на самом деле ничего не завершается.

Answer 2

Проблема в том, что 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]);

только освободит структуры, но не строки, на которые указывают эти структуры, и вы получите утечку памяти.

Answer 3

По моему мнению, то, что вы делаете критически неправильно, не использует надлежащие инструменты: -)

Одной из проблем является следующая строка:

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

Вы выделяете количество байтов, равноедлина первой песни, но вы хотите массив указателей на символы.Это может сработать по счастливой случайности, если название первой песни содержит больше символов, чем количество байтов, необходимое для количества используемых указателей символов.

Но было бы лучше сделать

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

или даже динамически расширять массив 'song' и realloc, когда это необходимо.

Кстати, вы никогда не инициализируете songs_c чему-либо, что означает, что вы, возможно, вообще не выделяли songs.

Далее вы выделяете albums с помощью

albums = malloc( sizeof(struct album));

Опять же, это может сработать из-за глупой удачи, поскольку размер двух указателей может быть меньше размера struct album, но я думаю,Вы действительно имели в виду

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

Все эти проблемы должны быть обнаружены либо статическим анализом кода, либо средствами анализа времени выполнения.

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% портативна, так как не стандартизирована.Конечно, вы должны выполнить замену для всех мест, где вам нужно скопировать строку в динамически выделенную память.

Answer 5

В функции initalbum переменная song_c для альбома [1] не инициализируется. Это будет иметь значение мусора.

В функции add_song, поскольку индекс не инициализирован, он вызывает sEGV.