[newprogrammer57]

I recently got into programming with the Irrlicht graphics engine (v 1.7.3), and I was trying to create a FIFO queue to store messages that would then be displayed at the top and center of the window. I created a class called TQueue, that uses a template, as well as a templated struct Node. The class that uses it is MessageTracker, which stores the queue and calls the functions in it, as well as displaying the messages. The program compiles, but when I go to run it, I get a segmentation fault (core dumped) error right after one of the functions in the queue is called. I think that it is me using pointers wrong, but I can't figure it out. Here is my code for TQueue.h:

#ifndef TQUEUE_H_
#   define TQUEUE_H_
    template<class T> struct Node {
        T* data;
        Node<T>* next;
    };
    template<class T> class TQueue{
        private:
            Node<T>* mpFront;
            Node<T>* mpCur;
            int mSize;
        public:
            TQueue(){
                this->mpFront = NULL;
                this->mpCur   = NULL;
                this->mSize   = 0;
            }
            ~TQueue(){
                if(this->mpFront!=NULL){
                    delete [] this->mpFront;
                    this->mpFront = NULL;
                }
                if(this->mpCur != NULL){
                    delete [] this->mpCur;
                    this->mpCur = NULL;
                }
            }
            bool hasNext(){
                if(this->isEmpty())return false;
                return true;
            }
            T* next(){
                T empty
                if(this->isEmpty()||this->mpCur==NULL)return &empty;
                this->mpCur = this->mpCur->next;
                return this->mpCur->data;
            }
            void addNode(T* node){
                Node<T> tmp3;
                tmp3.data = node;
                tmp3.next = NULL;
                Node<T>* tmp2 = &tmp3;
                if(this->isEmpty()){
                    this->mpFront = tmp2;
                }else{
                    Node<T>* tmp = this->mpFront;
                    while(tmp->next!=NULL){
                        tmp = tmp->next;
                    }
                    tmp->next = tmp2;
                }
                this->mSize++;
                this->reset();
            }
            void addNodeAtFront(T* node){
                Node<T>* tmp;
                tmp->data = node;
                tmp->next = this->mpFront;
                this->mpFront = tmp;
                this->mSize++;
                this->reset();
            }
            T pop(){
                T empty;
                if(this->isEmpty())return empty;
                Node<T>* tmp = this->mpFront;
                Node<T> tmp2 = (*tmp);
                this->mpFront = this->mpFront->next;
                T* tDat = tmp2.data;
                this->mSize--;
                return (*tDat);
            }
            T getLast(){
                if(this->isEmpty())return NULL;
                Node<T>* tmp = this->mpFront;
                while(tmp->next!=NULL){
                    tmp = tmp->next;
                }
                Node<T> tmp2 = (*tmp);
                T* tDat = tmp2.data;
                return (*tDat);
            }
            void removeFirst(){
                if(this->isEmpty())return;
                this->mpFront = this->mpFront->next;
                this->reset();
                this->mSize--;
            }
            void removeLast(){
                if(this->isEmpty())return;
                Node<T>* tmp = this->mpFront;
                while(tmp->next->next!=NULL){
                    tmp = tmp->next;
                }
                tmp->next = NULL;
                this->reset();
                this->mSize--;
            }
            void removeAll(){
                this->mpFront = NULL;
                this->mpCur   = NULL;
                this->mSize   = 0;
            }
            void reset(){
                this->mpCur = this->mpFront;
            }
            bool isEmpty(){
                return (this->mpFront == NULL);
            }
            int size(){
                return this->mSize;
            }
    };
#endif

here is my code for MessageTracker.h:

#ifndef MESSAGE_TRACKER_H_
#   define MESSAGE_TRACKER_H_
#   include <irrlicht.h>
#   include <string>
#   include "GameManager.h"
#   include "TQueue.h"
    struct MessageNode {
        irr::u32 mStartTime;
        const wchar_t* mMsg;
        MessageNode():mStartTime(0),mMsg(L"empty_stringw_"){}
    };
    class MessageTracker {
        private:
            static irr::u32 sMaxDisplayTime;
            static irr::u32 sNumMsgDisplay;
            static irr::video::SColor sColor;
            irr::u32 mFontHeight;
            irr::core::dimension2d<irr::u32> mScreenSize;
            static irr::gui::IGUIFont* spFont;
            static MessageTracker* spMsgTrackInstance;
            static TQueue<MessageNode> mQueue;
        private:
            MessageTracker();
            MessageTracker(MessageTracker const&);
            ~MessageTracker();
        private:
            void RemoveOldMessages(irr::u32);
        public:
            static MessageTracker* Instance();
            void SetFont(irr::gui::IGUIFont*);
            void SetColor(irr::video::SColor);
            void AddMessage(irr::core::stringw);
            void DrawMessages();
    };
#endif

and here is my code for MessageTracker.cc:

#include <irrlicht.h>
#include "MessageTracker.h"
#include "GameManager.h"
#include "TQueue.h"

MessageTracker* MessageTracker::spMsgTrackInstance = NULL;
irr::gui::IGUIFont* MessageTracker::spFont         = NULL;
irr::video::SColor MessageTracker::sColor          = irr::video::SColor(255,255,255,255);
irr::u32 MessageTracker::sMaxDisplayTime           = 3000;
irr::u32 MessageTracker::sNumMsgDisplay            = 5;
TQueue<MessageNode> MessageTracker::mQueue                         = TQueue<MessageNode>();

MessageTracker* MessageTracker::Instance(){
    if(!spMsgTrackInstance){
        spMsgTrackInstance = new MessageTracker;
    }
    return spMsgTrackInstance;
}

MessageTracker::MessageTracker(){}
MessageTracker::MessageTracker(MessageTracker const&){}
MessageTracker::~MessageTracker(){}
void MessageTracker::RemoveOldMessages(irr::u32 curTimeMils){
    if(mQueue.isEmpty())return;
    mQueue.reset();
    do{
        MessageNode* tmp;
        tmp = mQueue.next();
        if(tmp->mStartTime==0)return;               // this hasn't been initialized yet
        if(tmp->mStartTime+sMaxDisplayTime<curTimeMils){
            mQueue.pop();
        }
        if(mQueue.isEmpty())return;
    }while(mQueue.hasNext());
}
void MessageTracker::SetFont(irr::gui::IGUIFont* font){
    spFont = font;
    if(!spFont)return;
    this->mFontHeight = spFont->getDimension(L"the quick brown fox jumps over the lazy dog. THE QUICK BROWN FOX JUMPS OVER THE LAZY DOG?").Height;
}
void MessageTracker::SetColor(irr::video::SColor mColor){
    sColor = mColor;
}
void MessageTracker::AddMessage(irr::core::stringw msg){
    MessageNode temp;
    temp.mMsg = msg.c_str();
    temp.mStartTime = 0;
    mQueue.addNode(&temp);
}
void MessageTracker::DrawMessages(){
    GameManager* mGMgr = GameManager::Instance();
    if(!mGMgr)return;
    irr::IrrlichtDevice* dev = mGMgr->pGetDevice();
    if(!dev)return;
    irr::u32 curTime = dev->getTimer()->getTime();
    this->RemoveOldMessages(curTime);
    mQueue.reset();
    if(mQueue.isEmpty()||!spFont)return;
    irr::u32 pos = 0;
    this->mScreenSize = dev->getVideoDriver()->getScreenSize();
    irr::f32 msgX = 0;
    for(;pos<sNumMsgDisplay&&mQueue.hasNext();pos++){
        const wchar_t* mMsg = mQueue.next()->mMsg;
        irr::f32 msgWidth = spFont->getDimension(mMsg).Width;
        msgX = (this->mScreenSize.Width / 2) - (msgWidth / 2);
        spFont->draw(mMsg, irr::core::rect<irr::s32>(msgX, pos*this->mFontHeight, msgX + msgWidth, (pos*this->mFontHeight)+this->mFontHeight), sColor);
        mGMgr->Sleep(500);
    }
}

this code references another class, GameManager, however, all of the functions in that class work properly and do as they're supposed to.

[Salem_c]

> void addNode(T* node){
> Node<T> tmp3;
This adds a pointer to a local variable (which will cease to exist when this function exits) to your list.

> void addNodeAtFront(T* node){
> Node<T>* tmp;
This uses an uninitialised pointer, then adds some random memory location to your list.

Both should be
Node<T>* tmp = new Node<T>;

Also, your destructor should not be using the [] form for delete, unless you use the [] form for new.

[newprogrammer57]

I made those changes, but I'm not sure what to change the destructor to. I've compiled it with no errors and no warnings, but I still get a segmentation fault.

[newprogrammer57]

Oh, and I'm using gcc 4.6, Ubuntu 12.04 and Code::blocks 10.05 if that helps any.

[Salem_c]

You need to stop focussing on "well it compiles" and start thinking about what it is you're actually writing.

C and C++ will quite happily let you write perfectly correct syntatic nonsense (it will compile), but the result is a complete pile of crap when it comes to running the code.

Here is another example of your illegal pointer use, found just by browsing the code.

    MessageNode temp;
    temp.mMsg = msg.c_str();
    temp.mStartTime = 0;

c_str() just returns a POINTER to some internal data. If that object is deleted, or goes out of scope, then temp.mMsg is broken.
Add another constructor to your MessageNode class, one that takes a c_str() type argument, and make a COPY of the string.


> Oh, and I'm using gcc 4.6, Ubuntu 12.04 and Code::blocks 10.05 if that helps any.
Yes, you should compile a debug version and then run the code in the debugger.
It will then point you at an exact line of code which is causing the segfault.

But this is only the first step. Having found out where it crashes, you then need to backtrack to find out what caused it to crash. A segfault by it's very nature is only the point where the system noticed that something was very wrong.

Here is a short example

char *p = new char[10];
strcpy(p,"this is too long"); // may or may not crash here
/// more code
delete [ ] p;  // may or may not crash here

The true cause in this example is not allocating enough space to begin with.
Particularly nasty bugs of this kind show up in completely unrelated areas of code, which make finding the real cause all that more complicated.


It's the first step towards understanding, which will be a lot better than posting a lot of code and saying "it doesn't work".

[newprogrammer57]

So I need to find every place that I have used a pointer, and make sure that it has enough memory allocated to it?

So would a call to malloc work for creating memory in each of those instances?

This is getting confusing, but I will understand eventually.

[jimblumberg]

Quote

So I need to find every place that I have used a pointer, and make sure that it has enough memory allocated to it?

Yes.

Quote

So would a call to malloc work for creating memory in each of those instances?

You seem to be compiling a C++ program so instead of malloc/free you should be thinking about new/delete.

Jim

[newprogrammer57]

Thank you all for your help, I solved the problem! Pointers make a lot more sense now