Member Avatar for VBNick

I was trying to make a for loop count down backwards using this loop, but when it gets to zero, instead of realizing that decrementing will bring the number below zero, it tries to make the unsigned short (-1), which is actually 65535 as unsigned.

of course, this code produces an access violation because there is no elements 65535.

for(unsigned short i = n_elements - 1; i >= 0; i--)
{
    array[i] = blah;
}
Edited by VBNick because: n/a
  • 6 Contributors
  • 9 Replies
  • 170 Views
  • 2 Days Discussion Span
  • Latest Post Latest Post by William Hemsworth
Member Avatar for Nick Evan

>> for(unsigned short i = n_elements - 1; i >= 0; i--) What you're saying here is :
"As long as i is greater than or the same as 0, decrement i."
So when I is 0 this statement will still be true and "i" will be decremented. Because it's an unsigned short, it'll overflow and will become 65535. To fix this: for(unsigned short i = n_elements - 1; i > 0; i--)

Member Avatar for Fbody

@Nick Evan:
>> for(unsigned short i = n_elements - 1; i > 0; i--)
That's a good idea, but with that the OP can't access element 0 of the array because the loop won't run. I don't like couning on the overflow, but I think a better answer would be to make the looping condition compound. for(unsigned short i = n_elements - 1; ((i >= 0) [B]&& (i < n_elements)[/B]); i--) Another option would be to avoid the overflow situation altogether by placing the subtraction in the body of the loop instead. This could be confusing to other readers of the code though.

for(unsigned short i = n_elements; i > 0; i--)
{
    array[i - 1] = blah;
}

@OP:
Nope, not a bug in VS2010. This phenomenon is known as "overflow". It's what happens when a variable is pushed past it's legal numeric limits.

The unsigned nature of your variable really causes an unusual situation, as you are now seeing, you may want to reconsider that choice.

How big is your array? Is there any specific reason you can't just use a signed int? A regular signed int will get you a range of approximately -2.1 Billion to +2.1 Billion and your original condition will work just fine.

Edited by Fbody because: n/a
commented: I guess posting when I should be in bed affects my ability to think :) +15
Member Avatar for chococrack

This isn't a bug in VS if that is the question.

Is there any reason you are insistent on using an unsigned short for your loop control variable? It would be much simpler to just use a plain old int

Member Avatar for Ancient Dragon

With the loop counter declared as unsigned short what will your program do if n_elements is 0? I would think that declaring the loop counter as int would be a lot safer. If n_elements is an unsigned int then you might want to typecast it to int in order to remove warnings about comparing signed and unsigned int.

Edited by Ancient Dragon because: n/a
Member Avatar for VBNick

my point is this: the compiler didn't generate any warnings for this situation. Shouldn't it generate some kind of error or warning if an attempt is made to decrement an unsigned number below 0?

Nope, not a bug in VS2010. This phenomenon is known as "overflow". It's what happens when a variable is pushed past it's legal numeric limits.

but I guess there are more situations than just this, like incrementing past its upper limit and whatnot. This specific error though really confused me when I got it..and I think that the compiler should at least generate some kind of warning

I would think that declaring the loop counter as int would be a lot safer.

I agree this would be the simplest fix.

edit-

what will your program do if n_elements is 0

unfortunately, also a good point =/
in which case I guess the solution would be to use this: for(int i = n_elements; i > 0; i--){} and use (i - 1) inside the loop.

This could be confusing to other readers of the code though.

its a very short loop, so I think I will use the second option.


Thanks guys

P.S.
I also attached the code if anyone is interested. I needed a linked list with some special properties for my specific application, so I made this. Its a "Warm list" linked list. The properties of "ItemController" are meant to be overridden.

usage would be like this:

class MyController : public ItemController<MyItemType>
{
public:
	bool Enable(MyItemType *p);
	void Disable(MyItemType *p);
	bool Reset(MyItemType *p);
};

LList<MyItemType, MyController> my_list;

my_list.Build(n_items);

MyItemType *item = my_list.Get()

// ...

my_list.Release(item);

my_list.Collapse();
#ifndef INC_LLIST_H
#define INC_LLIST_H

#include <list>

template<class T>
class ItemController
{
public:
	bool Enable(T *s){return true;}
	void Disable(T *p){}
	bool Reset(T *p){return true;}
};

template<class T,  class C = ItemController<T>>
class LList
{
private:
	struct ITEM
	{
		T p;
		int ind;
	};

	C i_ctrl;

	ITEM *items;
	std::list<int> free_items;
	int n_elements;
public:
	LList()
	{
		items = null;
		n_elements = 0;
	}

	~LList()
	{
		Collapse();
	}

	bool Build(int n_elements_)
	{
		Collapse();

		n_elements = n_elements_;
		
		items = new ITEM[n_elements];
		for(int i = n_elements; i > 0; i--)
		{
			items[i-1].ind = i-1;

			if(!i_ctrl.Enable((T*)&items[i-1]))
			{
				Collapse();
				return false;
			}
			free_items.push_back(i-1);
		}

		return true;
	}

	void Collapse()
	{
		free_items.clear();
		for(int i = 0; i < n_elements; i++)
		{
			i_ctrl.Disable((T*)&items[i]);
		}
		n_elements = 0;
	}

	T *Get()
	{
		T *tmp = null;

		if(free_items.empty() == false)
		{
			tmp = (T*)&items[free_items.back()];
			free_items.pop_back();
		}
		return tmp;
	}

	void Release(T *p)
	{
		free_items.push_back(((ITEM*)p)->ind);
		i_ctrl.Reset(p);
	}

};

#endif /* INC_LLIST_H */
Edited by VBNick because: n/a
Member Avatar for Fbody

>>I think that the compiler should at least generate some kind of warning
Unfortunately, overflow conditions generally don't, because the compiler doesn't check that while compiling. There's really no efficient way to do it without actually running the code.

All an integer overflow does is change the value the system sees, just like any other operation. For example, if you have a signed short int == 0, it's represented as 0000 0000 0000 0000. If you decrement it, it becomes -1 which is 1111 1111 1111 1111 in a 2's compliment system (which most PCs are). With an unsigned, 1111 1111 1111 1111 is interpreted as 65535 instead. It is still a mathematically valid operation, it just produces "strange" results because of how the represented value is interpreted.

Floating point overflow on the other hand, now that can get ugly in a hurry. It's because CPUs operate on integers and floating points differently.

Edited by Fbody because: n/a
Member Avatar for William Hemsworth

Another more common method would be to just loop from the start to end, instead of from the end to the start.

for (unsigned short i = 0; i < n_elements; ++i) {
   array[i] = blah;
}
Member Avatar for VBNick

Thank you captain obvious =)

commented: ha +2
Member Avatar for William Hemsworth

Thank you captain obvious =)

Well, the problem's so easily avoided I think it's barely worth trying to understand,
(also I admit I didn't read through the posts properly first :P)

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.