Phân bổ bộ nhớ cho một chuỗi bên trong một cấu trúc

Question

Tôi biết câu hỏi này đã xảy ra, nhưng tôi thấy câu trả lời hơi mờ, dài hoặc khó hiểu; vì vậy tôi sẽ đặc biệt tham khảo mã của tôi để có được điểm đầy đủ.

vì vậy tôi đã struct này:

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

các chức năng sau:

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

Và một chức năng chính:

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 lỗi khi phát hành strncpy để thêm một bài hát trong "add_song()".

Tôi đang làm gì sai nghiêm trọng? khi nghe nói nhiều lần, không có cách thực hiện "đúng" trong c, miễn là nó hoạt động và nó không lỗi, nó là ok, nhưng khi mới bắt đầu sẽ là tuyệt vời để nhận được một số phản hồi thận trọng hoặc lời khuyên về việc sử dụng phân bổ bộ nhớ cùng với các cấu trúc dữ liệu phức tạp.

Cảm ơn một nhóm! /s

Answer 1

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

Đây không phải là ý hay. Bạn phải hát x qua a->songs[x], vì vậy bạn cần phân bổ a->songs làm (char**)malloc(sizeof(char*)*numsongs). Khi chỉ có một bài hát, bạn vẫn nên đặt nó vào con trỏ phụ.

Một lý do bạn đang segfaulting là bởi vì ở trên không có một +1 cho NUL như bạn có khắp mọi nơi khác ... khác là bạn không thêm +1 với độ dài strncpy, vì vậy không có gì thực sự được chấm dứt.

Answer 2

Vấn đề là strncpy() sẽ không null-chấm dứt chuỗi dành cho bạn:

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

kể từ khi bạn nói với nó bộ đệm chỉ có không gian cho chuỗi riêng của mình. Giải pháp ở đây là chỉ sử dụng strcpy() - bạn biết chắc chắn rằng bộ đệm đủ lớn.

Ngoài này:

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

chỉ sẽ giải phóng các cấu trúc, nhưng không dây chỉ vào từ những cấu trúc và bạn đã có một rò rỉ bộ nhớ.

Answer 3

Theo tôi, những gì bạn làm giới phê bình sai không sử dụng các công cụ thích hợp :-)

Một vấn đề là dòng sau:

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

Bạn phân bổ một lượng byte tương đương với chiều dài của bài hát đầu tiên, nhưng bạn muốn có một mảng con trỏ char. Điều này có thể làm việc bằng câm may mắn, nó tiêu đề bài hát đầu tiên có nhiều ký tự hơn số byte cần thiết cho số lượng con trỏ char được sử dụng.

Nhưng nó sẽ là tốt hơn để làm

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

hoặc thậm chí mở rộng mảng 'ca khúc' động và realloc khi cần thiết.

Nhân tiện, bạn không bao giờ intialize songs_c cho bất kỳ điều gì, có nghĩa là bạn có thể chưa phân bổ songs.

Hơn nữa, bạn phân bổ albums với

albums = malloc(sizeof(struct album)); 

Một lần nữa, điều này có thể hoạt động bằng cách câm may mắn vì kích thước của hai con trỏ có thể ít hơn so với kích thước của struct album, nhưng tôi nghĩ rằng bạn thực sự có nghĩa là

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

Tất cả những vấn đề này phải được đánh bắt bằng cách phân tích mã tĩnh hoặc các công cụ phân tích thời gian chạy.

Answer 4

Trong chức năng initalbum, biến_c_của album [1] không được khởi tạo. Điều này sẽ có một giá trị rác.

Trong hàm add_song vì chỉ mục không được khởi tạo, nó đang gây ra sEGV.

Answer 5

Nghiêm túc xem xét thay thế này:

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

với điều này:

a->artist = my_strdup(artist); 

đâu:

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

tôi nghĩ rằng nó là rõ ràng rằng sau này là rõ ràng hơn. Nó cũng tốt hơn chức năng khôn ngoan, kể từ khi strncpy() có ngữ nghĩa kinh khủng và thực sự cần tránh, theo ý kiến ​​của tôi. Hơn nữa, giải pháp của tôi là khá có khả năng nhanh hơn. Nếu hệ thống của bạn có strdup(), bạn có thể sử dụng trực tiếp, nhưng nó không phải 100% di động vì nó không được chuẩn hóa tốt. Tất nhiên, bạn nên làm thay thế cho tất cả những nơi mà bạn cần phải sao chép một chuỗi vào bộ nhớ động phân bổ.