Sign in to follow this  
Endar

Debug mode error (VC++ 6)

Recommended Posts

I know that everyone always has problems with debug mode working and release mode not, because they don't init their variables/memory before using, but I'm having a problem with debug mode not working and release mode working, so I'm assuming that its the same kind of thing. I have an iterator for a linked list and in the operator++ function, I've got something like this: Note: current is a Node*

void operator++(int j)	
{	
	if( this->current == NULL )
		return;
	else
		this->current = this->current->next;
}


// with a node in the list being:

/**
 * A node in a doubly linked list.
 */
struct Node{
	T obj;			///< An object of type T
	Node* next;		///< A pointer to the next node
	Node* prev;		///< A pointer to the previous node

	/**
	 * Constructor
	 * \param o A const reference to an object of type T
	 * \param n A pointer to the next node in the list
	 * \param p A pointer to the previous node in the list
	 */
	Node(const T& o, Node* n = NULL, Node* p = NULL)
		:	obj(o), next(n), prev(p)
	{	}
};



And I'm finding, that at some point, 'current' becomes 0xdddddddd, and then when it tries to ++ another time, it obviously fails. Has anyone come across something like this? Because I'm sure that I'm initing everything correctly, and, I've been using this code for a while (since the start of september), and I haven't had a single problem with it (after of course all the initial debugging).

Share this post


Link to post
Share on other sites
Quote:
Original post by Endar
I know that everyone always has problems with debug mode working and release mode not, because they don't init their variables/memory before using, but I'm having a problem with debug mode not working and release mode working, so I'm assuming that its the same kind of thing.

In debug mode, you WANT your program to crash. In fact, the runtime does a lot of things behind the scenes to make it more likely that your program will crash. Why? So that you're more likely to notice bugs that might otherwise go unnoticed until after you'd shipped your final product. In this particular case, it looks like you're running into the fact that the runtime puts garbage in newly allocated memory as well as deallocated memory. You weren't noticing it in release mode because the data which happened to be there happened to work.

Share this post


Link to post
Share on other sites
I realise that, but is there an instance in which the constructor for the Node object I have above will not assign NULL to the 'next' and 'prev' pointers (considering I only use the 'const T&' param)?

I can't think of one, so, would it really matter if garbage was put there when the mem was deallocated, or for newly allocated memory? Because I believe that the constructor is called on that piece of memory right after it has been allocated (is that right?). So, as far as I can tell, the two pointers are assigned NULL, and it's not as if I can check if 'pointer != NULL' when the pointer has a garbage address, is it?

Or am I still missing something? (well, I obviously am since my code doens't work, but, is it incredibly glaring?)

[Edited by - Endar on October 22, 2005 7:49:45 AM]

Share this post


Link to post
Share on other sites
I've had problems like this in the past myself. For me, the culprit was always in the redefinition of the NULL value. If you redefine your own NULL value - as I do from time to time - check to make sure the definition is occurring for your code.

I redefine NULL (and other values) in order to keep from #including a slew of useless headers that Windows inadvertantly adds to the program.

Regards,
Jumpster

Share this post


Link to post
Share on other sites
Just to point out that if you'd been using the SC++L list class template instead of rolling your own you could have been confident from the very start that the problem was in the code that used the list and not left wondering whether it might in fact be a subtle error in your list class that you hadn't stumbled across until now.

If you're able to in your debugger try setting a watch on current and see where it changes.

Enigma

Share this post


Link to post
Share on other sites
Here's the code that I was wokring on:




#ifndef _IMBASE_H_
#define _IMBASE_H_

#include "list.h"

/**
* Provides the basic menmory managment for objects.
*/

class IMBase
{
private:

static util::list<IMBase*> activeList; ///< The list of active objects (with ref_count > 0)
static util::list<IMBase*> deadList; ///< The list of dead objects (with ref_count == 0)

long ref_count; ///< Reference variable

public:

/**
* Default constructor
* Sets ref variable to 1
*/

IMBase()
: ref_count(1)
{
activeList.push_back(this); // add this object to the live list
}

/**
* Virtual destructor so destructors of the derived classes will be called.
*/

virtual ~IMBase()
{

}

/**
* Called by when a function or an object is using this object.
*/

void get()
{
ref_count++; // add another thing using this object
}

/**
* Called by a function or an object when it is finished using this object.
*/

void drop()
{
ref_count--; // an object is done using this object

if( ref_count <= 0 )
IMBase::makeDead(this);
}

/**
* Search for the passed pointer in the activeList and if found, delete
* it from the list, and insert it into the deadList.
* \param p A pointer of type IMBase*, which is the pointer to delete from activeList
* and insert into the deadList.
*/

static void makeDead(IMBase* p)
{
util::list<IMBase*>::iterator j = activeList.begin();;
for( ; j != activeList.end(); j++)
(*j);


util::list<IMBase*>::iterator i = activeList.begin();
for( ; i != activeList.end(); i++ )
if( (*i) == p ){
activeList.erase(i); // delete element i from the active list
deadList.push_back(p);
return; // object adds itself, so there will never be more than
// 1 copy of it's pointer in the list
}
}

/**
* Deletes all the items in the deadList from memory.
*/

static void collectGarbage()
{
util::list<IMBase*>::iterator i = deadList.begin();
for( ; i != deadList.end(); i++ )
delete (*i);

}

/**
* Delete all the items in the deadList and activeList.
*/

static void collectAllGarbage()
{
collectGarbage(); // delete all in deadList

// delete all items in the activeList
util::list<IMBase*>::iterator i = activeList.begin();
for( ; i != activeList.end(); i++ )
delete *i;

}

};

#endif





The problem was in the 'makeDead' function, where the equality test between '(*i)' and 'p' was. I added the return inside the if statement because once it has removed itself from one list and added itself to the next, then it's done, it shouldn't keep going through the whole list, because apart from being time-wasting, it's useless.

The constructor for IMBase adds the object to the activeList, and the 'drop' function adds the object to the deadList. So, because the constructor adds the object, each object should never be present in either list more than once.

So, what's the point of traversing the rest of the list?

Share this post


Link to post
Share on other sites
[QUOTE][CODE]
static void makeDead(IMBase* p)
{
util::list<IMBase*>::iterator j = activeList.begin();;
for( ; j != activeList.end(); j++)
(*j);

util::list<IMBase*>::iterator i = activeList.begin();
for( ; i != activeList.end(); i++ )
if( (*i) == p )
{
activeList.erase(i); // delete element i from the active list
deadList.push_back(p);
return; // object adds itself, so there will never be more than
// 1 copy of it's pointer in the list
}

}
[/CODE][/QUOTE]

Ok. This being the code of makeDead(), I can see a few problems here...

First: What's the point of the first 'for(;j!=activeList.end();j++)' loop? All you're doing is cycling through them until j = activeList.end(). What a waste of precious time you got going there - especially if you have A LOT of elements in your list<>... Of course, any decent optimizing compiler will eliminate that loop in release build - since you're not using j anywhere else within the function - but still, continuous improvement says you could do better... :)


Really though, the only thing I see is that the list<> object is in the util:: namespace. Is this a class you developed; or copied from a book? I ask because the STL list<> wouldn't have a problem with the code you display (except *maybe* in multi-thread stuff - which I doubt is a problem here).

Try using the STL std::list<> class. If you're already doing so, you have to let us know - it seems like you're using a non-standard list<> class, which all of us others don't seem to have a clue about...

I will help you with your problem, personally, if you contact me via email. My email is Jump@fourman.com. Contact me there if you feel the need.

Regards,
Jumpster

Share this post


Link to post
Share on other sites
Quote:

First: What's the point of the first 'for(;j!=activeList.end();j++)' loop? All you're doing is cycling through them until j = activeList.end(). What a waste of precious time you got going there - especially if you have A LOT of elements in your list<>... Of course, any decent optimizing compiler will eliminate that loop in release build - since you're not using j anywhere else within the function - but still, continuous improvement says you could do better... :)


Yeah, sorry, that wasn't stupid code, that was stupid me. I wanted to examine the iterators before the loop with the equality test, just in case something wierd was happening in that loop. I know that it probably wasn't, and this was just me being paranoid, but it was basically some debug code that I added this morning and didn't rip out before posting.

And yes, I'm not using the STL list, although when just adding, deleting and accessing elements, I've been able use them interchangably. And no, there's no particular reason that I haven't used the STL. I probably will for future projects, but I started this project (simple graphics engine) by doing this list and other small things, and since then this is actually the only problem I've had with the list.


#ifndef _UTIL_LIST_H_
#define _UTIL_LIST_H_

#include <stddef.h>

namespace util{


/**
* A templated doubly linked list class
*/

template <class T>
class list
{
private:

/**
* A node in a doubly linked list.
*/

struct Node{
T obj; ///< An object of type T
Node* next; ///< A pointer to the next node
Node* prev; ///< A pointer to the previous node

/**
* Constructor
* \param o A const reference to an object of type T
* \param n A pointer to the next node in the list
* \param p A pointer to the previous node in the list
*/

Node(const T& o, Node* n = NULL, Node* p = NULL)
: obj(o), next(n), prev(p)
{ }
};


Node* head; ///< The first linked list element in the list
unsigned int size; ///< The number of elements in the list


public:

/**
* An iterator that is used to step through the linked list
*/

struct iterator{
private:
friend class list<T>; ///< Declare this a friend to list<T> so, list<T> can access its members
Node* current; ///< The current list element being pointed to

public:
/**
* Constructor
* \param n A pointer to an element in the list
*/

iterator(Node* n = NULL)
: current(n)
{ }

/**
* Overloaded operator* (for de-referencing)
* \return A reference of type T to the object element of the
*/

T& operator*() { return current->obj; }

/**
* Overloaded prefix operator++
* Moves the iterator forward in the list
*/

void operator++(int j)
{
if( current == NULL )
return;
else
current = current->next;
}

/**
* Overloaded prefix operator--
* Moves the iterator backward in the list
*/

void operator--(int j)
{
if( current == NULL )
return;
else
current = current->prev;
}

/**
* Overloaded operator==
* \param i A const reference to an iterator object
* \return true if current pointer is equal to i.current pointer, else false
*/

bool operator==(const iterator& i) { return current == i.current; }

/**
* Overloaded operator!=
* \param i A const reference to an iterator object
* \return true if current pointer is not equal to i.current pointer, else false
*/

bool operator!=(const iterator& i) { return current != i.current; }

/**
* Overloaded assignment operator
* \param i A const refernce to an iterator object
* \return A reference to 'this' iterator object
*/

iterator& operator=(const iterator& i)
{
if( &i != this )
current = i.current;

return *this;
}

}; // iterator


/**
* Constructor
*/

list()
: head(NULL), size(0)
{ }

/**
* Copy constructor
* \param l A const reference to a list object
*/

list(const list& l)
{
iterator i = l.begin(); // iterator for list l

for( ; i != l.end(); i++ )
push_back(*i); // add element int list l to 'this' list
}

/**
* Destructor
*/

~list()
{
clear(); // erase all elements in the list
}

/**
* Return the number of elements in the list.
* \return The number of elements in the list.
*/

unsigned int getSize() const
{
return size;
}

/**
* Delete all elements in the list.
*/

void clear()
{
while( begin() != end() )
erase( begin() ); // erase the first element

size = 0; // set size of list to 0
}

/**
* Return an iterator to the first element in the list
* \return An iterator object to the first element in the list.
*/

iterator begin() const
{
return iterator(head);
}

/**
* Return an iterator to the point after the last element in the list,
* which effectively is a NULL pointer.
*/

iterator end() const
{
return iterator(NULL);
}

/**
* Add an element to the back of the list.
* \param o A const reference to an object of type T
*/

void push_back(const T& o)
{
iterator i = begin();
iterator j;

if( i.current == NULL ){
head = new Node(o);
size++;
return;
}

for( ; i != end(); i++)
j = i;

// if size > 1, then at this point in the function
// i == i-1, i == NULL

j.current->next = new Node(o);
(j.current->next)->prev = j.current; // element (j+1)->prev = &j
size++;
}// push_back

/**
* Add a list element to the front of the list
* \param o A const reference to an object of type T
*/

void push_front(const T& o)
{
iterator i = begin();

if( head == NULL ){
head = new Node(o);
size++;
return;
}

head = new Node(o);
head->next = i.current; // head->next is equal to address of previous first element
i.current->prev = head; // previous first element->prev pointer is equal to head
size++;
}// push_front

/**
* Insert an item in the list before the item pointed to by the specified iterator.
* \param i An iterator pointing to the item to insert before
* \param o The item to insert before iterator i
*/

void insertBefore(iterator& i, const T& o)
{
Node* n;
iterator j(i);

// if i == head
if( i.current->prev == NULL ){
push_front(o);
return;
}

j--; // move iterator j back 1 position

n = new Node(o); // create a new list element

j.current->next = n; // New list element pointed to by 'n' is now after element pointed to by
i.current->prev = n; // iterator j and before element pointed to by iterator i.

n->next = i.current; // Element pointed to by iterator i is now after 'n'
n->prev = j.current; // and element pointed to by iterator j is now before 'n'

size++; // increase number of elements in list
}

/**
* Insert an item in the list after the item pointed to by the specified iterator.
* \param i An iterator pointing to the item to insert after
* \param o The item to insert after iterator i
*/

void insertAfter(iterator& i, const T& o)
{
Node* n;
iterator j(i);

// if i == last element in list
if( i.current->next == NULL ){
push_back(o);
return;
}

j++; // move iterator j forward 1 position

n = new Node(o); // create a new list element

i.current->next = n; // New list element pointed to by 'n' is now after element pointed to by
j.current->prev = n; // iterator i and before element pointed to by iterator j.

n->next = j.current; // Element pointed to by iterator j is now after 'n'
n->prev = i.current; // and element pointed to by iterator i is now before 'n'

size++; // increase number of elements in list
}

/**
* Erase a list element.
* \param pos The position of the element to erase
*/

void erase(unsigned int pos)
{
iterator j(head);

if( pos > size - 1) // if position is out of bounds
return;

for(int i=0; i < pos; i++, j++);

erase(j);
}

/**
* Erase the element at this iterator.
* \param i A const reference to an iterator object
*/

void erase(iterator& i)
{
iterator j;

if( i.current == NULL )
return;

if( i.current == head && head != NULL ){
i++;
delete head;
head = i.current;
size--;
return;
}

j = i.current->prev;
j.current->next = i.current->next;
i.current->prev = j.current;
size--;
delete i.current;
}// erase

};

} // namespace util

#endif




But, the 'return' statement looks like it's fixed it. If you feel like looking through the code, please do. I think that everything should be fine, except perhaps for some the comments (I remember do them all being right, though), and you want to throw some advice my way, for doing something wrong, or simply how I could do something better, please let me know.

Jumpster, thanks very much for your offer. If I have any more problems related to the list, I'll definatly take you up on that.

Thanks [smile]

Edit: Oh, and is it just me, or does a lot of the STL code give you a headache when you look at it?

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this