Re: [AD] compressing empty files

[rpgrca@xxxxxxxxxx]

Please check (and confirm) this:

1. First this, which is straightforward (and was discovered after fixing the
   EOF problem):

   AL_INLINE(int, pack_getc, (PACKFILE *f),
   {
      f->buf_size--;
      if (f->buf_size > 0)
         return *(f->buf_pos++);
      else
         return _sort_out_getc(f);
   })

   We have one byte left in the buffer, none in the disk. So f->buf_size is 1.
   The f->buf_size becomes 0, and fails the check. We call the _sort_out_getc
   (which fills the pack buffer). But we lost the byte we had. So, I suggest
   either postdecrementing in the check or checking for greater or equal to 0.

2. Now, most file.c rely on pack_getc() returning EOF. However, that is not
   true (maybe a bug?). Check the pack_getc() again. The _sort_out_getc() is
   called (as well as in a few more places):

   int _sort_out_getc(PACKFILE *f)
   {
      if (f->buf_size == 0) {
         if (f->todo <= 0)
            f->flags |= PACKFILE_FLAG_EOF;
         return *(f->buf_pos++);
      }
      return refill_buffer(f);
   }

   If the packfile is completely empty (no bytes left in the buffer, no bytes
   left in the disk), it just sets the PACKFILE_FLAG_EOF flag on, but returns
   *(f->buf_pos++). What is inside there? As far as I checked, the buffer has
   not been filled with EOF. Check this at pack_fread():

      i = _sort_out_getc(f);
      if (i == EOF)

3. Finally, refill_buffer() ends this way:

      f->todo -= f->buf_size;
      f->buf_pos = f->buf;
      f->buf_size--;
      if (f->buf_size <= 0)
         if (f->todo <= 0)
            f->flags |= PACKFILE_FLAG_EOF;

      return *(f->buf_pos++);

   So, if both the buffer and the disk are empty, turns the PACKFILE_FLAG_EOF
   on, but still returns the following position, which might or mightn't be EOF.

This is the patch I suggest (merging all the patchs around):

--- cvs/allegro/src/file.c      Wed Mar  6 09:50:12 2002
+++ mod/allegro/src/file.c      Fri Jun 28 23:13:44 2002
@@ -2083,7 +2083,7 @@
    if (f->buf_size == 0) {
       if (f->todo <= 0)
 	 f->flags |= PACKFILE_FLAG_EOF;
-      return *(f->buf_pos++);
+         return (EOF);
    }
    return refill_buffer(f);
 }
@@ -2153,8 +2153,10 @@
    f->buf_pos = f->buf;
    f->buf_size--;
    if (f->buf_size <= 0)
-      if (f->todo <= 0)
+      if (f->todo <= 0) {
 	 f->flags |= PACKFILE_FLAG_EOF;
+         return (EOF);
+      }
 
    return *(f->buf_pos++);
 
--- cvs/allegro/include/allegro/inline/file.inl	Tue Nov  6 13:06:46 2001
+++ allegro/include/allegro/inline/file.inl	Fri Jun 28 22:40:38 2002
@@ -26,7 +26,7 @@
 AL_INLINE(int, pack_getc, (PACKFILE *f),
 {
    f->buf_size--;
-   if (f->buf_size > 0)
+   if (f->buf_size >= 0)
       return *(f->buf_pos++);
    else
       return _sort_out_getc(f);


I tested it under DJGPP, and worked (pack input packed; pack u packed output ->
output is equal to input, being it 0, 1 or 2 bytes, more test encouraged ;).

There seems to be a small recursion (pack_getc -> _sort_out_getc ->
refill_buffer -> pack_getc -> _sort_out_getc -> refill_buffer, which breaks the
recursion returning EOF). I think this behaviour is also done without this
patch. Maybe someone should look at that.

I am going out for a week, so I cannot test this under anything else, nor check
if there are more of this "turn the flag but not returning EOF" case. I am
taking the relevant files in the Palm, though (hmm... PalmAlleg? ;)


--
Roberto Alfonso (rpgrca@xxxxxxxxxx <> rpgrca@xxxxxxxxxx)
Rhynox, the Noble Rockfriend, Wild Battlerager of the Neidar Clan and
Proficient Blacksmith, rising hero, male dwarf (Genesis is the key)
ICQ: 44361979