C++: Seg fault in doubly linked list remove function

时间:2022-09-05 19:51:14

There is a segmentation fault in the remove function of this doubly linked list. Debugging with gdb in the linux terminal shows that
1) Only the first condition, if(curr->prev == NULL), where the node to delete is the first node in the list is ever reached.
2) On the command delete curr, the program steps into the destructor for the class that models the data object, and breaks on the command delete title.

这个双向链表的删除功能存在分段错误。在linux终端中使用gdb进行调试显示1)只有第一个条件if(curr-> prev == NULL),其中要删除的节点是列表中的第一个节点。 2)在命令delete curr上,程序进入为数据对象建模的类的析构函数,并中断命令delete title。

Edit:

After implementing user6545984 answer, the only issue is a segementation fault on tail = tail->prev in the else if(curr->next == NULL) condition. The program is unable to get past this conditional statement.
Also, additional code has been added below. I apologize for the length.

在实现user6545984回答之后,唯一的问题是在else if(curr-> next == NULL)条件下tail = tail-> prev的segementation故障。该程序无法通过此条件声明。此外,下面还添加了其他代码。我为这个长度道歉。

Code for the add and remove functions, as well as the for the destructor:

添加和删​​除函数的代码,以及析构函数的代码:

Add Function (creates doubly linked list)

 void SongList::addSongSorted(const Song &aSong)
    {
        char title[MAX_CHAR];
        char currTitle[MAX_CHAR];
        aSong.getTitle(title);

        Node * newNode = new Node(aSong);
        Node * prev = NULL;
        Node * curr = head;

        while(curr)
        {
            curr->song.getTitle(currTitle);

            if(strcmp(title, currTitle) < 0)

                break;

            prev = curr;
            curr = curr->next;
        }
        newNode->next = curr;
        newNode->prev = prev;     //Added due to answer. See edit note.

        if(!prev)

            head = newNode;

        else
        {
            prev->next = newNode;
        }

        size++;
    }

Remove function

void SongList::remove(const Song& aSong)
{
    Node *curr = head;

    int indexRem = 0;
    int j = 0;
    //get the index to remove
    cout << "Enter the index to remove.\n";
    cout << "This value can be obtained via the list all   functionality.\n";
    cin >> indexRem;
    while(indexRem > getSize()|| indexRem < 0)
    {
        cout << "Enter a valid index to remove.\n";
        cin >> indexRem;
    }
    while(curr && j < indexRem)
    {
        curr=curr->next;
        j++;
    }

    if(!curr)
    {
        cout << "didnt find anything" << endl;
    }

    else
    {
       if(curr->prev == NULL)      //NOTE: this is the only condition ever engaged
       {

        head = head->next;
        head->prev = NULL;
        delete curr;            //NOTE: goes to destructor
        curr = NULL;
        }
        else if(curr->next == NULL)
        {
            tail = tail->prev;      //EDIT: only issue is seg fault here
            tail->next = NULL;
            delete curr;
            curr = NULL;
        }
        else
        {
            Node * previous = curr->prev;
            Node * following = curr->next;
            previous->next = following;
            following->prev = previous;
            delete curr;

        }
    }

Destructor (please now ignore)

    Song::~Song()
        {
            if(title != NULL)
                delete [] title;    //Breaks here
            if(artist != NULL)
                delete [] artist;
            if(album != NULL)
                delete [] album;
        }

Header file for object class

#ifndef SONG_H
#define SONG_H
#include<cstring>
#include<stdio.h>
#include<cstdlib>
const int MAX_CHAR = 101;
const int SONG_CAPACITY = 100;

class Song
{
public:
    //constructors
    Song();
    Song(const char title[], const char artist[], const char album[], int min, int sec);

    //Destructor
    ~Song();

    //accessor functions
    void getTitle(char title[]) const;
    void getArtist(char artist[]) const;
    void getAlbum(char album[]) const;
    int getMin()const;
    int getSec()const;
    void print() const;

    //mutator functions
    void setTitle(const char title[]);
    void setArtist(const char artist[]);
    void setAlbum(const char album[]);
    void setMin(int &min);
    void setSec(int &sec);


private:
    char*    title;
    char*    artist;
    char*    album;
    int     min;
    int     sec;
};


#endif

Source file for object class

    #include "song.h"
    #include <iostream>
    using namespace std;


    Song::Song()
    {
        title = new char[strlen("no title")+1];
        strcpy(title, "no title");
        artist = new char[strlen("no artist")+1];
        strcpy(artist, "no artist");
        album = new char[strlen("no album")+1];
        strcpy(album, "no album");
        min = 0;
        sec = 0;
    }


    Song::Song(const char title[], const char artist[], const char album[], int min, int sec)
    {
        this->title = new char[strlen(title)+1];
        strcpy(this->title, title);
        this->artist = new char[strlen(artist)+1];
        strcpy(this->artist, artist);
        this->album = new char[strlen(album)+1];
        strcpy(this->album, album);
        this->min = min;
        this->sec = sec;
    }

    Song::~Song()
    {
        if(title != NULL)
            delete [] title;
        if(artist != NULL)
            delete [] artist;
        if(album != NULL)
            delete [] album;
    }

    void Song::getTitle(char title[]) const
    {
        strcpy(title, this->title);
    }

    void Song::getArtist(char artist[]) const
    {
        strcpy(artist, this->artist);
    }

    void Song::getAlbum(char album[]) const
    {
        strcpy(album, this->album);
    }

    int Song::getMin()const
    {
        return min;
    }

    int Song::getSec()const
    {
        return sec;
    }


    void Song::print() const
    {
       cout << title << '\t'
            << artist << '\t'
            << album << '\t'
            << min << '\t'
            << sec << endl;
    }

    void Song::setTitle(const char title[])
    {
        if(this->title != NULL)
            delete [] this->title;
        this->title = new char[strlen(title)+1];
        strcpy(this->title, title);
    }

    void Song::setArtist(const char artist[])
    {
        if(this->artist != NULL)
            delete [] this->artist;
        this->artist = new char[strlen(artist)+1];
        strcpy(this->artist, artist);
    }

    void Song::setAlbum(const char album[])
    {
        if(this->album != NULL)
            delete [] this->album;
        this->album = new char[strlen(album)+1];
        strcpy(this->album, album);
    }

    void Song::setMin(int &min)
    {
        this->min = min;
    }

    void Song::setSec(int &sec)
    {
        this->sec = sec;
    }

###Header file for object management class

    #ifndef SONG_LIST
    #define SONG_LIST
    #include "song.h"
    #include <iostream>
    using namespace std;

    class SongList
    {
    public:
        SongList();
        SongList(const char fileName[]);

        ~SongList();
        bool get(int index, Song& aSong) const;
        //Search function declaration
        void searchArtist(const char artist[], Song& match) const;
        void searchAlbum(const char album[], Song& match) const;
        int getSize() const;
        void printAll() const;
        void saveToFile(const char fileName[]) const;

        void addSong(const Song& aSong);
       // void addAtStart(const Song& aSong);
       // void append(const Song& aSong);
        void addSongSorted(const Song& aSong);
        void loadFromFile(const char fileName[]);
        void remove(const Song& aSong);

    private: 
        struct Node
        {
            Song    song;
            Node *  next;
            Node *  prev;

            Node(const Song& aSong)
            {
                char    title[MAX_CHAR];
                char    artist[MAX_CHAR];
                char    album[MAX_CHAR];
                int     min;
                int     sec;

                aSong.getTitle(title);
                aSong.getArtist(artist);
                aSong.getAlbum(album);
                min =  aSong.getMin();
                sec =  aSong.getSec();

                song.setTitle(title);
                song.setArtist(artist);
                song.setAlbum(album);
                song.setMin(min);
                song.setSec(sec);
                next = NULL;
            }
        };

        Node * head;
        Node * tail;
        int size;
    };

    #endif

###Source file for object management class
#include "songlist.h"
#include <iostream>
#include <fstream>
#include <iomanip>
#include <cstring>

using namespace std;

//Ok as is
SongList::SongList()
{
    head = NULL; 
    tail = NULL;   
    size = 0;
}

//OK as is
SongList::SongList(const char fileName[])
{
    head = NULL;
    tail = NULL;
    size = 0;
    loadFromFile(fileName);
}

//Needs modification to deconstruct tail...
SongList::~SongList()
{
    Node * curr = head;

    while(head != NULL)
    {
        curr = head->next;
        delete head;
        head = curr;
    }
}
//OK as is
void SongList::loadFromFile(const char fileName[])
{
    ifstream        in;
    char            title[MAX_CHAR];
    char            artist[MAX_CHAR];
    char            album[MAX_CHAR];
    int             min;
    int             sec;
    Song    aSong;

    in.open (fileName);
    if(!in)
    {
        in.clear();
        cerr << endl << "Fail to open " 
        << fileName << " for input!" << endl << endl;
        exit(1);
    }
    //cout << "loading point reached" << endl;
    in.getline(title, MAX_CHAR, ';');
    while (!in.eof())
    {
        //cout << "check input from file" << endl;
        in.getline(artist, MAX_CHAR, ';');
        in.getline(album, MAX_CHAR, ';');
        in >> min;
        in.ignore(MAX_CHAR, ';');
        in >> sec;
        in.ignore(MAX_CHAR, '\n');

        aSong.setTitle(title);
        aSong.setArtist(artist);
        aSong.setAlbum(album);
        aSong.setMin(min);
        aSong.setSec(sec);
        addSong(aSong);
     // aSong.print();

        in.getline(title, MAX_CHAR, ';');
    }
    in.close();
}

//Ok as is
int SongList::getSize()const
{
    return size;
}

bool SongList::get(int index, Song &aSong) const
{
    char    title[MAX_CHAR];
    char    artist[MAX_CHAR];
    char    album[MAX_CHAR];
    int     min;
    int     sec;

    if(index < 0 || index >= size)
        return false;

    int i;
    Node * curr = head;
    for(i = 0; i < index; i++)
    {
        curr = curr->next;
    }

    curr->song.getTitle(title);
    curr->song.getArtist(artist);
    curr->song.getAlbum(album);
    curr->song.getMin();
    curr->song.getSec();
    aSong.setTitle(title);
    aSong.setArtist(artist);
    aSong.setAlbum(album);
    aSong.setMin(min);
    aSong.setSec(sec);
    return true;
}
//OK as is 
void SongList::searchArtist(const char artist[], Song& match) const
{
    Node * curr;
    char    currentTitle[MAX_CHAR];
    char    currentArtist[MAX_CHAR];
    char    currentAlbum[MAX_CHAR];
    int     currentMin;
    int     currentSec;

    for(curr = head; curr != NULL; curr = curr->next)
    {
        curr->song.getTitle(currentTitle);
        curr->song.getArtist(currentArtist);
        curr->song.getAlbum(currentAlbum);
        curr->song.getMin();
        curr->song.getSec();
        if(strcmp(artist, currentArtist) == 0)
        {
            cout << "Found: " << currentTitle << '\t' 
                              << currentArtist << '\t'
                              << currentAlbum << '\t'
                              << currentMin << ':' 
                              << currentSec << endl;
        }
    }
}

//Ok as is
void SongList::searchAlbum(const char album[], Song &match) const
{
    Node * curr;
    char    currentTitle[MAX_CHAR];
    char    currentArtist[MAX_CHAR];
    char    currentAlbum[MAX_CHAR];
    int     currentMin;
    int     currentSec;

    for(curr = head; curr != NULL; curr = curr->next)
    {
        curr->song.getTitle(currentTitle);
        curr->song.getArtist(currentArtist);
        curr->song.getAlbum(currentAlbum);
        curr->song.getMin();
        curr->song.getSec();
        if(strcmp(album, currentAlbum) == 0)
        {
            cout << "Found: " << currentTitle << '\t'
                              << currentArtist << '\t'
                              << currentAlbum << '\t'
                              << currentMin << ':'
                              << currentSec << endl;
        }
    }
}
//OK as is 
void SongList::printAll() const
{
    Node * curr;

    char    title[MAX_CHAR];
    char    artist[MAX_CHAR];
    char    album[MAX_CHAR];
    int     min;
    int     sec;
    int     index = 0;
    for(curr = head; curr != NULL; curr = curr->next)
    {

        curr->song.getTitle(title);
        curr->song.getArtist(artist);
        curr->song.getAlbum(album);
        min = curr->song.getMin();
        sec = curr->song.getSec();

        cout << index << " " << title << " "  
             << artist << " " << album << " " 
             << min << ':' << sec << endl;
        index++;
    }
}
//OK as is 
void SongList::saveToFile(const char fileName[])const
{
    ofstream    out;
    Node*       curr;
    char        title[MAX_CHAR];
    char        artist[MAX_CHAR];
    char        album[MAX_CHAR];
    int         min = 0;
    int         sec = 0;

    out.open(fileName);
    if(!out)
    {
        out.clear();
        cerr << endl << "Fail to open" << fileName << ". Check your directory." << endl << endl;
        exit(1);
    }

    for(curr = head; curr; curr = curr->next)
    {
        curr->song.getTitle(title);
        curr->song.getArtist(artist);
        curr->song.getAlbum(album);
        min = curr->song.getMin();
        sec = curr->song.getSec();
        out << title << ';' << artist << ';' << album << ';' << min << ';' << sec << endl;
    }

    out.close();
}

void SongList::addSong(const Song &aSong)
{
    addSongSorted(aSong);
}   
//Don't think I need this
/*
void SongList::addAtStart(const Song &aSong)
{
    Node * newNode = new Node(aSong);

    newNode->next = head;
    head = newNode;
    size++;
}
//...or this
void SongList::append(const Song &aSong)
{
    Node * newNode = new Node(aSong);

    if(!head)
        head = newNode;
    else
    {
        Node * curr = head;

        while(curr->next)
        {
            curr = curr->next;
        }

        curr->next = newNode;
    }
    size++;
}

*/

void SongList::addSongSorted(const Song &aSong)
{

    char title[MAX_CHAR];
    char currTitle[MAX_CHAR];
    aSong.getTitle(title);

    Node * newNode = new Node(aSong);

    Node * prev = NULL;
    Node * curr = head;

    while(curr)
    {
        curr->song.getTitle(currTitle);

        if(strcmp(title, currTitle) < 0)

            break;


        prev = curr;
        curr = curr->next;

    }
    newNode->next = curr;
    newNode->prev = prev;

    if(!prev)

        head = newNode;

    else
    {
        prev->next = newNode;
    }

   // printAll();

    size++;

}



void SongList::remove(const Song& aSong)
{
    Node *curr = head;

    int indexRem = 0;
    int j = 0;

    cout << "Enter the index to remove.\n";
    cout << "This value can be obtained via the list all functionality.\n";
    cin >> indexRem;
    while(indexRem > getSize()|| indexRem < 0)
    {
        cout << "Enter a valid index to remove.\n";
        cin >> indexRem;
    }
    while(curr && j < indexRem)
    {
        curr=curr->next;
        j++;
    }

    if(!curr)
    {
        cout << "didnt find anything" << endl;
    }

    else 
    {
       if(curr->prev == NULL)
       { 

        head = head->next;

        head->prev = NULL;
        delete curr;
        curr = NULL;
        }
        else if(curr->next == NULL)
        {
            tail = tail->prev;
            tail->next = NULL;
            delete curr;
            curr = NULL;
        }
        else
        {
            Node * previous = curr->prev;
            Node * following = curr->next;
            previous->next = following;
            following->prev = previous;
            delete curr;

        }
    }

    size--;
}

Any suggestions?

有什么建议么?

2 个解决方案

#1


2  

Your forget the ->prev pointer when add a new node, which makes if(curr->prev == NULL) always be true. Just add newNode->prev = prev; in function addSongSorted().

在添加新节点时忘记 - > prev指针,这使if(curr-> prev == NULL)始终为true。只需添加newNode-> prev = prev;在函数addSongSorted()中。

By the way, you may be want --size when remove a node.

顺便说一下,删除节点时可能需要--size。

#2


-1  

It gives you an error because you should never use delete or delete[] without using new or new[]. New gives you some memory from heap during runtime, while char[size] gives you some memory during compilation, so the size of the variable is hard-coded in your code

它会给你一个错误,因为你不应该使用删除或删除[]而不使用new或new []。 New在运行时为堆提供了一些内存,而char [size]在编译期间为你提供了一些内存,因此变量的大小在代码中是硬编码的

#1


2  

Your forget the ->prev pointer when add a new node, which makes if(curr->prev == NULL) always be true. Just add newNode->prev = prev; in function addSongSorted().

在添加新节点时忘记 - > prev指针,这使if(curr-> prev == NULL)始终为true。只需添加newNode-> prev = prev;在函数addSongSorted()中。

By the way, you may be want --size when remove a node.

顺便说一下,删除节点时可能需要--size。

#2


-1  

It gives you an error because you should never use delete or delete[] without using new or new[]. New gives you some memory from heap during runtime, while char[size] gives you some memory during compilation, so the size of the variable is hard-coded in your code

它会给你一个错误,因为你不应该使用删除或删除[]而不使用new或new []。 New在运行时为堆提供了一些内存,而char [size]在编译期间为你提供了一些内存,因此变量的大小在代码中是硬编码的