Geek Answers Handbook

Allocating memory for a string inside a structure

I know this question exists, but I found the answers a bit hazy, long and / or confusing; so I'm going to specifically reference my code to fully understand the gist.

so I got this structure:

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

The following functions are performed:

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

And the main function:

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

Segmentation error on strncpy release to add a song to "add_song ()".

What am I doing critically wrong? as I heard a bunch of times, the “no” “correct” way to implement it in c, if it works and it doesn’t fail, is fine, but as a beginner, it would be great to get some careful feedback or tips on using memory allocation along with complicated ones data structures.

Thank! /with

+5
5
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). , -.

, , , +1 NUL, . , +1 strncpy, .

+3

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

, , , .

+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 *));

.

+1

initalbum song_c [1] . .

add_song, , sEGV.

0

:

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

It seems obvious to me that the latter is clearer. It also improves functionality because it strncpy()has terrible semantics and should actually be avoided, in my opinion. In addition, my decision is likely to be faster. If your system has one strdup(), you can use it directly, but it is not 100% portable, as it is not very standardized. Of course, you must replace all the places where you need to copy a string to dynamically allocated memory.

0
source

More articles:


All Articles