[AD] bug with Allegro packfile compression

[Peter Wang <tjaden@xxxxxxxxxx>]

The bug was introduced here: http://www.allegro.cc/forums/thread/587170

I think I have a fix.  I would appreciate help looking it over (see the
comments for detailed explanation of the problem) and stress testing it,
since Evert is planning to release 4.2.1 this weekend.  In addition to
the test case in the thread, I've attached a shell script which produces
files using /dev/zero and /dev/urandom, compresses and decompresses them
with the `pack' tool and checks they are the same.

I've tried to minimise the number of changes and maintaining PACKFILE
semantics, in particular that pack_feof() returns TRUE immediately after
the last byte has been read (unlike feof() which returns true once you
try to read _past_ the last byte).

Peter

--- include/allegro/internal/aintern.h	(revision 7523)
+++ include/allegro/internal/aintern.h	(local)
@@ -136,6 +136,8 @@ AL_VAR(int, _packfile_datasize);
 AL_VAR(int, _packfile_type);
 AL_FUNC(PACKFILE *, _pack_fdopen, (int fd, AL_CONST char *mode));
 
+AL_FUNC(int, _al_lzss_incomplete_state, (AL_CONST LZSS_UNPACK_DATA *dat));
+
 
 /* config stuff */
 void _reload_config(void);
--- src/file.c	(revision 7523)
+++ src/file.c	(local)
@@ -2688,6 +2688,28 @@ static int normal_fclose(void *_f)
 
 
 
+/* normal_no_more_input:
+ *  Return non-zero if the number of bytes remaining in the file (todo) is
+ *  less than or equal to zero.
+ *
+ *  However, there is a special case.  If we are reading from a LZSS
+ *  compressed file, the latest call to lzss_read() may have suspended while
+ *  writing out a sequence of bytes due to the output buffer being too small.
+ *  In that case the `todo' count would be decremented (possibly to zero),
+ *  but the output isn't yet completely written out.
+ */
+static INLINE int normal_no_more_input(PACKFILE *f)
+{
+   /* see normal_refill_buffer() to see when lzss_read() is called */
+   if (f->normal.parent && (f->normal.flags & PACKFILE_FLAG_PACK) &&
+       _al_lzss_incomplete_state(f->normal.unpack_data))
+      return 0;
+
+   return (f->normal.todo <= 0);
+}
+
+
+
 static int normal_getc(void *_f)
 {
    PACKFILE *f = _f;
@@ -2697,7 +2719,7 @@ static int normal_getc(void *_f)
       return *(f->normal.buf_pos++);
 
    if (f->normal.buf_size == 0) {
-      if (f->normal.todo <= 0)
+      if (normal_no_more_input(f))
 	 f->normal.flags |= PACKFILE_FLAG_EOF;
       return *(f->normal.buf_pos++);
    }
@@ -2790,7 +2812,7 @@ static int normal_fseek(void *_f, int of
       f->normal.buf_size -= i;
       f->normal.buf_pos += i;
       offset -= i;
-      if ((f->normal.buf_size <= 0) && (f->normal.todo <= 0))
+      if ((f->normal.buf_size <= 0) && normal_no_more_input(f))
 	 f->normal.flags |= PACKFILE_FLAG_EOF;
    }
 
@@ -2815,7 +2837,7 @@ static int normal_fseek(void *_f, int of
 	    lseek(f->normal.hndl, i, SEEK_CUR);
 	 }
 	 f->normal.todo -= i;
-	 if (f->normal.todo <= 0)
+	 if (normal_no_more_input(f))
 	    f->normal.flags |= PACKFILE_FLAG_EOF;
       }
    }
@@ -2857,7 +2879,7 @@ static int normal_refill_buffer(PACKFILE
    if (f->normal.flags & PACKFILE_FLAG_EOF)
       return EOF;
 
-   if (f->normal.todo <= 0) {
+   if (normal_no_more_input(f)) {
       f->normal.flags |= PACKFILE_FLAG_EOF;
       return EOF;
    }
@@ -2908,7 +2930,7 @@ static int normal_refill_buffer(PACKFILE
    f->normal.buf_pos = f->normal.buf;
    f->normal.buf_size--;
    if (f->normal.buf_size <= 0)
-      if (f->normal.todo <= 0)
+      if (normal_no_more_input(f))
 	 f->normal.flags |= PACKFILE_FLAG_EOF;
 
    if (f->normal.buf_size < 0)
--- src/lzss.c	(revision 7523)
+++ src/lzss.c	(local)
@@ -547,3 +547,15 @@ int lzss_read(PACKFILE *file, LZSS_UNPAC
 
    return size;
 }
+
+
+
+/* _al_lzss_incomplete_state:
+ *  Return non-zero if the previous lzss_read() call was in the middle of
+ *  unpacking a sequence of bytes into the supplied buffer, but had to suspend
+ *  because the buffer wasn't big enough.
+ */
+int _al_lzss_incomplete_state(AL_CONST LZSS_UNPACK_DATA *dat)
+{
+   return dat->state == 2;
+}

Attachment: stress.sh
Description: Bourne shell script