sezero / mikmod

Mikmod Sound System (mirror of git repo at https://sf.net/projects/mikmod/)
http://mikmod.sourceforge.net/
69 stars 21 forks source link

UniTrk issue #43

Closed sezero closed 3 years ago

sezero commented 3 years ago

As reported by Thomas Neumann (@neumatho):

As I understand, then UniExpand() only makes sure, that there is enough
room in the buffer for the given number of bytes. If that's true, then
in UniNewline(), there is a call to UniExpand(). What I can see, it only
updates the len for the current row, but it need to be sure that there
is room for the next len or end marker. I will mean that the argument
should just be the number 1 (also the unitt and unipc was reversed anyway,
so it always gave a negative number).

Why is UniExpand() called at all in UniDup()? It just make a copy of the
current row, no need to expand. Well, its write the end marker, but we
know there is room for that in the UniNewline() call. Even if that's not
called, we know there is a buffer on at least 1 byte, which was allocated
in UniInit().

I propose the following patch:

diff --git a/libmikmod/playercode/munitrk.c b/libmikmod/playercode/munitrk.c
index bf41a9b..bc5ca46 100644
--- a/libmikmod/playercode/munitrk.c
+++ b/libmikmod/playercode/munitrk.c
@@ -275,7 +275,7 @@ void UniNewline(void)
        unibuf[lastp]+=0x20;
        unipc = unitt+1;
    } else {
-       if (UniExpand(unitt-unipc)) {
+       if (UniExpand(unipc-unitt)) {
            /* current and previous row aren't equal... update the pointers */
            unibuf[unitt] = len;
            lastp = unitt;

Comments? @AliceLR?

sezero commented 3 years ago

Slightly modified patch pushed as 8bf9844c4f2a177a544240bdfb1dad0c8bd129e5