Re: [AD] bugfix in floodfill()

[Eric Botcazou <ebotcazou@xxxxxxxxxx>]

> No, because the first thing that floodfill() does is make sure
> coordinates are in range. Only nonnegative values are in range. Well, it
> doesn't really matter, I can take away the `unsigned' and use my first
> patch.

I'd prefer.

> Are you seriously suggesting to check explicitly that an array does not
> have more than 2,147,483,647 elements??? With 12 bytes per element, we
> would first have to allocate 6 times more bytes than can be addressed by a
> 32-bit word, which is absurd (especially since crucial parts of Allegro
> still assume sizeof(pointer)=sizeof(int)=sizeof(long)>=4). With the same
> logic, we would have to check integer overflow every time we change an
> array index. We don't want to do that :-)

Yes, the ASSERT would be probably never trigger. My suggestion was made 
essentially from the maintenance viewpoint: when someone will look at the 
diff in the future, he will only see a 'short' -> 'int' change.

But I'd be happy with a comment instead.

> > Moreover, there is no array in flood.c at all, only a pointer to
> > scratch mem.
>
> It is used as ((FLOODED_LINE*)_scratch_mem + p->next), so it is used in
> pointer addition, which is equivalent to array indexing.

Essentially, yes. 

> More seriously though, it could be useful to do
>
>     ASSERT(bmp->w<32768 && bmp->h<32768)
>
> But that test would probably be better to do once and for all when the
> bitmap is created, because there are too many places that need this
> assumption.

Sure.

-- 
Eric Botcazou