I'm trying to create my own linked list for handling bullets in my game. When the bullet has gone too far I want it to be removed but as soon as this is happening (when the bullet has travelled 11 steps) the game crashes.

These are the functions used when it crashes:

SDL_Surface* BulletList::display()      //Creates a BMP of bullet on screen
{
    if(bullet_sprite==NULL)
        std::cout<<"bullet_sprite är tom"<<std::endl;
    SDL_Surface* image = NULL;                              //Det vi ska returnera när alla kulor är tryckta
    if(head != NULL){
    node* current;                                 
    current = (*head).next;
    std::cout<<"head är inte NULL"<<(*current).x<<" Y: "<<(*current).y<<std::endl;  //This is where the segmentation fault appears
    while(current != NULL)
    {
        apply_surface((*current).x, (*current).y, bullet_sprite, image);
        current = (*current).next;
    }
    }
    return image;
}
//---------

void BulletList::move(int distance, int max)        //Hur långt och vad är max tillåtna, troligen vill man flytta alla kulor av samma sort lika mycket samtidigt
{
    node* current;
    node* previous;
    std::cout<<"Mellan tid:"<<std::endl;

    if(head==NULL)      //Inget behövde göras
        return;
    previous  = head;
    current = (*head).next;       //Förutsätter att head inte innehåller något

    std::cout<<(*current).travled;
    std::cout<<"Mellan tid: 2"<<std::endl;

    while(current!=NULL)        //Kanske löser *current.next == NULL
    {
        //std::cout<<(*current).travled;
        if((*current).travled<=max)     //Faktiskt ett plus max(dvs 10 ger 11 steg)
        {
            (*current).x+=distance;
            (*current).travled+=(distance<0? (-1)*distance:distance);       //Absoluta vaärdet av distance så att kulan kan färdas baklänges.
            std::cout<<"Felet måste ligga här"<<std::endl;
        }
        else
        {
            std::cout<<"Travled är: "<<(*current).travled<<" och ska därför bort"<<std::endl;
            remove(current,previous);
            std::cout<<"Den är nu borta"<<std::endl;
        }
        previous = current;
        current = (*current).next;
        std::cout<<"Ny current";
    }
}
//-----------

void BulletList::remove(node* remove, node* rebind) //Head.next, head
{
    std::cout<<"Allt är väl 3"<<std::endl;
    if((*remove).next==NULL)
        std::cout<<"remove.next var null"<<std::endl;
    (*rebind).next=(*remove).next;
    if((*rebind).next==NULL)
        std::cout<<"Rebind.next är null"<<std::endl;
    std::cout<<"Allt är väl"<<std::endl;
    delete remove;          //Struct:en är borta
    remove = NULL;      //Pekaren är tom.
}

So it crashes at the 7th line in the display() function. Notice how head can't be NULL because of the if statement in that function, even though head should be the only thing left after what it pointed to was removed by the remove function.

This is how it called in main()

if(event.key.keysym.sym==SDLK_SPACE)
       player_bullets.insert(player1.getx(),player1.gety());
      ....
       player_bullets.move(1,10);
      ....
      apply_surface(0,0,player_bullets.display(),screen);

And this is the header file and insert fuction

//Header file:
#ifndef BULLETLIST_H_INCLUDED
#define BULLETLIST_H_INCLUDED
#include <SDL/SDL.h>
#include <string>

struct node{
        int x;      //X-position
        int y;      //Y-position
        int travled;    //Hur långt har kulan färdats, är det dags att ta bort den?
        node* next; //Pekare till nästa element
};

class BulletList
{
    public:
        BulletList(std::string x);
        void insert(int posx, int posy);
        void move(int step, int max);
        SDL_Surface* display();
        void print();
        void removeNum(int num);
        static void remove(node* remove, node* rebind);
        ~BulletList();
    protected:
    private:
    node* head;
    SDL_Surface* bullet_sprite;

};

void BulletList::insert(int posx, int posy)
{
    node* current = new node;
    if(head==NULL)
    {
        head = new node;        //Head ska väl egentligen inte fyllas, lös detta med arv?

        (*head).next = current;     //Överväg att head inte kan ha y och x
        std::cout<<"Head next har något att peka på\n";
        //current = new node;
        (*current).x = posx;
        (*current).y = posy;
        (*current).travled=0;
        std::cout<<"Head ska få något att peka på\n";
        (*current).next = NULL;
    }
    else
    {
        node* previous;
        previous = (*head).next;            //Sista kan max vara den som head.next pekare på om head är fylld
        while((*previous).next !=NULL)          //Hitta sista elemtnet(det går inte att komma åt det snabbt
            previous = (*previous).next;
        //current = new node;                     //Skapar en ny node
        (*current).x = posx;
        (*current).y = posy;
        (*current).travled = 0;
        (*current).next = NULL;
        (*previous).next = current;                //Sista elemnetet pekar på det nya, skulle inte behövas om while(previous != NULL
    }
}

Would really appreciate any help!

Edited 5 Years Ago by Epicurus: n/a

The head node itself appears to be uninitialized, except for the next pointer. Try changing the insert() method so that if head == NULL its set to current instead of allocating another new node.

void BulletList::insert(int posx, int posy)
{
    node* current = new node;
   (*current).next = NULL;
    if(head==NULL)
    {
       head = current;        //Head ska väl egentligen inte fyllas, lös detta med arv?

        (*head).next = NULL;     //Överväg att head inte kan ha y och x
        std::cout<<"Head next har något att peka på\n";
        //current = new node;
        (*current).x = posx;
        (*current).y = posy;
        (*current).travled=0;
        std::cout<<"Head ska få något att peka på\n";
        (*current).next = NULL;
    }
    else
    {
        node* previous;
        previous = head;            //Sista kan max vara den som head.next pekare på om head är fylld
        while((*previous).next !=NULL)          //Hitta sista elemtnet(det går inte att komma åt det snabbt
            previous = (*previous).next;
        //current = new node;                     //Skapar en ny node
        (*current).x = posx;
        (*current).y = posy;
        (*current).travled = 0;
        (*current).next = NULL;
        (*previous).next = current;                //Sista elemnetet pekar på det nya, skulle inte behövas om while(previous != NULL
    }
}

Edited 5 Years Ago by Ancient Dragon: n/a

Comments
Identifies the problem and provides a solution. Great stuff!

You are correct the problem was with head not containing anything. This can be demonstrated if head is set to NULL in remove(). However I recall someone saying that the head in a list shouldn't contain anything but I don't see how this could be possible in practise. Head has to contain a .next but if it does it contains all an full featured node, right? Otherwise everything would be called head if .next was regular private variable.

I've set remove() to be non static until I fixed this head and can skip

if(head == rebind)
        head = NULL;

Scratch that I managed to do it with some major modifications. Thread solved

Comments
Thanks for letting us know the final results :)
This question has already been answered. Start a new discussion instead.