Another iterator bug


04-12-2007 12:25:39

Browsing source code again a found this

#if defined(__x86_64__)
for(long int i = 0; (it+i) != text.end(); i++)
for(int i = 0; (it+i) != text.end(); i++)
tempcp = (it+i).getCharacter();
tempx += mTextHelper->getGlyphWidth(tempcp);
if( tempx > (textArea.x + textArea.width) )
pos.x = textArea.x;
pos.y += getNewlineHeight();
// Ignore this space, because it is used for wrapping
cp = it.getCharacter();
if( i != 0 && mTextHelper->isSpace(tempcp) )
break; // There is another space before we need a new line

in Text::_setCaptionHorizontal.

The problem here is, that this piece of code sits inside a for loop:

for( it = text.begin(); it != text.end(); ++it )

which means, if it is incremented in the first code block and it is equal text.end() then, the increment part of the for statement, will push the iterator past the end, which yields undefined behavior (most likely resulting in a crash).

btw. what is that

#if defined(__x86_64__)
for(long int i = 0; (it+i) != text.end(); i++)

good for? If you want the largest possible unsigned integer type, you should use std::size_t instead.


04-12-2007 12:49:48

don't say where you see it ;)


04-12-2007 17:38:51

Thanks for bringing this up, I will review it. I'm not fond of having integer addition mixed with iterators, I'll probably rewrite this function.

I believe that code was added in for 64 bit compatibility. I wonder if using size_t would work for everybody, like you suggest.


05-12-2007 18:24:25

The use of: std::size_t works here on Linux-AMD64. So if it also works on 32-bit platforms, we should probably remove the: #if defined(__x86_64__) section and go with that.