holder = (song_t *)malloc(sizeof(song_t));
/* snip */
/* else if */
holder = temp2->next->next;
/* else */
holder = temp;
temp2->next = temp->next;
free(holder);
當分配holder = temp2->next->next;
(順便說一句,這總是NULL
那裏),或holder = temp;
,你失去你的參考malloc
版內存。由於您根本沒有真正使用holder
,因此修復方法是將其從功能中刪除。在第一種情況下,除非指定複雜的NULL
,否則在第二種情況下除去holder = temp;
和free
ing temp
是正確的方法。
有一些更奇怪的事情和錯誤:
song_t *DeleteSong(song_t *head, string key){
song_t *temp, *temp2, *holder;
holder = (song_t *)malloc(sizeof(song_t)); // as said, remove holder completely
temp = head;
/* Find song with given title */
while(temp != NULL && strcasecmp(temp->title, key) != 0){
temp = temp->next;
}
if(temp == NULL){
newline;
printf("Song not found.");
newline;
newline;
}
/* It's the very first in the list */
else if(temp == head){
if(temp->next == NULL){
/* It's even the only one */
temp->next = NULL; // This runs only if temp->next is already NULL
free(temp); // Also free the members of temp, or you're leaking
return NULL;
}
else{
head = temp->next; // You should now free temp and members, or you're leaking memory
}
}
/* It's the last one in the list, but not the first */
else if(temp->next == NULL){
temp2 = head;
/* Find the penultimate song */
while(temp2->next->next != NULL){
temp2 = temp2->next;
}
holder = temp2->next->next;
temp2->next = NULL; // You should now free temp and members, or you're leaking memory
}
/* Neither first nor last */
else{
temp2 = head;
/* Find song before */
while(temp2->next != temp){
temp2 = temp2->next;
}
holder = temp;
temp2->next = temp->next;
free(holder);
}
return head;
}
但除了泄漏,它是正確的,但不必要的複雜。
song_t *DeleteSong(song_t *head, string key) {
song_t *prev = NULL, curr = head;
/* Find song and node before that */
while(curr != NULL && strcasecmp(curr->title, key) != 0) {
prev = curr;
curr = curr->next;
}
if (curr == NULL) {
/* Not found */
newline;
printf("Song not found.");
newline;
newline;
} else if (prev == NULL) {
/* It's the very first song in the list
* so let head point to its successor
* and free the song; it doesn't matter
* if it's the last in the list
*/
head = curr->next;
free(curr->title); // Probably, but not if title isn't malloced
free(curr->artist); // Ditto
free(curr->album);
free(curr->genre);
free(curr);
} else {
/* We have a predecessor, let that point
* to the successor and free the song
*/
prev->next = curr->next;
free(curr->title); // See above
free(curr->artist);
free(curr->album);
free(curr->genre);
free(curr);
}
return head;
}
另一方面,您的RemoveDuplicate
函數不會做你想要的。除了複製件緊跟在原件之後的情況和沒有複製件的情況之間的區別之外,我可以看到沒有任何理由,作業temp = DeleteSong(temp,temp2->title);
或相應的作業。 temp2 = DeleteSong(temp2->next,temp2->title);
改變什麼temp
resp。 temp2
指向,但不是列表中相應的前任指向的地方。讓我們舉例說明一個小ASCII藝術的問題:
temp temp2
| |
v v
song1->song2->song3->song4->song5->...
其中song2
和song3
是重複的。現在temp = DeleteSong(temp,temp2->title);
,因爲兩首歌是重複的DeleteSong
的head
說法已經匹配,並在你的DeleteSong
版本,head = temp->next; return head;
是所有做的,所以名單都沒變,你只會得到
temp temp2
| |
v v
song1->song2->song3->song4->...
兩個指向同一個列表節點的指針。在釋放節點的DeleteSong
版本中,song1.next
現在將指向free
d內存,ouch。
如果重複不立即按照原來的,temp2 = DeleteSong(temp2->next,temp2->title);
可能無法找到匹配的標題歌曲,因爲搜索已知的匹配,在這種情況下,列表不會被修改,在所有後開始和temp2
是隻是改變爲指向繼任者。如果在此之後有歌曲匹配標題,則發現的副本仍未從列表中刪除,並且之後的部分可能會更改,可能會導致狀態不正確。如果temp2
指向列表中的倒數第二個節點,並且最後一個節點具有匹配的標題,則最後一個節點爲free
d,但next
指針的重複內容未更改,因此現在指向free
d內存,你有一個懸掛指針(這是一個段等待發生)。
的另一個問題是,在DeleteSong
和RemoveDuplicate
去除的標準是不同的,前者,你只檢查稱號,後者也藝術家,專輯和流派,所以在使用前函數在後者的風險刪除不應該刪除的歌曲(考慮封面版本)。
當你想從一個單獨的鏈表中刪除一個節點時,你需要一個指向前一個節點的指針來改變那些指向的內容,否則你將創建懸掛指針。我已經給了一個標準的方法,那上面做,你基本上可以複製到內循環,但在這裏它可以永遠不會發生,我們要刪除的第一個節點,所以這是一個有點簡單:
// void, since head is never changed, only duplicates after the original are removed
void RemoveDuplicates(song_t *head) {
song_t *orig = head, prev, curr;
while(orig != NULL && orig->next != NULL) {
prev = orig;
curr = orig->next;
while(curr != NULL) {
// Find next song to remove
while(curr != NULL && !meets_deletion_criteria(orig, curr)) {
prev = curr;
curr = curr->next;
}
// Now either curr is NULL, or it shall be deleted
if (curr != NULL) {
// Let the predecessor point to curr's successor
prev->next = curr->next;
clean_up(curr); // free all malloced members and the node itself
curr = prev->next;
}
}
orig = orig->next;
}
}
在else聲明我不明白你爲什麼要傳遞temp2-> next作爲頭指針。 並且在'temp = temp-> next'(內部while之後)不應該放置'temp2 = temp-> next;'? –