Bug#688934: Another way to trigger this bug

[Sylvain]

Hi,

On Sun, Oct 14, 2012 at 11:13:42AM +0200, Bas Wijnen wrote:
> > I'm not sure in what way an unclean spr[0] will affect the game, but
> > it might, and cleaning it would affect compatibility with the original
> > Dink.
> 
> I don't think it would. Seth doesn't actually use it. He just skipped it
> because he wants to start his indices at 1. However, I don't suggest it
> should be cleaned. That would only hide other bugs.
[snip!]
> As for the bug, it really is several bugs. You have fixed the warp
> problem by ignoring memory corruption, which is good. Still, I would
> suggest that the memory should not get corrupted in the first place.
> That is, if the engine tries to create a sprite, but there are no free
> slots for it, it should also detect this and refuse to write into
> spr[0]. I have found two places where the engine creates sprites
> (add_random_blood and the creation of flying duck heads), and that may
> be all, but I didn't check. In both cases, there is no check if the
> sprite creation actually worked, and there should be.

Well, check this in the first release NEWS file :

  - Some D-Mods have sequences with more than 50 frames. This triggers
    a buffer overflow in the original game BUT the overwritten data is
    the animation metadata ('delay' and 'special' fields), which is
    not critical for non-animated sprite sequences. Example: "The
    Green Voice in my Head - Part 1 - Hangover & Agony", first save
    game then east then house on the right: it shows a shelf with
    broken vases with is seq 66 frame 65 (> 50). FreeDink reproduces
    this behavior by allowing up to 1000 frames in a sequence but
    without the buffer overflow part.

[I think I lowered 1000 to MAX_FRAMES_PER_ABUSED_SEQUENCE afterwhile]

Which means: last time I fixed a struct miswrite, I broke a D-Mod.

Clearly Seth didn't *intend* to use spr[0], but the code just might.
And D-Mods might depend on this to work, especially those with
not-so-clean DinkC code.

At the very least, let's tag the miswrites in the code with a TODO.
If you tell me you're perfectly sure this has no side effect, we can
fix them as well, but I'd suggest doing that only if we're sure.
Comments?

(I avoid the term 'buffer overflow' here because we 'safely' write in
pre-allocated struct memory, so no need for security people to jump
off their chairs)

-- 
Sylvain