構造体内の文字列にメモリを割り当てる

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

そして主な機能：に曲を追加するためにはstrncpyを発行

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（）"

私は批判的に何をしていますか？何度も聞いたように、動作していてバグでない限り、Cの実装方法は「正しい」ものではありません。大丈夫ですが、初心者であれば、メモリの割り当てに関する注意やフィードバック複雑なデータ構造とともに

ありがとうございました！あなたは、バッファのみを文字列自体のためのスペースがあり、それを伝えるため、

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

： /sの

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)と割り当てる必要があります。ソングが1つだけの場合でも、それをサブポインタに配置する必要があります。あなたは他のどこでも持っているように上記NULのための+1を持っていないので、あなたがセグメンテーションフォルトしている

一つの理由は...もう一つは、あなたが+1strncpy長さに追加していないので、何が実際に得ないということです終了しました。

Answer 2

問題がstrncpy()はあなたのための文字列をNULLで終了しませんです。ここでの解決策は、strcpy()を使用することです。バッファが十分に大きいことがわかります。

また、この：

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

文字列だけが、これらの構造から指さ、あなたがメモリリークを持っている構造を解放しませんが。私の意見で

Answer 3

、

一つの問題点は次の行です:-)何を批判的に間違って行うことは適切なツールを使用していません。

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

あなたがの長さに等しいバイトの量を割り当てます最初の曲ですが、charポインタの配列が必要です。これは愚かな運がうまくいくかもしれません。最初の曲のタイトルには、使用されたcharポインタの数に必要なバイト数より多くの文字があります。

しかし、

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

を行う、あるいは必要に応じて動的かつrealloc「歌」アレイを拡張する方が良いだろう。

ちなみに、songs_cは何にも決して初期化されません。つまり、songsが割り当てられていない可能性があります。

さらに、次の2つのポインタのサイズがstruct albumのサイズよりも小さいかもしれないので、ここでも、これはダム運で働くかもしれない

albums = malloc(sizeof(struct album)); 

でalbumsを割り当てるが、私はあなたが本当に

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

を意味だと思います

これらの問題はすべて、静的コード解析ツールまたは実行時解析ツールによって検出されます。

Answer 4

initalbum関数でalbum_ [1]の変数song_cが初期化されていません。これにはゴミ値があります。

インデックスが初期化されていないため、add_song関数では、sEGVが発生しています。私はそれは後者が明確であることは明らかだと思います

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

：

a->artist = my_strdup(artist); 

：これで

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

：

Answer 5

は真剣にこれを置き換えることを検討します。私は、strncpy()が恐ろしいセマンティクスを持ち、実際には避けなければならないので、機能的な方が良いと思っています。さらに、私の解決策はかなり速いでしょう。あなたのシステムにstrdup()があるなら、それを直接使うことができますが、標準化されていないので100％移植性がありません。もちろん、文字列を動的に割り当てられたメモリにコピーする必要があるすべての場所の置換えを行う必要があります。